jemalloc breaks shell builds on OS X

RESOLVED FIXED in Firefox 39

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jandem, Assigned: jimb)

Tracking

unspecified
mozilla39
All
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
I'm unable to build the shell on OS X, see the error below. This is a regression from bug 1134039.

In jemalloc.c I see:

  /* This was added by Mozilla for use by SQLite. */  
  #if defined(MOZ_MEMORY_DARWIN) && !defined(MOZ_REPLACE_MALLOC)
  static
  #else
  MOZ_MEMORY_API
  #endif
  size_t
  malloc_good_size_impl(size_t size)

The defined(MOZ_MEMORY_DARWIN) check probably explains why this only fails on OS X.

inbound/memory/mozjemalloc/jemalloc.c:6479:1: error: static declaration of 'malloc_good_size' follows non-static declaration
malloc_good_size_impl(size_t size)
^
inbound/memory/build/mozmemory_wrap.h:196:53: note: expanded from macro 'malloc_good_size_impl'
#define malloc_good_size_impl    mozmem_malloc_impl(malloc_good_size)
                                                    ^
inbound/memory/build/mozmemory_wrap.h:177:35: note: expanded from macro 'mozmem_malloc_impl'
#  define mozmem_malloc_impl(a)   a
                                  ^
/usr/include/malloc/malloc.h:101:15: note: previous declaration is here
extern size_t malloc_good_size(size_t size);
              ^
1 warning and 1 error generated.
(Reporter)

Updated

2 years ago
Flags: needinfo?(mh+mozilla)
(Reporter)

Updated

2 years ago
OS: All → Mac OS X
This also breaks the AreWeFastYet mac slaves. I had to add "--disable-jemalloc" to get them to build again!
(Reporter)

Comment 2

2 years ago
It'd also be nice to have OS X shell builds on Treeherder, in addition to the Linux and Windows builds.
(In reply to Jan de Mooij [:jandem] from comment #2)
> It'd also be nice to have OS X shell builds on Treeherder, in addition to
> the Linux and Windows builds.

Yes, especially if you're on an osx box and trying to reproduce a shell failure. At the moment, autospider.sh is doing something stupid on osx. I haven't had a chance to look at it. (I need to dig out my osx box and hook it up to try anything.)

If anyone else can hack in the changes necessary to get autospider.sh to do a successful osx build, I can add in osx shell tests to TH.
(Reporter)

Comment 4

2 years ago
(In reply to Steve Fink [:sfink, :s:] from comment #3)
> If anyone else can hack in the changes necessary to get autospider.sh to do
> a successful osx build, I can add in osx shell tests to TH.

Cool, I can look at this as soon as this jemalloc issue is fixed.
(Assignee)

Comment 5

2 years ago
Well, this sucks.

From reading the comments in mozmemory_wrap.h, it seems like malloc_usable_size_impl ought to be #defined as je_malloc_usable_size, but it's not getting the prefix. Perhaps the problem is that XP_DARWIN isn't #defined when we #include mozmemory_wrap.h from jemalloc.c?
(Assignee)

Comment 6

2 years ago
Created attachment 8581773 [details] [diff] [review]
Define XP_DARWIN in JS_STANDALONE builds.

Untested, just a theory. Posting for jandem to try out.
(Reporter)

Comment 7

2 years ago
Comment on attachment 8581773 [details] [diff] [review]
Define XP_DARWIN in JS_STANDALONE builds.

Review of attachment 8581773 [details] [diff] [review]:
-----------------------------------------------------------------

This fixes the build for me \o/
Attachment #8581773 - Flags: feedback+
(Assignee)

Updated

2 years ago
Attachment #8581773 - Flags: review?(sphink)
Attachment #8581773 - Flags: review?(sphink) → review+
(Assignee)

Comment 8

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e5d8f34c53e
(Assignee)

Updated

2 years ago
Flags: needinfo?(mh+mozilla)
(Assignee)

Updated

2 years ago
Assignee: nobody → jimb
Flags: in-testsuite-
Target Milestone: --- → mozilla39
https://hg.mozilla.org/mozilla-central/rev/8e5d8f34c53e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.