With a build compiled on FC6 (as well as the nightlies from tinderbox), valgrind complains a lot about using uninitialized memory in multiple places. With a build on Fedora 8, I see no errors. Both boxes have gcc 4.1.2, although the compilers have different additional patches. I've attached a log with some of the errors. Additionally, valgrind was complaining about http://mxr.mozilla.org/seamonkey/source/js/src/jsgc.c#1729 I added a line to print localMallocBytes and the error went away. I removed the print and the error didn't come back. valgrind also complained about jsinterp.c:121 (this is one of the errors in the attachment). I added a line to print out cache->disabled and valgrind still complained, but it consistently printed as 0 (which doesn't seem uninitialized).

I'm seeing this with a build from 2008-03-15-09 but not from 2008-03-14-08.

64-bit or 32-bit architecture? /be

32-bit

I can reproduce this on 32-bit and 64-bit.

This bug is observable on Ubuntu 7.10 64-bit and 32-bit.

Both sayre and brendan agree this should block beta pending investigation. +'ing, P1, TM=beta5

Jason: this sounds like a bug calloc'ing large-sized blocks.

These errors are not present in a build using --disable-jemalloc.

What are the right Component and QA Contact field settings for this bug? /be

valgrind only recognizes memory allocations from functions that embed the VALGRIND_MALLOCLIKE_BLOCK() macro from valgrind/valgrind.h. jemalloc does not currently embed the necessary macro calls for integration with valgrind. My understanding is that ordinarily, valgrind replaces the default malloc implementation by replacing the appropriate symbols after libc is loaded. However, the jemalloc symbols override those supplied by valgrind, thus preventing valgrind from using its own allocator. In the absence of memory corruption or crashes that appear memory related, I do not think this is a bug.

Is there a bug to get jemalloc using the right VALGRIND_* macrology? That would be the bug to fix. If it's not on file, feel free to reopen and morph this one, since we really do want to treat the symptom here, not just pin this bug to a hypothesis about cause that was faulty. Thanks, /be

No, there isn't currently a bug to track adding valgrind support in jemalloc, so I'm morphing this one to track the feature addition, as you suggest.

Not sure we can stand the false positives in valgrind output for 1.9/fx3 -- valgrind is pretty important for pre-release testing. Any chance of a fast fix, at least in our tree? /be

I'm working on it this morning, so I should have a patch soon.

The attached patch adds the --with-valgrind configure option. If that option is enabled, and valgrind/valgrind.h is present, jemalloc compiles in the necessary valgrind macro glue to support running Firefox under valgrind. This code should not be left enabled for production builds, because it has to disable in-place realloc(). Valgrind does not provide a way to modify the size of an existing allocation, so we have to malloc/memcpy/free instead. I seem to have tripped on a valgrind bug while developing this patch. An apparent workaround is included in the patch, but I do not yet know if the workaround is sufficient to always avoid problems. The patch also removes the MALLOC_LAZY_FREE code (which was not enabled anyway). That code was removed from the FreeBSD jemalloc (benchmarks showed it to not be a consistent improvement), and it would have required extra work to make lazy deallocation work with valgrind. Note that 'configure' is left out of the patch, so run something like 'make -f client.mk RUN_AUTOCONF_LOCALLY=yes AUTOCONF=autoconf2.13 configure' to generate it locally before adjusting your .mozconfig and rebuilding. I do not know how important this feature is to others, or whether it should be part of the next beta/release. If you are in the know, please feel free to set bug status accordingly.

The supposed valgrind bug turned out to be a bug in my patch. In one place I called VALGRIND_FREELIKE_BLOCK(ptr, size), but the second argument is redzone size, and should have been 0. This mistake caused a neighboring object deallocation to mark part of a remaining object as inaccessible, since the bogus redzone overlapped.

Why not --with-jemalloc-valgrind instead of --with-valgrind, which sounds like you're enabling some valgrind thing for all of Gecko when it's just the jemalloc valgrind hooks?

I hope that we will be able to add other valgrind things to mozilla.

Comment on attachment 311149 [details] [diff] [review] Add --with-valgrind option (In reply to comment #18) > I hope that we will be able to add other valgrind things to mozilla. Ok, then we shouldn't be jemalloc specific in the description of the build option: >+ AC_ARG_WITH([valgrind], >+ [ --with-valgrind Enable valgrind integration hooks in jemalloc], >+ [enable_valgrind="yes"], [enable_valgrind="no"]) >+ AC_CHECK_HEADER([valgrind/valgrind.h], [], [enable_valgrind="no"]) >+ if test "x$enable_valgrind" = "xyes" ; then >+ AC_DEFINE(MOZ_MEMORY_VALGRIND) >+ fi So, maybe "Enable valgrind integration hooks" instead and MOZ_VALGRIND or something as the define since MOZ_MEMORY_* is all jemalloc stuff?

The patch in bug 348798 for arenas uses VALGRIND_HINTS.

Comments #17, #18, #19 were partly addressed, but --with-valgrind still only works if --enable-jemalloc is set. The patch at bug #475876 disentangles this dependency, making the two options independent.