1224642 - XPCOMBinaryComponent with msvcrt='static' links to microsoft malloc, not jemalloc

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Kent James (:rkent)]

I'm creating a binary extension (yes I know not supported in Firefox) that has a moz.build line like this:

XPCOMBinaryComponent('websrvcs', msvcrt='static')

When I do that, free() calls are getting sent to the Microsoft malloc implementation, not jemalloc. I'll document my problems, questions, and solutions here, because I'm reasonably sure that the existing m-c code is incorrect. Not clear if Mozilla will want the changes, and I can do them locally probably, but it won't hurt to document the issues I suppose.

Comment 1

[Kent James (:rkent)]

One of my questions is this: in memory/build/moz.build we have this comment: "# Keep jemalloc separated when mozglue is statically linked" How is jemalloc supposed to be added to the link in that case?

I also don't understand how the code after the comment either reflects the condition, or how it accomplishes the separation:

# Keep jemalloc separated when mozglue is statically linked
if CONFIG['MOZ_MEMORY'] and (CONFIG['OS_TARGET'] in ('WINNT', 'Darwin', 'Android') or
                             CONFIG['MOZ_NATIVE_JEMALLOC']):
    FINAL_LIBRARY = 'mozglue'


Perhaps a related question is, in mozalloc.cpp, we have the comment:

// When jemalloc is disabled, or when building the static runtime variant,
// we need not to use the suffixes.

So somehow jemalloc is supposed to be added to the library when linked statically. How is that supposed to be done?

Comment 2

[Mike Hommey [:glandium]]

(In reply to Kent James (:rkent) from comment #0)
> I'm creating a binary extension (yes I know not supported in Firefox) that
> has a moz.build line like this:
> 
> XPCOMBinaryComponent('websrvcs', msvcrt='static')

You can't use msvcrt='static' here. I guess we should error out when it's used.

Comment 3

[Kent James (:rkent)]

Feel free to close as WONTFIX. The audience for this issue is pretty tiny.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Don't allow arguments to the XPCOMBinaryComponent template

PCOM binary components imply dependent linkage, don't work with msvcrt
static linkage and have to be built against mozglue, so let's remove all
the footguns changing those add.

Comment 5

[Mike Hommey [:glandium]]

DXR says it's safe, too:
https://dxr.mozilla.org/comm-central/search?q=XPCOMBinaryComponent&redirect=false&case=true

Comment 6

[Kent James (:rkent)]

Thanks glandium for the comments. I've compiled using XPCOMBinaryComponent('websrvcs') and it now has the correct malloc routines.

But there is an additional issue that I had to work around. Not sure if you care about this or not, and I can work around it locally, but I'll post here the issue.

There are several places in the code where external libraries are created for use by external XPCOM components. One such place is /intl/unicharutil/util

moz.build there has:

# This file builds the unicharutil_external_s library which should be used
# by frozen (dependent) linkage components. 

but has:

USE_STATIC_LIBS = True

This creates the library unicharutil_external_s which is compiled with static linkage, but static linkage is no long correct since you are now requiring dynamic linkage.

For this to work with code like XPCOMBinaryComponent('websrvcs') you can't have USE_STATIC_LIBS = True, as this generates a link error complaining of mismatch between the link options and the compile options. For my purposes, I simply commented out the USE_STATIC_LIBS = True. If you have any expectation of supporting XPCOMBinaryComponents, these lines should probably be removed in the tree.

Comment 7

[Mike Hommey [:glandium]]

The fact that intl/unicharutil/util uses USE_STATIC_LIBS = True pretty much says you shouldn't be linking it into a XPCOM binary component as is. Why can't you use @mozilla.org/intl/unicharutil;1?

Comment 8

[Kent James (:rkent)]

(In reply to Mike Hommey [:glandium] from comment #7)
> The fact that intl/unicharutil/util uses USE_STATIC_LIBS = True pretty much
> says you shouldn't be linking it into a XPCOM binary component as is. Why
> can't you use @mozilla.org/intl/unicharutil;1?

For my purposes, simply removing USE_STATIC_LIBS = True, at least with a quick check, seemed to do what I needed, and is easier than what you suggest. I'll try the XPCOM approach though if I have more problems.

Does the external build of intl/unicharutil/util have any purpose other than to support XPCOM binaries (for which it no longer works)? If not, I suggest that that be changed or removed. That's why I mention it.

Comment 9

[Mike Hommey [:glandium]]

It's not used in m-c, and only used in unused parts of c-c's moz.build that wouldn't build anyways, so yes, we can probably remove it. Feel free to file a bug :)

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f20f90d8cbbb

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f20f90d8cbbb