Bug 83968

Summary: test suite fails on test-dbus with --enable-tests (but no assertions)
Product: dbus Reporter: Ralf Habacker <ralf.habacker>
Component: coreAssignee: Havoc Pennington <hp>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: msniko14
Version: 1.5Keywords: love
Hardware: All   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Fix test failure caused by incomplete dbus_assert implementation when running tests configured with --enable-tests --enable-embedded-tests
Fix test suite fails on test-dbus with enabled-tests

Description Ralf Habacker 2014-09-17 08:04:17 UTC
Just did run dbus test suite on an opensuse 13.1 x86_64 system with git master and got a test failure on test_dbus

I configured with 

cd dbus-autotools-build
../dbus/configure --enable-tests --enable-installed-tests

xxx@yyy:~/src/dbus-autotools-build> make check
Making check in dbus
make[1]: Entering directory `/home/xxx/src/dbus-autotools-build/dbus'
make  check-am
make[2]: Entering directory `/home/xxx/src/dbus-autotools-build/dbus'
make[2]: Für das Ziel »check-am« ist nichts zu tun.
make[2]: Leaving directory `/home/xxx/src/dbus-autotools-build/dbus'
make[1]: Leaving directory `/home/xxx/src/dbus-autotools-build/dbus'
Making check in bus
make[1]: Entering directory `/home/xxx/src/dbus-autotools-build/bus'
make[1]: Für das Ziel »check« ist nichts zu tun.
make[1]: Leaving directory `/home/xxx/src/dbus-autotools-build/bus'
Making check in tools
make[1]: Entering directory `/home/xxx/src/dbus-autotools-build/tools'
make[1]: Für das Ziel »check« ist nichts zu tun.
make[1]: Leaving directory `/home/xxx/src/dbus-autotools-build/tools'
Making check in test
make[1]: Entering directory `/home/xxx/src/dbus-autotools-build/test'
Making check in .
make[2]: Entering directory `/home/xxx/src/dbus-autotools-build/test'
make  check-TESTS
make[3]: Entering directory `/home/xxx/src/dbus-autotools-build/test'
make[4]: Entering directory `/home/xxx/src/dbus-autotools-build/test'
PASS: ../bus/test-bus
../../dbus/test-driver: Zeile 95:   359 Speicherzugriffsfehler  "$@" > $log_file 2>&1
FAIL: ../dbus/test-dbus
PASS: ../bus/test-bus-launch-helper
PASS: ../bus/test-bus-system
PASS: test-shell
PASS: test-printf
PASS: test-corrupt
PASS: test-dbus-daemon
PASS: test-dbus-daemon-eavesdrop
PASS: test-loopback
PASS: test-marshal
PASS: test-refs
PASS: test-relay
PASS: test-syntax
PASS: test-syslog
make[5]: Entering directory `/home/xxx/src/dbus-autotools-build/test'
Making all in .
make[6]: Entering directory `/home/xxx/src/dbus-autotools-build/test'
make[6]: Leaving directory `/home/xxx/src/dbus-autotools-build/test'
Making all in name-test
make[6]: Entering directory `/home/xxx/src/dbus-autotools-build/test/name-test'
make[6]: Für das Ziel »all« ist nichts zu tun.
make[6]: Leaving directory `/home/xxx/src/dbus-autotools-build/test/name-test'
make[5]: Leaving directory `/home/xxx/src/dbus-autotools-build/test'
============================================================================
Testsuite summary for dbus 1.8.99
============================================================================
# TOTAL: 15
# PASS:  14
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See test/test-suite.log
Please report to https://bugs.freedesktop.org/enter_bug.cgi?product=dbus
============================================================================
make[4]: *** [test-suite.log] Fehler 1
make[4]: Leaving directory `/home/xxx/src/dbus-autotools-build/test'
make[3]: *** [check-TESTS] Fehler 2
make[3]: Leaving directory `/home/xxx/src/dbus-autotools-build/test'
make[2]: *** [check-am] Fehler 2
make[2]: Leaving directory `/home/xxx/src/dbus-autotools-build/test'
make[1]: *** [check-recursive] Fehler 1
make[1]: Leaving directory `/home/xxx/src/dbus-autotools-build/test'
make: *** [check-recursive] Fehler 1

xxx@yyy:~/src/dbus-autotools-build> cat test/test-suite.log
======================================
   dbus 1.8.99: test/test-suite.log
======================================

# TOTAL: 15
# PASS:  14
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: ../dbus/test-dbus
=======================

>>> >>> Each value by itself 120 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 120 this test (120 cumulative)
>>> >>> Each value by itself with arrays as blocks 120 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 120 this test (240 cumulative)
>>> >>> All values in one big toplevel 1 iteration
 100% 1 this test (241 cumulative)
>>> >>> Each value,value pair combination as toplevel, in both orders 14400 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 14400 this test (14641 cumulative)
>>> >>> Each container containing each value 840 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 840 this test (15481 cumulative)
>>> >>> Each container containing each value with arrays as blocks 840 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 840 this test (16321 cumulative)
>>> >>> Each container of same container of each value 840 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 840 this test (17161 cumulative)
>>> >>> Each container of same container of same container of each value 840 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 840 this test (18001 cumulative)
>>> >>> Each value,value pair inside a struct 14400 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 14400 this test (32401 cumulative)
>>> >>> All values in one big struct 1 iteration
 100% 1 this test (32402 cumulative)
