Last Comment Bug 1146267 - jemalloc breaks shell builds on OS X
: jemalloc breaks shell builds on OS X
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All Mac OS X
-- normal (vote)
: mozilla39
Assigned To: Jim Blandy :jimb
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 1134039
  Show dependency treegraph
 
Reported: 2015-03-23 02:40 PDT by Jan de Mooij [:jandem]
Modified: 2015-03-24 08:46 PDT (History)
7 users (show)
jimb: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Define XP_DARWIN in JS_STANDALONE builds. (681 bytes, patch)
2015-03-23 10:43 PDT, Jim Blandy :jimb
sphink: review+
jdemooij: feedback+
Details | Diff | Splinter Review

Description User image Jan de Mooij [:jandem] 2015-03-23 02:40:31 PDT
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.
Comment 1 User image Hannes Verschore [:h4writer] 2015-03-23 02:51:32 PDT
This also breaks the AreWeFastYet mac slaves. I had to add "--disable-jemalloc" to get them to build again!
Comment 2 User image Jan de Mooij [:jandem] 2015-03-23 02:56:12 PDT
It'd also be nice to have OS X shell builds on Treeherder, in addition to the Linux and Windows builds.
Comment 3 User image Steve Fink [:sfink] [:s:] 2015-03-23 09:39:24 PDT
(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.
Comment 4 User image Jan de Mooij [:jandem] 2015-03-23 09:41:50 PDT
(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.
Comment 5 User image Jim Blandy :jimb 2015-03-23 10:38:05 PDT
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?
Comment 6 User image Jim Blandy :jimb 2015-03-23 10:43:26 PDT
Created attachment 8581773 [details] [diff] [review]
Define XP_DARWIN in JS_STANDALONE builds.

Untested, just a theory. Posting for jandem to try out.
Comment 7 User image Jan de Mooij [:jandem] 2015-03-23 11:09:03 PDT
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/
Comment 9 User image Ryan VanderMeulen [:RyanVM] 2015-03-24 08:46:03 PDT
https://hg.mozilla.org/mozilla-central/rev/8e5d8f34c53e

Note You need to log in before you can comment on or make changes to this bug.