The default bug view has changed. See this FAQ.

libffi doesn't allocate executable trampoline buffers in OSX 10.7

RESOLVED FIXED in mozilla10

Status

()

Core
js-ctypes
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

This is causing crashes on my lion dev machine. Patch coming right up.
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.
Attachment #556071 - Flags: review?(dwitte)
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.
Attachment #556071 - Flags: review?(khuey)

Comment 3

6 years ago
Awesome. I'll look at this next week. If I don't, poke me :)
What is $target on your machine?
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #4)
> What is $target on your machine?

x86_64-apple-darwin11.1.0
Blocks: 682504
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.
Attachment #556071 - Flags: review?(khuey) → review+
(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
Assignee: nobody → bobbyholley+bmo
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.
Attachment #556071 - Flags: review?(dwitte)
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.
Attachment #556071 - Attachment is obsolete: true
Attachment #565304 - Flags: review?(khuey)
Why don't we just make this '*-apple-darwin*' ?
(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.
Though really, I'm fine with either.
Ok ... I'll r+ *-apple-darwin1*, even though this seems a bit silly.
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.
Attachment #565304 - Attachment is obsolete: true
Attachment #565304 - Flags: review?(khuey)
Attachment #565318 - Flags: review+
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
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/def68a106743
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
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
Attachment #653179 - Flags: review?(mh+mozilla)
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
Attachment #653179 - Flags: review?(mh+mozilla) → review-
Comment on attachment 653179 [details] [diff] [review]
properly regen configure

Discard, superseded by bug #783950
You need to log in before you can comment on or make changes to this bug.