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

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rkent, Assigned: glandium)

Tracking

Trunk
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

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.
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?
(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.
Feel free to close as WONTFIX. The audience for this issue is pretty tiny.
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.
Assignee: nobody → mh+mozilla
Attachment #8688170 - Flags: review?(gps)
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.
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?
(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.
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 :)
Attachment #8688170 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/f20f90d8cbbb
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.