Re: [igt-dev] [PATCH i-g-t 3/7] lib: Stop using assert() for runtime checks

[Petri Latvala <petri.latvala@intel.com>]

On Tue, Apr 23, 2019 at 08:36:43PM -0400, Lyude Paul wrote:
> On Wed, 2019-04-17 at 11:52 +0300, Petri Latvala wrote:
> > On Tue, Apr 16, 2019 at 04:10:57PM -0400, Lyude wrote:
> > > From: Lyude Paul <lyude@redhat.com>
> > > 
> > > In the process of trying to get an up to date version of igt packaged
> > > for Fedora I discovered that if igt was built with -Db_ndebug=true, a
> > > significant portion of the test infrastructure unit tests would start
> > > failing:
> > > 
> > >   1/265 lib: igt_assert                         OK       0.11 s
> > >   2/265 lib: igt_can_fail                       OK       0.08 s
> > >   3/265 lib: igt_can_fail_simple                OK       0.08 s
> > >   4/265 lib: igt_exit_handler                   OK       0.05 s
> > >   5/265 lib: igt_fork                           FAIL     0.05 s (killed by
> > > signal 9 SIGKILL)
> > >   6/265 lib: igt_fork_helper                    OK       0.42 s
> > >   7/265 lib: igt_hdmi_inject                    OK       0.05 s
> > >   8/265 lib: igt_list_only                      OK       0.01 s
> > >   9/265 lib: igt_invalid_subtest_name           OK       0.05 s
> > >  10/265 lib: igt_no_exit                        OK       0.04 s
> > >  11/265 lib: igt_segfault                       OK       0.38 s
> > >  12/265 lib: igt_simulation                     OK       0.02 s
> > >  13/265 lib: igt_stats                          OK       0.04 s
> > >  14/265 lib: igt_subtest_group                  OK       0.03 s
> > >  15/265 lib: igt_no_subtest                     SKIP     0.02 s
> > >  16/265 lib: igt_simple_test_subtests           UNEXPECTEDPASS 0.02 s
> > >  17/265 lib: igt_timeout                        EXPECTEDFAIL 1.02 s
> > > 
> > > Which appeared to stem from the fact that -Db_ndebug=true would strip
> > > assert() calls. While on a first glance of lib/tests/igt_tests_common.h
> > > one would assume that the only user of assert() was the test
> > > infrastructure unit tests themselves, it turns out we've actually been
> > > using this in multiple spots that seem to expect an unconditional
> > > runtime check.
> > 
> > Umm, yes. You've uncovered a bug, but not what you think. With meson,
> > we don't include lib/check-ndebug.h anymore, only with autotools so
> > you missed the important part: Do not attempt to build IGT with
> > b_ndebug=true.
> 
> If igt really isn't supposed to be built with -Db_ndebug=true we really should
> add explicit checks for this in our meson.build file, e.g.
>  
> if get_option("b_ndebug") == true
>     error("Building with -Db_ndebug=true is not supported")
> endif
> 
> Especially since the default in meson upstream is going to be changing to having
> -Db_ndebug=true on release builds pretty soon. I'll include a patch for this in
> the v3 respin.

Yep, an explicit check sounds good. And overriding the default in the
project() call in toplevel meson.build. And having lib/check-ndebug.h
used in meson builds is a TODO item as well, in case people have a
manual -DNDEBUG in their cflags.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev