429745 - --enable-jemalloc + --enable-debug fails (unresolved posix_memalign symbol in jsgc.c)

Status: RESOLVED WORKSFORME

(Core :: Memory Allocator, defect)

Description

[J. Paul Reed [:preed]]

When building xulrunner with --enable-debug and --enable-jemalloc, the build fails in jsgc.c with:

link -NOLOGO -DLL -OUT:js3250.dll -PDB:js3250.pdb -SUBSYSTEM:WINDOWS  jsapi.obj jsarena.obj jsarray.obj jsatom.obj jsbool.obj jscntxt.obj jsdate.obj jsdbgapi.obj jsdhash.obj jsdtoa.obj jsemit.obj jsexn.obj jsfun.obj jsgc.obj jshash.obj jsinterp.obj jsinvoke.obj jsiter.obj jslock.obj jslog2.obj jslong.obj jsmath.obj jsnum.obj jsobj.obj jsopcode.obj jsparse.obj jsprf.obj jsregexp.obj jsscan.obj jsscope.obj jsscript.obj jsstr.obj jsutil.obj jsxdrapi.obj jsxml.obj prmjtime.obj
 js3240.res -NXCOMPAT -SAFESEH -DYNAMICBASE -MANIFEST:NO  -DEBUG -DEBUGTYPE:CV
       ../../dist/lib/nspr4.lib ../../dist/lib/plc4.lib ../../dist/lib/plds4.lib
  kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib
   Creating library js3250.lib and object js3250.exp
