Last Comment Bug 682180 - libffi doesn't allocate executable trampoline buffers in OSX 10.7
: libffi doesn't allocate executable trampoline buffers in OSX 10.7
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: js-ctypes (show other bugs)
: unspecified
: x86 Mac OS X
-- normal (vote)
: mozilla10
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 682504
  Show dependency treegraph
 
Reported: 2011-08-25 18:48 PDT by Bobby Holley (:bholley) (busy with Stylo)
Modified: 2012-08-28 14:10 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Allocate executable trampoline buffers in darwin 10 and beyond. v1 (4.99 KB, patch)
2011-08-26 10:48 PDT, Bobby Holley (:bholley) (busy with Stylo)
khuey: review+
Details | Diff | Splinter Review
Allocate executable trampoline buffers in darwin 10 and beyond. v2 (5.13 KB, patch)
2011-10-06 12:09 PDT, Bobby Holley (:bholley) (busy with Stylo)
no flags Details | Diff | Splinter Review
Allocate executable trampoline buffers in darwin 10 and beyond. v3 (4.47 KB, patch)
2011-10-06 12:45 PDT, Bobby Holley (:bholley) (busy with Stylo)
bobbyholley: review+
Details | Diff | Splinter Review
properly regen configure (2.36 KB, patch)
2012-08-19 06:49 PDT, Landry Breuil (:gaston)
mh+mozilla: review-
Details | Diff | Splinter Review

Description User image Bobby Holley (:bholley) (busy with Stylo) 2011-08-25 18:48:04 PDT
This is causing crashes on my lion dev machine. Patch coming right up.
Comment 1 User image Bobby Holley (:bholley) (busy with Stylo) 2011-08-26 10:48:10 PDT
Created attachment 556071 [details] [diff] [review]
Allocate executable trampoline buffers in darwin 10 and beyond. v1

Added a patch for this.

I also fixed up the patch management a little bit, so that patches between our libffi and the vanilla libffi can be serially applied rather than as one megapatch. This should make it easier to upstream changes and prune changes that have been upstreamed. They can still be applied as one by simply cat-ing patches-libffi/* and piping the output to patch.

I also found a sneaky change in our libffi tree that wasn't reflected in libffi.patch. I included it in the new serial patch directory.

Flagging dwitte for review.
Comment 2 User image Bobby Holley (:bholley) (busy with Stylo) 2011-08-26 10:49:36 PDT
Comment on attachment 556071 [details] [diff] [review]
Allocate executable trampoline buffers in darwin 10 and beyond. v1

Also flagging khuey for review of the build stuff.
Comment 3 User image dwitte@gmail.com 2011-08-26 13:03:08 PDT
Awesome. I'll look at this next week. If I don't, poke me :)
Comment 4 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-26 18:48:14 PDT
What is $target on your machine?
Comment 5 User image Bobby Holley (:bholley) (busy with Stylo) 2011-08-26 19:42:01 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #4)
> What is $target on your machine?

x86_64-apple-darwin11.1.0
Comment 6 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-27 03:09:40 PDT
Comment on attachment 556071 [details] [diff] [review]
Allocate executable trampoline buffers in darwin 10 and beyond. v1

I don't pretend to understand why it's 11, but ok.
Comment 7 User image Bobby Holley (:bholley) (busy with Stylo) 2011-08-27 09:32:46 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> I don't pretend to understand why it's 11, but ok.

The darwin kernel has a major version bump every major release of MacOS:

http://en.wikipedia.org/wiki/Darwin_%28operating_system%29#Release_history
Comment 8 User image Bobby Holley (:bholley) (busy with Stylo) 2011-10-05 10:00:24 PDT
Comment on attachment 556071 [details] [diff] [review]
Allocate executable trampoline buffers in darwin 10 and beyond. v1

Dwitte and I talked about this extensively over gchat some months back, so I think khuey's review should be fine here.
Comment 9 User image Bobby Holley (:bholley) (busy with Stylo) 2011-10-06 12:09:32 PDT
Created attachment 565304 [details] [diff] [review]
Allocate executable trampoline buffers in darwin 10 and beyond. v2

So I got a couple of scattered oranges when pushing this to try.

It turns out that, depending on the builder, it might be of the form -apple-darwin-10, or of the form -apple-darwin-10.x.x.

Attaching a fix - flagging khuey for re-review. As soon as that happens and my try run completes, this whole stack of things can land.
Comment 10 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-06 12:10:36 PDT
Why don't we just make this '*-apple-darwin*' ?
Comment 11 User image Bobby Holley (:bholley) (busy with Stylo) 2011-10-06 12:12:38 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #10)
> Why don't we just make this '*-apple-darwin*' ?

There's clearly some reason why libffi doesn't want to just use the alternate malloc path for everything. Maybe performance (perceived, or otherwise?) Either way, I want to alter behavior as little as possible to increase the chances of this getting accepted upstream.
Comment 12 User image Bobby Holley (:bholley) (busy with Stylo) 2011-10-06 12:13:51 PDT
Though really, I'm fine with either.
Comment 13 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-06 12:17:18 PDT
Ok ... I'll r+ *-apple-darwin1*, even though this seems a bit silly.
Comment 14 User image Bobby Holley (:bholley) (busy with Stylo) 2011-10-06 12:45:41 PDT
Created attachment 565318 [details] [diff] [review]
Allocate executable trampoline buffers in darwin 10 and beyond. v3

Ok, sure. Attaching a patch and carrying over review.

I've pushed this to try here: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=e71f6e3a3771 - once that goes green, the ctype stuff can land.
Comment 15 User image Bobby Holley (:bholley) (busy with Stylo) 2011-10-07 10:54:44 PDT
Pushed to mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/def68a106743

This had a green try run, modulo known oranges: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=e71f6e3a3771
Comment 16 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-09 07:25:04 PDT
https://hg.mozilla.org/mozilla-central/rev/def68a106743
Comment 17 User image Landry Breuil (:gaston) 2012-08-19 06:49:13 PDT
Created attachment 653179 [details] [diff] [review]
properly regen configure

Let's properly regen configure to include the comment (and also massage the @@ lines so it applies without offsets), see bug 670719 comment 52
Comment 18 User image Mike Hommey [:glandium] 2012-08-19 23:18:11 PDT
Comment on attachment 653179 [details] [diff] [review]
properly regen configure

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

Let's do that in a separate bug
Comment 19 User image Landry Breuil (:gaston) 2012-08-28 14:10:39 PDT
Comment on attachment 653179 [details] [diff] [review]
properly regen configure

Discard, superseded by bug #783950

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