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

RESOLVED FIXED in Firefox 19

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gaston, Assigned: njn)

Tracking

unspecified
mozilla20
PowerPC
OpenBSD
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17 unaffected, firefox18 unaffected, firefox19 fixed, firefox20 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
+++ 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.
(Reporter)

Updated

5 years ago
Depends on: 754641
(Reporter)

Comment 1

5 years ago
Created attachment 690083 [details] [diff] [review]
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.
Assignee: general → landry
Attachment #690083 - Flags: review?(n.nethercote)
(Reporter)

Updated

5 years ago
Depends on: 781602
(Reporter)

Comment 2

5 years ago
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.
(Reporter)

Updated

5 years ago
Duplicate of this bug: 812696
(Assignee)

Comment 4

5 years ago
Created attachment 690674 [details] [diff] [review]
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?
(Assignee)

Updated

5 years ago
Assignee: landry → n.nethercote
(Assignee)

Updated

5 years ago
Attachment #690083 - Flags: review?(n.nethercote)
(Assignee)

Updated

5 years ago
Attachment #690674 - Flags: feedback?(landry)
(Reporter)

Comment 5

5 years ago
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.
Attachment #690674 - Flags: feedback?(landry) → feedback+
(Assignee)

Updated

5 years ago
Attachment #690674 - Flags: review?(luke)

Comment 6

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

Smart!
Attachment #690674 - Flags: review?(luke) → review+
(Assignee)

Comment 7

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

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/dd214d4f8e98
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
(Reporter)

Updated

5 years ago
status-firefox17: fixed → unaffected
status-firefox18: fixed → unaffected
status-firefox19: --- → affected
status-firefox20: --- → fixed
(Reporter)

Comment 9

5 years ago
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
Attachment #690674 - Flags: approval-mozilla-aurora?
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.
Attachment #690674 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Comment 11

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/98c25377489e
status-firefox19: affected → fixed
You need to log in before you can comment on or make changes to this bug.