928381 - ctypes/libffi: Fix stack alignment on *BSD/i386

Status: RESOLVED FIXED

(Core :: js-ctypes, defect)

Description

[Landry Breuil (:gaston)]

Context: http://sourceware.org/ml/libffi-discuss/2013/msg00127.html

This is a patch we've been carrying in the openbsd portstree since a while for all copies of libffi. i'd like to properly push it back to the copy we have within ctypes..

Comment 1

[Landry Breuil (:gaston)]

Jan, any hindsight on this or objections ?

Comment 2

[Bobby Holley (:bholley)]

Have these patches (this and bug 928390) been applied upstream?

Comment 3

[Landry Breuil (:gaston)]

(In reply to Bobby Holley (:bholley) from comment #2)
> Have these patches (this and bug 928390) been applied upstream?

the patch from this bug is just pending on the libffi-discuss mailing list, but was written by one of our binutils/toolchain hacker. The patches from the other bug have been locally applied since a while, and i think they were supposed to be pushed back at some point (but i'm not the one taking care of this)

Comment 4

[Bobby Holley (:bholley)]

I don't think it makes sense for Mozilla to act as a middleman for downstream patches. I think we should get them fully applied upstream and then upgrade our in-tree libffi to something more recent (see bug 810631).

Comment 5

[Landry Breuil (:gaston)]

Fair point, but there are already a bunch of patches that were applied this way which didnt make it upstream (yet?) iirc, because they dont really seem to care (see https://github.com/atgreen/libffi/issues/42)

Comment 6

[Bobby Holley (:bholley)]

(In reply to Landry Breuil (:gaston) from comment #5)
> Fair point, but there are already a bunch of patches that were applied this
> way which didnt make it upstream (yet?) iirc

Indeed. No question that Mozilla has been lazy on this front. I'm just saying that, at the point when something is being upstreamed, it should go to the source (largely because I don't know enough about libffi to properly review patches to it). Once they're there, I'm happy to either try to upgrade our in-tree libffi, or failing that, add the patch to our local set.

> because they dont really seem
> to care (see https://github.com/atgreen/libffi/issues/42)

Does "they" refer to the libffi folks, or the Mozilla folks?

Comment 7

[Landry Breuil (:gaston)]

(In reply to Bobby Holley (:bholley) from comment #6)
> (In reply to Landry Breuil (:gaston) from comment #5)
> > Fair point, but there are already a bunch of patches that were applied this
> > way which didnt make it upstream (yet?) iirc
> 
> Indeed. No question that Mozilla has been lazy on this front. I'm just
> saying that, at the point when something is being upstreamed, it should go
> to the source (largely because I don't know enough about libffi to properly
> review patches to it). Once they're there, I'm happy to either try to
> upgrade our in-tree libffi, or failing that, add the patch to our local set.

I dont know enough of libffi either, it's just that i'm trying to get rid of patches that:
- i apply locally anyway
- only affect openbsd
 
> > because they dont really seem
> > to care (see https://github.com/atgreen/libffi/issues/42)
> 
> Does "they" refer to the libffi folks, or the Mozilla folks?

I consider myself part of "the Mozilla folks" so would have rather used "we" in that case - that was referring to libffi upstream.

Comment 8

[Bobby Holley (:bholley)]

OK. Shifting these patches openbsd into mozilla-central seems kind of a like a wash in terms of global entropy. I think the way forward for anyone willing to expend energy to improve the situation is to press for the inclusion of any downstream patches in libffi proper (doing so with Mozilla's local libffi patches as well would be even more awesome). At that point, we can cleanly upgrade, and nobody needs to keep local patches around.

CCing glandium, since he seems to be the one who patch libffi most recently.

Comment 9

[Landry Breuil (:gaston)]

Attachment: ctypes/libffi: Fix stack alignment on *BSD/i386

I made you a patch 2 weeks ago but i eated it^W^Wforgot to ask for review...

Comment 10

[Mike Hommey [:glandium]]

(In reply to Bobby Holley (:bholley) from comment #8)
> OK. Shifting these patches openbsd into mozilla-central seems kind of a like
> a wash in terms of global entropy. I think the way forward for anyone
> willing to expend energy to improve the situation is to press for the
> inclusion of any downstream patches in libffi proper (doing so with
> Mozilla's local libffi patches as well would be even more awesome). At that
> point, we can cleanly upgrade, and nobody needs to keep local patches around.

It's kind of a chicken and egg problem. We can't really push all of our patches upstream because they conflict with newer libffi, and we can't upgrade to a newer libffi because our patches don't apply. RyanVM has been trying to get a new libffi working for us, but it breaks windows. So until we actually upgrade libffi, we can't really push much upstream. That being said there is also a number of patches that we took from upstream.

Comment 11

[Landry Breuil (:gaston)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/96420ca47200

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/96420ca47200

Comment 13

[Landry Breuil (:gaston)]

And just for the record, this got commited upstream too in https://github.com/atgreen/libffi/commit/becd754434173032f426d22ffcbfe24f55b3c137