jsgc.obj : error LNK2019: unresolved external symbol _posix_memalign referenced in function _NewGCChunk
js3250.dll : fatal error LNK1120: 1 unresolved externals
make[4]: *** [js3250.dll] Error 96
make[4]: Leaving directory `/c/builds/jemalloc/pristine/mozilla/compiled/xulrunner-debug/js/src'
make[3]: *** [libs_tier_js] Error 2
make[3]: Leaving directory `/c/builds/jemalloc/pristine/mozilla/compiled/xulrunner-debug'
make[2]: *** [tier_js] Error 2
make[2]: Leaving directory `/c/builds/jemalloc/pristine/mozilla/compiled/xulrunner-debug'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/c/builds/jemalloc/pristine/mozilla/compiled/xulrunner-debug'
make: *** [build] Error 2

After spending *way* too much time learning about Win32 DLL linking semantics ;-), it looks like the problem is that when we patch the Win32 CRT for jemalloc, we keep the RETAIL_LIB_NAME as msvcrt.

In debug builds, the linker is looking for msvcrtd (and we're not passing the right -DEFAULTLIB/-NODEFAULTLIB combination(s)), so the link fails, even though the symbol is (mostly) properly exported.

I've got a patch that does get it building in debug mode with jemalloc, but I don't like it for a number of reasons. I'll go into all of them when I post the patch.

Comment 1

[Bob Clary [:bc] (inactive)]

I am seeing this building the debug browser with jemalloc as well 

Comment 2

[J. Paul Reed [:preed]]

I have a patch that fixes this.

Unfortunately, it's pretty lengthy, and I need to clean it up to remove some copy/paste code from the CRT itself.

I'll try to do that by the end of the week.

Comment 3

[J. Paul Reed [:preed]]

Attachment: The CRT Patch, take 1

Here's the CRT patch (direct replacement for memory/jemalloc/crtsp1.diff) that we're "sorta" shipping with; I had to redo some of the preprocessor magic (pronounced: "crap") we're doing here, and I didn't do a full rebuild CRT + XULRunner + Songbird run; I *think* I got it mostly right (I diffed the .i files; whee! They looked Ok.)

This can probably be cleaned up a bit, since after I got it working, I was like "don't touch it!" :-)

I've got a patch for the build system guts-part itself... I'll post that in a sec.

Comment 4

[J. Paul Reed [:preed]]

Attachment: Build system gut-ectomy, Take 1

Goes with the above CRT patch.

Comment 5

[J. Paul Reed [:preed]]

Attachment: More tool munging, take 1

I need to poke at this more, as, again, what I had was working.

This (supposedly) disables jemalloc for certain executables that probably should be linked against the standard runtimes, no matter what (i.e. crashreporter, etc.)

But, after some limited testing, I don't think it's 100% correct; comments from ted or bsmedberg appreciated.

Comment 6

[J. Paul Reed [:preed]]

Changing summary, since this looks like it's not XR-only.

Comment 7

[Bob Clary [:bc] (inactive)]

$ patch -p0 --dry-run < 429745.p2.v1.patch 
patching file configure.in
Hunk #2 FAILED at 6196.
1 out of 2 hunks FAILED -- saving rejects to file configure.in.rej
patching file config/autoconf.mk.in
Hunk #1 succeeded at 583 (offset 4 lines).
patching file config/config.mk
patch: **** malformed patch at line 166: Index: memory/jemalloc/Makefile.in

Comment 8

[J. Paul Reed [:preed]]

(In reply to comment #7)
> $ patch -p0 --dry-run < 429745.p2.v1.patch 
> patching file configure.in
> Hunk #2 FAILED at 6196.
> 1 out of 2 hunks FAILED -- saving rejects to file configure.in.rej
> patching file config/autoconf.mk.in
> Hunk #1 succeeded at 583 (offset 4 lines).
> patching file config/config.mk
> patch: **** malformed patch at line 166: Index: memory/jemalloc/Makefile.in

Hey bc,

This patch was against 1.9b5, not head of trunk; I can look at updating the patch, but can you pull 1.9b5, and try to apply it there real quick?

Comment 9

[Bob Clary [:bc] (inactive)]

against FIREFOX_3_0b5_RELEASE

$ patch -p0 --dry-run < 429745.p2.v1.patch 
patching file configure.in
patching file config/autoconf.mk.in
patching file config/config.mk
patch: **** malformed patch at line 166: Index: memory/jemalloc/Makefile.in

it's not obvious to me what the malformation is... looking.

$ patch -p0 --dry-run < 429745.p3.v1.patch 
patching file config/mkdepend/Makefile.in
patching file modules/libmar/tool/Makefile.in
patching file toolkit/crashreporter/client/Makefile.in
patching file xpcom/typelib/xpidl/Makefile.in
patching file xpcom/typelib/xpt/tools/Makefile.in

Comment 10

[cmtalbert]

Stuart, can you comment here about preed's approach and also document your "by-hand" method for getting debug builds to work with je-malloc on windows?

Comment 11

[J. Paul Reed [:preed]]

(In reply to comment #9)
> against FIREFOX_3_0b5_RELEASE
> 
> $ patch -p0 --dry-run < 429745.p2.v1.patch 
> patching file configure.in
> patching file config/autoconf.mk.in
> patching file config/config.mk
> patch: **** malformed patch at line 166: Index: memory/jemalloc/Makefile.in
> 
> it's not obvious to me what the malformation is... looking.

I caught the problem; I made a stupid edit to the patch before posting it (further up in the file).

I'll attach a new patch against b5 as soon as the "cvs -q di" finishes running over the tree. :-)

A couple of other points of clarification:

1. I'm happy to update this patch to head; I just didn't do that because I didn't have the free time, and don't want to do it unless it was asked for.

2. I understand there's some concern about the scope of change in this patch, and that's completely understandable; I'm happy to recode this patch as:

ifdef MOZ_DEBUG || MOZ_NEW_CRT_MODE
   ... my patch ...
else
   ... old style ...

The nice thing about this is that it gets us debug builds, and makes it easier to make a switch to a mozcrt19.dll that identifies itself as such (which, AIUI, is a large part of the issue) for 1.9.next, but eliminates risk to Fx 3.0.x.y.

Comment 12

[J. Paul Reed [:preed]]

Attachment: Build system gut-ectomy, Take 2

bc: this should fix the patch errors you were seeing; the patch was invalid; it was my bad:

preed@preed-desktop:~/checkouts/test/mozilla$ patch -p0 --dry-run < ~/patches/mozilla/429745.p2.v2.patch 
patching file configure.in
patching file config/autoconf.mk.in
patching file config/config.mk
patching file memory/jemalloc/Makefile.in
patching file memory/jemalloc/build-crt.py
patching file memory/jemalloc/jemalloc.c
patching file toolkit/xre/nsAppRunner.cpp

You *might* see some fuz on xre/nsAppRunner.cpp; it should apply, though.

Comment 13

[Bob Clary [:bc] (inactive)]

yes, that applies cleanly. thanks.

Comment 14

[Bob Clary [:bc] (inactive)]

fyi, I get build failures on opt and debug with

make[5]: Entering directory `/c/work/mozilla/builds/1.9.0-jemalloc-test/mozilla/firefox-debug/config/mkdepend'
make[5]: echo: Command not found

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #14)
> make[5]: echo: Command not found

I've seen things like this before, usually means your path is messed up somehow in make. Try reconfiguring?