>>> >>> Each value in a large array 120 iterations
 0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100% 120 this test (32522 cumulative)
skipping remaining marshal-recursive tests, run with DBUS_TEST_SLOW=1 (or more) to enable
32522 total iterations of recursive marshaling tests
each iteration ran at initial offsets 0 through 9 in both big and little endian
out of memory handling was not tested
Comment 1 Simon McVittie 2014-09-17 11:42:30 UTC
(In reply to comment #0)
> ../../dbus/test-driver: Zeile 95:   359 Speicherzugriffsfehler  "$@" >
> $log_file 2>&1

My technical German is not very good; please run this in English, or say what a Speicherzugriffsfehler is.

I assume, from the fact that the log cuts off with no obvious error messages, that it is a segmentation fault or something. If so, backtrace please.
Comment 2 Simon McVittie 2014-09-17 11:48:06 UTC
Works for me (Debian unstable, x86-64), albeit possibly not with exactly the same configure options.
Comment 3 Simon McVittie 2014-09-17 11:52:11 UTC
Configuring in the same way you did produces this message:

> NOTE: building with embedded tests but without assertions means
> tests may not properly report failures (this configuration is
> only useful when doing something like profiling the tests)

I wouldn't be surprised if there's a bug in this configuration. Add --enable-asserts to get something more useful.
Comment 4 Simon McVittie 2014-09-17 11:59:01 UTC
Yes, the bug is that dbus/dbus-message-util.c has side-effects in assertions, e.g.:

  _dbus_assert (dbus_message_iter_open_container (&array_iter, DBUS_TYPE_STRUCT,
			  			  NULL, &struct_iter));

Patches welcome.
Comment 5 Ralf Habacker 2014-09-18 12:05:16 UTC
Created attachment 106499 [details] [review]
Fix test failure caused by incomplete dbus_assert implementation when running tests configured with --enable-tests  --enable-embedded-tests
Comment 6 Ralf Habacker 2014-09-18 12:05:54 UTC
(In reply to comment #4)
> Yes, the bug is that dbus/dbus-message-util.c has side-effects in
> assertions, e.g.:
> 
>   _dbus_assert (dbus_message_iter_open_container (&array_iter,
> DBUS_TYPE_STRUCT,
> 			  			  NULL, &struct_iter));
> 
The problem here is that with disabled asserts dbus_assert(...) compiles to a noop, which skips the contained functions call. 

> Patches welcome.

see attachment
Comment 7 Simon McVittie 2014-09-18 13:46:54 UTC
Comment on attachment 106499 [details] [review]
Fix test failure caused by incomplete dbus_assert implementation when running tests configured with --enable-tests  --enable-embedded-tests

Review of attachment 106499 [details] [review]:
-----------------------------------------------------------------

::: dbus/dbus-internals.h
@@ +125,4 @@
>  const char* _dbus_strerror (int error_number);
>  
>  #ifdef DBUS_DISABLE_ASSERT
> +#define _dbus_assert(condition) do { dbus_bool_t r = (condition) != 0 ? 0 : 1; } while (0)

No, the intention is that _dbus_assert(condition) does nothing in production builds of D-Bus, even if @condition is expensive to evaluate - just like standard C assert().

Possible solutions are

- don't configure dbus like this, it makes very little sense to enable
  tests but not enable the assertions that check for failures

- add a _dbus_assert_se() or something which *does* guarantee to evaluate
  its argument for its side-effects even if assertions are disabled

- use this idiom instead of _dbus_assert (dbus_do_some_thing (...)):

  dbus_bool_t ok;
  ...
  ok = dbus_do_some_thing (...);
  _dbus_assert (ok);
Comment 8 Simon McVittie 2014-09-18 13:49:06 UTC
> dbus_bool_t r = (condition) != 0 ? 0 : 1;

This is going to cause compiler warnings in recent gcc: "value assigned to r is never used" or similar.

If you're using code similar to this to implement _dbus_assert_se(), either use _DBUS_UNUSED, or something like

    do { if ((condition)) { /* do nothing */ } } while (0)

or have a look at what other projects do for their equivalent macros (I think systemd has an assert_se() and GLib has g_assert_true()).
Comment 9 Ralf Habacker 2014-09-24 21:24:59 UTC
Created attachment 106814 [details] [review]
Fix test suite fails on test-dbus with enabled-tests

Choosed the _dbus_bool_t ok approach.
Comment 10 Simon McVittie 2014-09-25 10:37:29 UTC
Comment on attachment 106814 [details] [review]
Fix test suite fails on test-dbus with enabled-tests

Review of attachment 106814 [details] [review]:
-----------------------------------------------------------------

Looks good to me!
Comment 11 Ralf Habacker 2014-09-25 10:42:26 UTC
Comment on attachment 106814 [details] [review]
Fix test suite fails on test-dbus with enabled-tests

committed to master

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.