15906 - [Porkjockey]Boehm Collector Leak Detector

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Patrick C. Beard]

The Boehm garbage collector leak detector needs to be integrated with Mozilla.
This work is ongoing, and this bug will be used to report status.

Comment 1

[Brendan Eich [:brendan]]

Hooking into the 14468 tree at the MLK node.

/be

Comment 2

[Jim Roskind]

Added some keywords (porkjockey, mlk) to summary to make it clearer to pointy
heads like me what this was all about.

Comment 3

[Patrick C. Beard]

Updated milestone.

Comment 4

[Wade Hennessey]

Attachment: nspr patches required to make boehm work on linux

Comment 5

[Wan-Teh Chang]

Attachment: Revised NSPR patches.

Comment 6

[Wan-Teh Chang]

I modified your patches to use the indentation
style and naming convention in the file. Please
give my patches (attachment id=12576) a try.
(I only tested without GC_LEAK_DETECTOR.) Thanks.

Comment 7

[Wan-Teh Chang]

More comments on my changes to the original
patches (attachment id=12559).
1. I changed the original patches to use the
   same indentation style and naming convention
   that are present in the files patched.
2. In the makefiles, prgcleak.c is compiled
   and linked in only if GC_LEAK_DETECTOR is
   defined.  This is to avoid compiling an empty
   file.
3. I added a new macro _PT_IS_GCABLE_THREAD(thr),
   which is defined to be the constant 1 if
   GC_LEAK_DETECTOR is defined.  This way we don't
   need to figure out where we need to set the
   PT_THREAD_GCABLE flag in thr->state.  I also
   changed ptthread.c to use the _PT_IS_GCABLE_THREAD(thr)
   macro.
4. I deleted #if 0 and commented out code.
5. In the makefiles, I changed
       ../../../dist/lib/libboehm.so
   to
       -L$(DIST)/lib -lboehm
   which is the preferred way to specify a
   library on the linker command line.
6. The code to get stack frame and stack top
   (in ptthread.c) is ifdef'd with __GLIBC__ >= 2,
   because it depends on the definition of jmp_buf
   in glibc2.
7. Only a copy of prgcleak.c is needed.  It will be
   in mozilla/nsprpub/pr/src/memory/.  This will require
   changes to the Mac project file (prgcleak.c is now
   in mozilla/nsprpub/pr/src/md/mac/), which I did not
   address in my patches.

Comment 8

[Wan-Teh Chang]

Attachment: mozilla/nsprpub/pr/src/memory/prgcleak.c

Comment 9

[Wan-Teh Chang]

In prgcleak.c, why was 'mutex' changed from a PRLock
to a PRMonitor?  Do you need it to be a reentrant lock?

Comment 10

[Wan-Teh Chang]

Attachment: Revised mozilla/nsprpub/pr/src/memory/prgcleak.c. Please ignore the previous attachment.

Comment 11

[Wade Hennessey]

We changed the mutex to a monitor because it had to be reentrant.

Comment 12

[Wan-Teh Chang]

I checked in my patch and added mozilla/nsprpub/pr/src/memory/prgcleak.c
on the NSPRPUB_CLIENT_BRANCH.

Comment 13

[Wan-Teh Chang]

Just wanted to note that my checkin was approved
by [e-mail] for the following reasons:
1. this is to support a useful memory leak
   detection method on Linux;
2. the changes are conditionalized with the
   GC_LEAK_DETECTOR macro, so regular builds
   are not affected.

Comment 14

[Wan-Teh Chang]

I fixed a typo ('allocTop' should be 'stackTop') in my
previous checkin (on the NSPRPUB_CLIENT_BRANCH).
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptthread.c, revision: 3.33.2.2

Comment 15

[Wade Hennessey]

All the changes to make Boehm work on Linux have been committed and I just 
verified that they work in a newly checked out and built tree. Now I'm working 
on adding more symbolic information, demangling, etc. in the Leak file so it's 
more equivalent to boehm on the mac.

Comment 16

[Wan-Teh Chang]

I merged my patches onto the main trunk.
/cvsroot/mozilla/nsprpub/config/config.mk, revision: 3.18
/cvsroot/mozilla/nsprpub/pr/include/private/primpl.h, revision: 3.51
/cvsroot/mozilla/nsprpub/pr/src/Makefile, revision: 3.35
/cvsroot/mozilla/nsprpub/pr/src/Makefile.in, revision: 1.14
/cvsroot/mozilla/nsprpub/pr/src/memory/Makefile, revision: 3.8
/cvsroot/mozilla/nsprpub/pr/src/memory/Makefile.in, revision: 1.9
/cvsroot/mozilla/nsprpub/pr/src/misc/prthinfo.c, revision: 3.8
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptthread.c, revision: 3.39

I also merged the patches on the NSPRPUB_RELEASE_4_0_BRANCH.
/cvsroot/mozilla/nsprpub/config/config.mk, revision: 3.14.6.2
/cvsroot/mozilla/nsprpub/pr/include/private/primpl.h, revision: 3.34.4.8
/cvsroot/mozilla/nsprpub/pr/src/Makefile, revision: 3.29.2.4
/cvsroot/mozilla/nsprpub/pr/src/Makefile.in, revision: 1.8.6.3
/cvsroot/mozilla/nsprpub/pr/src/memory/Makefile, revision: 3.5.6.1
/cvsroot/mozilla/nsprpub/pr/src/memory/Makefile.in, revision: 1.6.6.1
/cvsroot/mozilla/nsprpub/pr/src/memory/prgcleak.c, revision: 1.1.4.1
/cvsroot/mozilla/nsprpub/pr/src/misc/prthinfo.c, revision: 3.4.8.2
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptthread.c, revision: 3.32.4.3

Comment 17

[chris hofmann]

beard, 
what do you think the next steps on this one might be???
who should we pull into help?
has thesteve taken ownership of the tool?
do we have any recent results or analysis from the tool published somewhere?
can we round up someone to do that?

curt, this might be the next one to plug into daily smoke test
runs after we get waterson's reporting tools hoioked up.
thanks.

Comment 18

[Simon Paquet [:sipaq]]

happens on all platforms

Comment 19

[Scott Walters]

Attachment: SEGV: invoked program's (built with --enable-boehm) debugging output

SEGV result of attempt to run a program built with --enable-boehm (after
Makefile's were munged to do -lboehm).

Comment 20

[Wan-Teh Chang]

Marked the bug fixed based on comment 16.