QEMU tests, Coverity, and g_test_set_nonfatal_assertions()

[Peter Maydell <peter.maydell@linaro.org>]

Currently we generally assume that assertions are always present
and always fatal, and we tell Coverity this by putting this into
our model file:

void g_assertion_message_expr(const char     *domain,
                              const char     *file,
                              int             line,
                              const char     *func,
                              const char     *expr)
{
    __coverity_panic__();
}

However, this doesn't work for the tests, which use a variety
of other assertion macros like g_assert_cmpstr(), g_assert_true(),
etc, because those glib macros turn into calls to other functions:
 g_assertion_message
 g_assertion_message_cmpstr
 g_assertion_message_cmpnum
 g_assertion_message_error
which we don't model.

So, we could just add models of those four functions. However,
in some of our tests we call g_test_set_nonfatal_assertions()
(which does what it says on the tin for the g_assert_something
macros, but *not* for plain g_assert() or g_assert_not_reached()).
The rationale here is that non-fatal assertions for code in the
glib test framework allow failures to be reported cleanly as
"this test failed", whereas an abort will give less useful
information, eg it doesn't report the test failure name and
it doesn't proceed to do further tests in the same test binary:

**
ERROR:../../../tests/qtest/npcm7xx_rng-test.c:256:test_first_byte_runs:
assertion failed (calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE) >
0.01): (0.00204666737 > 0.01)
Bail out! ERROR:../../../tests/qtest/npcm7xx_rng-test.c:256:test_first_byte_runs:
assertion failed (calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE) >
0.01): (0.00204666737 > 0.01)
Aborted

vs

**
ERROR:../../../tests/qtest/npcm7xx_rng-test.c:232:test_first_byte_monobit:
assertion failed (calc_monobit_p(buf, sizeof(buf)) > 0.01):
(4.78548397e-05 > 0.01)
# ERROR:../../../tests/qtest/npcm7xx_rng-test.c:232:test_first_byte_monobit:
assertion failed (calc_monobit_p(buf, sizeof(buf)) > 0.01):
(4.78548397e-05 > 0.01)
not ok 5 /arm/npcm7xx_rng/first_byte/monobit
ok 6 /arm/npcm7xx_rng/first_byte/runs

Of course test code that opts into this needs to be prepared for
its g_assert_something() to return even on failure. But if we make
our Coverity model treat all these kinds of g_assert_something as
fatal-on-failure then we won't be able to use Coverity to identify
places in these tests that would accidentally crash on failure.

In summary, we have a few options:

(1) Expand "assertions always fatal" to test code, and add "panics"
models of the g_assertion_message* functions. Remove all the calls
to g_test_set_nonfatal_assertions().

(2) Aim to expand the ability to use g_test_set_nonfatal_assertions()
to other tests than the handful that currently use it (perhaps by
providing a standard place where it gets called for all tests, though
there isn't currently an obvious place to do that). Treat Coverity
issues in our test code which flag up "this would crash if the
assertion fired but execution continued" as bugs to be fixed (though
not very high-priority ones, obviously).

(3) Something else ?

I think I vaguely favour 2, though it is of course more work...
In any case, we need to make a decision so we can decide whether
the pile of coverity issues should be either dismissed as intentional
or gradually worked through and fixed.

thanks
-- PMM