Comment 16

[(not currently active) Ted Mielczarek]

yeah, the jemalloc build explicitly sets PATH in autoconf.mk.

Comment 17

[Bob Clary [:bc] (inactive)]

Attachment: build errors

I've tried and tried to get this to build, but keep failing. This is with a fresh tree checked out for Firefox 3.0b5 release with the patches applied and configure rebuilt with autoconf-2.13.

Comment 18

[J. Paul Reed [:preed]]

(In reply to comment #17)
> Created an attachment (id=320973) [details]
> build errors
> 
> I've tried and tried to get this to build, but keep failing. This is with a
> fresh tree checked out for Firefox 3.0b5 release with the patches applied and
> configure rebuilt with autoconf-2.13.

bc: I ran into this error while developing the patch, and I'm trying to remember exactly what caused ed.exe to barf; as I remember, it was something relatively stupid (the diff done in the wrong directory, etc.)

Did you drop the crt patch right into the same place directly, or?

I'm updating this patch to rc1 as we speak, since we're picking up new XulRunners right now.

I can post the new patch, although the only thing that changed that was interesting was the configure.in stuff.

Comment 19

[Bob Clary [:bc] (inactive)]

(In reply to comment #18)
> 
> Did you drop the crt patch right into the same place directly, or?

I overwrote crtsp1.diff

> I can post the new patch, although the only thing that changed that was
> interesting was the configure.in stuff.

rc1==trunk right? Yeah, I would like to try that out since I don't care about b5 any more ;-)

Comment 20

[J. Paul Reed [:preed]]

(In reply to comment #17)
> Created an attachment (id=320973) [details]
> build errors
> 
> I've tried and tried to get this to build, but keep failing. This is with a
> fresh tree checked out for Firefox 3.0b5 release with the patches applied and
> configure rebuilt with autoconf-2.13.

After working through the problem with bc, I think we figured out that his version of the CRT isn't the VS8SP1, or at least doesn't have the right combo of SDKs and service packs:

http://pastebin.mozilla.org/432345

I'm working on an rc1-compliant patch right now; will post when it's ready.

Comment 21

[Bob Clary [:bc] (inactive)]

preed, can you tell me what updates you've applied to visual studio 2005?

Comment 22

[J. Paul Reed [:preed]]

(In reply to comment #21)
> preed, can you tell me what updates you've applied to visual studio 2005?

For visual studio, I've applied:

* Security Update for Microsoft Visual Studio 2005 (KB937060)
* Visual Studio 2005 Service Pack 1
* Security Update for Microsoft Visual Studio 2005 (KB925674)

All of these were applied via Windows update.

Comment 23

[J. Paul Reed [:preed]]

Attachment: The CRT patch, take 2

Changes some instances of MOZ_DEBUG to MOZ_MEMORY_DEBUG because not all parts of the code (js, notably) define MOZ_DEBUG when in debug mode. MOZ_MEMORY_DEBUG, however, does get defined.

(We also propagate MOZ_MEMORY_DEBUG down into the CRT build environment now.)

Comment 24

[J. Paul Reed [:preed]]

Attachment: Build system gut-ectomy, Take 3

Updated for 3.0rc1.

Comment 25

[(not currently active) Ted Mielczarek]

Comment on attachment 320209 [details] [diff] [review]
More tool munging, take 1

So I know I've been putting off reviewing this, I apologize for that. What does this particular patch do that USE_STATIC_LIBS=1 doesn't do? I notice that one of the Makefiles you're changing here already has that present. Does it not work as expected with jemalloc enabled?

Comment 26

[J. Paul Reed [:preed]]

(In reply to comment #25)
> (From update of attachment 320209 [details] [diff] [review])
> So I know I've been putting off reviewing this, I apologize for that. What does
> this particular patch do that USE_STATIC_LIBS=1 doesn't do? I notice that one
> of the Makefiles you're changing here already has that present. Does it not
> work as expected with jemalloc enabled?

We chatted on IRC about this, but beyond the "getting it building in debug"-part, I (mostly, it turns out; not entirely) wanted to separate the concept of building statically and building against jemalloc.

That's why there's now "DISABLE_MOZ_MEMORY" in addition to USE_STATIC_LIBS. (Right now, it's somewhat confusing that USE_STATIC_LIBS automatically implies linking against the standard CRT, and the fact that this sorta "just happens" because the static .libs get deleted in the directory that precedes the standard CRT directory in the PATH is... kinda scary; so I wanted to make that more explicit, by resetting the environment to its standard state and creating [yet another] a flag for it.)

In the Songbird 0.6 cycle, we found a couple of issues with the debug version of this patch, so I'll post that patch; for now, you can probably ignore these patches, Ted.

I'll get them updated against 3.0 final (which should be easy) and include the debug fixes. 

Comment 27

[(not currently active) Ted Mielczarek]

Comment on attachment 320209 [details] [diff] [review]
More tool munging, take 1

Clearing review requests, waiting for updated patches.

Comment 28

[J. Paul Reed [:preed]]

Attachment: crtpatch, v4

This is pretty much the same as take 2, except it exports a few more debug symbols that the standard CRT exports (_sample_.def lies!!)

Ted: after you've looked through the other stuff, I'd be happy to figure out a way to make verifying this particular patch easier in various configurations; I think running this through the cl equiv of -i and comparing the output would be useful, but we can chat about it on IRC.

Comment 29

[J. Paul Reed [:preed]]

Attachment: Build system gut-ectomy, Take 4

Fundamental change here:

-- the LIB, PATH, and INCLUDE environment variables are no longer munged; we munge -I and -LIBPATH as necessary; this makes it clearer when compiling what's actually happening, and also allows config.mk and rules.mk to change the behavior of the compile, instead of putting logic in autoconf.mk.in, which is the more standard pattern.
-- USE_STATIC_LIBS implies DISABLE_MOZ_MEMORY now; there's a comment as to why

Comment 30

[J. Paul Reed [:preed]]

Attachment: Make more stuff static, take 1

Makes various supporting tools (most notably, updater.exe and crashreporter.exe) tools not use jemalloc.

Comment 31

[(not currently active) Ted Mielczarek]

This all sounds great! I promise I'll take a look at it by next week, please ping me repeatedly if I fail to honor that promise.

Comment 32

[(not currently active) Ted Mielczarek]

Comment on attachment 333785 [details] [diff] [review]
Make more stuff static, take 1

Shoulda looked at this patch first. Srsly.

Comment 33

[(not currently active) Ted Mielczarek]

Comment on attachment 333784 [details] [diff] [review]
Build system gut-ectomy, Take 4

 # These are for custom CRT building
 ifdef MOZ_MEMORY

Guess this should just read "this is for custom CRT building" now.

+
+#
+# jemalloc crap; the INCLUDE and LIB paths are munged on Win32
+#

You're not actually munging INCLUDE and LIB, could you phrase this differently?

+MOZ_MEMORY_LDFLAGS += -LIBPATH:$(WIN32_CUSTOM_CRT_DIR)
+MOZ_MEMORY_LDFLAGS += -NODEFAULTLIB:msvcrt -NODEFAULTLIB:msvcrtd
+MOZ_MEMORY_LDFLAGS += -NODEFAULTLIB:libcmt -NODEFAULTLIB:libcmtd

Just make this one statement, with backslash line continuation. You can end the first line after the += to make everything line up nicely.

         WIN32_CRT_SRC_DIR="$VCINSTALLDIR\crt\src"
+

Hey, no introducing stray empty lines into my nice clean configure.in!

+libs:: mozcrtlibs mozcrtruntimedll

You could just replace both of these targets with lib::, I think, for the same effect without extra targets.

+mozcrtlibs: $(MOZ_CRT_DLL) $(MOZ_CRT_LIB) $(MOZ_CRT_PDB)
+	$(INSTALL) $^ $(DIST)/lib

Do you need to install the DLL into dist/lib? We don't tend to stick DLLs there.

+	# Ooh, let's be clever...

Why do you need to do all this file copying and sed? Couldn't this be part of the CRT patch?

+	# debug mode
+	# In debug mode, we need to copy the debug headers that we've patched,
+	# so the mozilla build uses the same headers that we used for the CRT
+	# build; incidentally, this is why INCLUDE now gets munged

I don't see you munging INCLUDE anywhere.

diff -r 889e2e6df7c8 memory/jemalloc/jemalloc.c

Please r?jasone on this bit.

r- to answer or address the points above, but overall looks good!

Comment 34

[Stuart Parmenter]

Comment on attachment 333784 [details] [diff] [review]
Build system gut-ectomy, Take 4

i suggest adding those windows functions to some separate file and possibly #including them, rather than adding them to jemalloc.c or something else clever.

Comment 35

[J. Paul Reed [:preed]]

Quick update:

I haven't forgotten about this; I'm currently working through issues that are most likely the same described by Nelson here:

http://groups.google.com/group/mozilla.dev.builds/browse_thread/thread/e9779909bb9f71bb#

pk12util.exe now crashes after bug 428009 landed.

Comment 36

[(not currently active) Ted Mielczarek]

Comment on attachment 333783 [details] [diff] [review]
crtpatch, v4

Clearing this review request until we get some new patches. I don't really want to review a CRT patch in this format anyway. As we discussed at some point, hopefully you could mail me a unified diff of the CRT source with the current patch applied vs. the CRT source with your new patch applied.

Comment 37

[J. Paul Reed [:preed]]

I'm going to kick this to nobody@; I'm not actively working on this anymore and don't foresee having the time in the (near) future to do so.

Hopefully someone can pick it up if it's decided there's value in being able to get a debug build in the configuration that Win32 Fx ships in (although, with PGO, who knows if that's possible anymore).

FWIW, Songbird has been using this patch for 3-4 releases, so it has had some testing in that regard; the release-mode stuff appears to be stable/usable.

Some answers to Ted's last review questions:

(In reply to comment #33)
> +libs:: mozcrtlibs mozcrtruntimedll
> 
> You could just replace both of these targets with lib::, I think, for the same
> effect without extra targets.

Yeah, the reason I did it this way is to make a distinction between the runtime dll that's necessary, and the rest of the libs that are only useful in an SDK/XR context. As evidenced by below, I wasn't 100% sure where all of this stuff should go, since NSS, NSPR, and the Firefox build itself all do slightly different things; it's possible (probable) that I just didn't discern the proper pattern.

> +mozcrtlibs: $(MOZ_CRT_DLL) $(MOZ_CRT_LIB) $(MOZ_CRT_PDB)
> +    $(INSTALL) $^ $(DIST)/lib
> 
> Do you need to install the DLL into dist/lib? We don't tend to stick DLLs
> there.

Yeah; I did this because I wasn't 100% sure where they should go, and this was a holdover from getting the xulrunner build to do the right thing. I couldn't find any consistent guidance from the makefiles on where to put .dlls vs. the .pdbs vs. the .libs (the latter two of which are possibly-more important in the XR case, but not the Fx case).

> +    # Ooh, let's be clever...
> 
> Why do you need to do all this file copying and sed? Couldn't this be part of
> the CRT patch?

I did it this way because we take the sample def (and friends) files, copy them to new names required for the CRT's build system, and make minor modifications to those; if we included them in the CRT patch, since there new files, there were be a bunch of line additions which would basically consist of a dump of Microsoft-copyrighted source code.

Maybe they don't care, because it's sample code, but I didn't want to take the chance... so we copy the files to the new names in the makefile, and patch them with the CRT patch.

Comment 39

[Bob Clary [:bc] (inactive)]

It would really be *nice* to be able to test debug windows builds with jemalloc. Nominating for wanted1.9.*

Comment 40

[(not currently active) Ted Mielczarek]

I think these changes are probably too invasive to take on branch.

Comment 41

[Benjamin Smedberg]

I agree, we should not be taking jemalloc+debug on the stable branches.

Comment 42

[Samuel Sidler (old account; do not CC)]

Deferring to Ted and Benjamin. Not wanted on either branch.

Comment 43

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: error out if the build would fail

Let's at least error out at configure time if the build wouldn't succeed, so that people aren't confused.

Comment 44

[(not currently active) Ted Mielczarek]

Comment on attachment 443148 [details] [diff] [review]
error out if the build would fail

I think you want test -n for maximum consistency with the rest of configure. (But I could be wrong.)

Sorry, we should have done this ages ago.

Comment 45

[Serge Gautherie (:sgautherie)]

I just got this trying to build a Debug SeaMonkey on MoMe Try:
http://build.mozillamessaging.com/buildbot/try/builders/WINNT%205.2%20tryserver%20build/builds/120/steps/compile/logs/stdio
{
   Creating library mozalloc.lib and object mozalloc.exp
mozalloc.obj : error LNK2019: unresolved external symbol _posix_memalign referenced in function _moz_xposix_memalign
mozalloc.obj : error LNK2019: unresolved external symbol _memalign referenced in function _moz_xmemalign
mozalloc.obj : error LNK2019: unresolved external symbol _malloc_usable_size referenced in function _moz_malloc_usable_size
mozalloc.dll : fatal error LNK1120: 3 unresolved externals
make[6]: *** [mozalloc.dll] Error 96
make[6]: Leaving directory `/e/buildbot/tryserver-win32/build/objdir/mozilla/memory/mozalloc'
}

Comment 46

[Justin Lebar (not reading bugmail)]

I've been building --enable-jemalloc --enable-debug successfully on Linux for a month or so.  I never realized this wasn't supposed to work.  :)  This may be WORKSFORME.

Comment 47

[Dmitry Dartz]

Sorry, Justin, you missed the point. This issue is all about jemalloc and debug under WINDOWS platfrom. The other platforms may NOT be affected because they have nothing to do with mentioned MSVCRT (ms visual C runtime library).

Comment 48

[Justin Lebar (not reading bugmail)]

Indeed; thanks for clearing that up for me.

Comment 49

[Jason Orendorff [:jorendorff]]

This bug tripped me up today, leading to the following exchange on IRC:

<glandium> khuey: we should probably make --enable-jemalloc and --enable-debug
  mutually exclusive in configure.in on windows
<khuey> yeah
• khuey would be ok with that
<khuey> or fixing our python script not to replace __imp__free when it's part of
  __imp__free_dbg
[...]
<jorendorff> khuey, glandium, bsmedberg:
  https://bug429745.bugzilla.mozilla.org/attachment.cgi?id=443148
<jorendorff> already has review from ted, never landed
<bsmedberg> jorendorff: feel free ;-)

But the patch doesn't apply cleanly anymore. I'll post a new one in a minute.

Comment 50

[Jason Orendorff [:jorendorff]]

Attachment: error out if the build would fail, v2

Comment 51

[[e-mail]]

(In reply to Jason Orendorff from comment #49)
> <glandium> khuey: we should probably make --enable-jemalloc and
>            --enable-debug mutually exclusive in configure.in on windows
That's overkill. The 32-bit debug CRT doesn't import free, so there's no downside to using jemalloc.

> <khuey> or fixing our python script not to replace __imp__free when it's
>         part of __imp__free_dbg
That's unhelpful, because that means the CRT will try to free stuff...

My best idea is to export _frex_dbg from mozutils.

Comment 52

[Jason Orendorff [:jorendorff]]

I tried this:
>--- a/memory/mozutils/mozutils.def.in
>+++ b/memory/mozutils/mozutils.def.in
>@@ -56,3 +56,7 @@ EXPORTS
>   ; A hack to work around the CRT (see giant comment in Makefile.in)
>+#ifdef MOZ_MEMORY_DEBUG
>+  frex_dbg=je_dumb_free_thunk
>+#else
>   frex=je_dumb_free_thunk
>+#endif
> #endif

but although this does change the output in $OBJDIR/memory/mozutils/mozutils.def, it didn't cause the build to succeed. Same error:

   Creating library mozalloc.lib and object mozalloc.exp
mozcrt.lib(crtdll_fixed.obj) : error LNK2019: unresolved external symbol __imp__
frex_dbg referenced in function _CRT_INIT
mozalloc.dll : fatal error LNK1120: 1 unresolved externals

Comment 53

[Jason Orendorff [:jorendorff]]

Er-- I don't mean to say comment 51 is wrong or anything like that, I just could use a bit of help figuring out how to implement the change it proposes.

Comment 54

[[e-mail]]

(In reply to [e-mail] from comment #51)
> (In reply to Jason Orendorff from comment #49)
> > <glandium> khuey: we should probably make --enable-jemalloc and
> >            --enable-debug mutually exclusive in configure.in on windows
> That's overkill. The 32-bit debug CRT doesn't import free, so there's no
> downside to using jemalloc.
Actually that's not quite true.
The 32-bit release CRT references free. 32-bit calling convention prefixes an underscore, and then exporting adds __imp_ resulting in __imp__free which we change to __imp__frex.
The 64-bit release CRT references free. 64-bit calling convention has no prefix, but we still get the __imp_ resulting in __imp_free which we change to __imp__frex.
The 32-bit debug CRT references _free_dbg. 32-bit calling convention prefixes an extra underscore, plus the __imp_ resulting in __imp___free_dbg which we ignore because it has too many underscores!
The 64-bit debug CRT references _free_dbg. This only gets the __imp_ treatment resulting in __imp__free_dbg which confuses us resulting in __imp__frex_dbg.
What I don't know yet is whether we actually try to free something we shouldn't; in my hacked 64-bit debug jemalloc build I do hit je_dumb_free_thunk.

Comment 55

[Mike Hommey [:glandium]]

We enabled jemalloc on Windows debug builds in bug 1014976, so this is obviously not a problem anymore.