816250 - Fix 32-bit --disable-methodjit builds after a recent jsscript.h commit

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Landry Breuil (:gaston)]

+++ This bug was initially created as a clone of Bug #786588 +++

A recent commit to jsscript changed the struct size, thus breaking (*AGAIN* :) --disable-methodjit (ie, powerpc in my case) builds :

src/mozilla-central/js/src/jsscript.h:931:1: error: size of array 'moz_static_assert96' is negative

src/mozilla-aurora/js/src/jsscript.h:931:1: error: size of array 'moz_static_assert96' is negative

m-c on the 4/11 was fine, and it's now in aurora - so it's likely a jsscript modification between the 4/11 and the last uplift that broke it.

Comment 1

[Landry Breuil (:gaston)]

Attachment: Readd non-methodjit 32bits padding to fix js on ppc

Readding the !MJ&&32bits pad32 fixes the issue for me, that patch applies to both m-c and m-a, currently building here. I've tested that this produces a working js shell both with m-c and m-a. Will report tomorrow is this also produces a working firefox for both branches.

While here, added a big scary comment to remind ppl to test --disable-methodjit on 32 bits archs when touching the JSScript struct layout. I can amend that of course.

Given that on the 4/11 i had built m-c fine, it got broken after that date. Looking at the jsscript.h hg log, i'd be tempted to blame bug 781602 which removed the 32-bit only padding and added the parallelIon struct member. That's probably the commit that changed the layout of the struct, thus breaking !JS_METHODJIT.

Comment 2

[Landry Breuil (:gaston)]

Firefox is broken at runtime (xpcshell crashes during package) on OpenBSD/ppc on all branches, but i'm pretty sure that's an unrelated issue. The build fix itself remains needed.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Make it harder to break the JSScript size constraints.

Here's an alternative approach that will be much harder to break.  The
disadvantage is that on non-JM, non-IM and non-JM-or-IM builds (which are
rare) JSScript might be a slightly bigger than necessary.

Landry, what do you think?

Comment 5

[Landry Breuil (:gaston)]

Comment on attachment 690674 [details] [diff] [review]
Make it harder to break the JSScript size constraints.

Yes, that also works for me. That's an alternative i've been considering too.... and it would make much more sense that always fiddle with padding each times the struct changes.

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 690674 [details] [diff] [review]
Make it harder to break the JSScript size constraints.

Smart!

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd214d4f8e98

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/dd214d4f8e98

Comment 9

[Landry Breuil (:gaston)]

Comment on attachment 690674 [details] [diff] [review]
Make it harder to break the JSScript size constraints.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 781602 ?
User impact if declined: build failure on non-methodjit archs (say, powerpc)
Testing completed (on m-c, etc.): in progress, working here
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

Comment 10

[bhavana bajaj [:bajaj]]

Comment on attachment 690674 [details] [diff] [review]
Make it harder to break the JSScript size constraints.

Approving on Aurora as its a low risk patch and avoids build failures on non-methodjit arch.

Comment 11

[Landry Breuil (:gaston)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/98c25377489e