GNOME Bugzilla – Bug 702648
Tests failing on PPC (egg-secure-memory.c)
Last modified: 2019-02-22 11:58:26 UTC
Hello, From the debian BTS http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=678496 =====8<==== As you can see in the build logs linked from https://buildd.debian.org/status/logs.php?pkg=gcr&arch=powerpc , some tests fail on powerpc. I tracked this down to a crash caused by gcc-4.6 miscompiling egg/egg-secure-memory.c with -O2 or -O1. It doesn't happen if only that file is compiled with -O0 or no -O stanza. The same crash occurs in my GNOME session's gnome-keyring-daemon, making the keyring non-functional. Unfortunately, I wasn't able to further isolate a specific -f... option causing the problem. However, the problem doesn't occur when compiling with gcc-4.7, even with -O2. So, I suggest a temporary workaround in gcr, either: * Build egg/egg-secure-memory.c without optimization on powerpc if the gcc version is 4.6. Note that this file is compiled twice, once for the egg/libegg_la-egg-secure-memory.lo target and once for egg/egg-secure-memory.lo. * Force building with gcc-4.7 on powerpc. =====8<==== The 2 checks failing are: TEST: test-secmem... (pid=20565) /secmem/alloc_free: FAIL GTester: last random seed: R02Se7e6bc714912d8da2301edd6e06a137d /gck/builder/data-secure: FAIL GTester: last random seed: R02Sd1fc7537db3ebc2c6dfea2655cfc5039
(In reply to comment #0) > suggest a temporary workaround in gcr, either: > > * Build egg/egg-secure-memory.c without optimization on powerpc if the gcc > version is 4.6. Note that this file is compiled twice, once for the > egg/libegg_la-egg-secure-memory.lo target and once for > egg/egg-secure-memory.lo. > * Force building with gcc-4.7 on powerpc. Are these things you can do in your package?
This could probably be done by forcing the CC executable to 4.7, the optimizations might be more tricky. An idea how to trouble shoot this? I could get access to a PPC machine to test this.
Hi, I gave a gdb a shot with test-secmem on ppc. Turned out VALGRIND_MAKE_MEM_DEFINED was corrupting its pointer argument in egg-sec-memory.c:sec_free(). Following patch let all the tests pass except test-system-prompt (see below). By the way, the autoconf stuff to enable valgrind support code looks dodgy. What would be the rationale for unleashing the valgrind voodo in production code ? Thanks, Benjamin Jacobs TEST: test-system-prompt... (pid=30982) /gcr/system-prompt/open: (test-system-prompt:30982): Gcr-CRITICAL **: mock prompter couldn't get session bus address: Error spawning command line `dbus-launch --autolaunch=cff03f234c4fbcf69d7849ba4a240564 --binary-syntax --close-stderr': Child process exited with code 1 FAIL diff -urN gcr-3.8.2.vanilladebian/egg/egg-testing.c gcr-3.8.2/egg/egg-testing.c --- gcr-3.8.2.vanilladebian/egg/egg-testing.c 2013-03-25 11:18:02.000000000 +0100 +++ gcr-3.8.2/egg/egg-testing.c 2013-06-20 00:15:38.000000000 +0200 @@ -48,7 +48,7 @@ #ifdef WITH_VALGRIND return RUNNING_ON_VALGRIND; #else - return FALSE + return FALSE; #endif } diff -urN gcr-3.8.2.vanilladebian/egg/Makefile.am gcr-3.8.2/egg/Makefile.am --- gcr-3.8.2.vanilladebian/egg/Makefile.am 2013-04-21 14:31:46.000000000 +0200 +++ gcr-3.8.2/egg/Makefile.am 2013-06-20 00:15:38.000000000 +0200 @@ -10,8 +10,7 @@ INCLUDES = \ -I$(top_srcdir) \ -I$(top_builddir) \ - -I$(top_srcdir)/build \ - -DWITH_VALGRIND + -I$(top_srcdir)/build libegg_la_CFLAGS = \ $(GLIB_CFLAGS) \ diff -urN gcr-3.8.2.vanilladebian/egg/Makefile.in gcr-3.8.2/egg/Makefile.in --- gcr-3.8.2.vanilladebian/egg/Makefile.in 2013-05-04 21:33:28.000000000 +0200 +++ gcr-3.8.2/egg/Makefile.in 2013-06-20 00:16:40.000000000 +0200 @@ -426,8 +426,7 @@ INCLUDES = \ -I$(top_srcdir) \ -I$(top_builddir) \ - -I$(top_srcdir)/build \ - -DWITH_VALGRIND + -I$(top_srcdir)/build libegg_la_CFLAGS = \ $(GLIB_CFLAGS) \
Hi again, As requested by Laurent, I tested the configure with --enable-valgrind. This does indeed sets WITH_VALGRIND in config.h (altough you need then to have valgrind installed for the configure script to pass). I suppose the Makefile.in/am is a just mistake that slipped in. The question to enable valgrind support without having valgrind installed is something that I'll leave to you ;-) Thanks, BJ
(In reply to comment #4) > Hi, > > I gave a gdb a shot with test-secmem on ppc. Turned out > VALGRIND_MAKE_MEM_DEFINED was corrupting its pointer argument in > egg-sec-memory.c:sec_free(). Could you file a bug with valgrind? These macros are supposed to be innocuous if valgrind is not running or not supported. > Following patch let all the tests pass except test-system-prompt (see below). > By the way, the autoconf stuff to enable valgrind support code looks dodgy. That's about running under valgrind by default, not about the macros. > What would be the rationale for unleashing the valgrind voodo in production > code ? The macros are markers that when running under valgrind, tell valgrind to treat allocated memory in certain ways. The valgrind macros are supposed to be no-ops when not in use. One wants to be able run packaged code under valgrind.
By the way the macros are defined in build/valgrind/valgrind.h and build/valgrind/memcheck.h. Perhaps there are newer headers floating around that solve this problem for ppc + gcc-4.6? The headers state: /* This file is for inclusion into client (your!) code. You can use these macros to manipulate and query Valgrind's execution inside your own programs. The resulting executables will still run without Valgrind, just a little bit more slowly than they otherwise would, but otherwise unchanged. When not running on valgrind, each client request consumes very few (eg. 7) instructions, so the resulting performance loss is negligible unless you plan to execute client requests millions of times per second. Nevertheless, if that is still a problem, you can compile with the NVALGRIND symbol defined (gcc -DNVALGRIND) so that client requests are not even compiled in. */
> One wants to be able run packaged code under valgrind. Sure, but I also think that the --enable/disable-valgrind flag should be honored > By the way the macros are defined in build/valgrind/valgrind.h and build/valgrind/memcheck.h. Perhaps there are newer headers floating around that solve this problem for ppc + gcc-4.6? The weird part is (if I understood properly) that if you pass --enable-valgrind to the configure, it will look for the memcheck.h abd valgrind.h headers in the system path and not in the build/valgrind directory
I checked the build logs on other arches and I see similar failures in the checks (didn't test if it was related to valgrind or not although). But it's interesting to note that the valgrind package is not built on all the architectures that debian support, it's only available on: amd64, armel, armhf, i386, mips, mipsel, powerpc, s390x. I'm not what are the implications of having the valgrind macros compiled in on the unsupported ones (ia64, s390, kfreebsd-i386, kfreebsd-amd64 and hurd-i386 *cough*)
Hi Stef, (In reply to comment #6) > The macros are markers that when running under valgrind, tell valgrind to treat > allocated memory in certain ways. The valgrind macros are supposed to be no-ops > when not in use. One wants to be able run packaged code under valgrind. That's where I might disagree with you ;) IMHO, Valgrind is a developer tool. Also, in my experience, Valgrind support for the non mainstream arch is fluctuating a lot between releases. I personnaly wouldn't want to enable a code that's messing with hand crafted assembly and ABI like those macros seem to do in user/production releases. > Could you file a bug with valgrind? These macros are supposed to be innocuous > if valgrind is not running or not supported. I cannot find that assertion about this supposed behaviour in the valgrind documentation. Maybe one might want to conditionalize the use of those macro with a real runtime check ? (In reply to comment #8) > The weird part is (if I understood properly) that if you pass --enable-valgrind > to the configure, it will look for the memcheck.h abd valgrind.h headers in the > system path and not in the build/valgrind directory No, the build still use the local copy. I'd expect it to use the system version, which is, indeed, more up to date in Debian.
Tested with an updated (debian) version of valgrind.h and memcheck.h. Still failing at the same locations.
Created attachment 247437 [details] [review] Fix compilation if valgrind support is completely disabled
Created attachment 247438 [details] [review] Allow valgrind support to be completely disabled Valgrind macros will still be enabled by default
Hey Stef, What are the plans for my patches here?
Bug 710983 is essentially the same (and has a fix). It seems that this is a bug in valgrind.h.
The two other patches have been commited, but the root cause of this issue is still not fixed
Bug 710983 has the real fix, as applied to GLib. I recommend applying the same here.
The patch just committed here is breaking jhbuild. gcr now fails to configure with: checking valgrind... checking valgrind/valgrind.h usability... no checking valgrind/valgrind.h presence... no checking for valgrind/valgrind.h... no configure: error: The valgrind headers are missing *** Error during phase configure of gcr: ########## Error running ./autogen.sh --prefix /home/desrt/.cache/jhbuild/install --libdir '/home/desrt/.cache/jhbuild/install/lib' --disable-installed-tests *** [34/90]
Created attachment 264370 [details] [review] Search for valgrind headers in build/ directory This is fixing the build failure introduced by the previous patch if the valgrind headers are not installed in the system path.
Review of attachment 264370 [details] [review]: Looks like this already got committed.