[ppc64] [ppc64le] 45.1.0 ESR build fails because class js::jit::MacroAssembler has no member named 'branch64'

RESOLVED FIXED

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jhorak, Assigned: nbp)

Tracking

45 Branch
Points:
---

Firefox Tracking Flags

(firefox45 unaffected, firefox46+ fixed, firefox47 unaffected, firefox48 unaffected, firefox-esr38 unaffected, firefox-esr45 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
We've hit following issue with Firefox 45.1.0 ESR build on ppc64 and ppc64le:

In file included from /builddir/build/BUILD/firefox-45.1.0/firefox-45.1.0esr/objdir/js/src/Unified_cpp_js_src13.cpp:2:0:
/builddir/build/BUILD/firefox-45.1.0/firefox-45.1.0esr/js/src/jit/IonCaches.cpp: In function 'void CheckDOMProxyExpandoDoesNotShadow(JSContext*, js::jit::MacroAssembler&, JSObject*, jsid, js::jit::Register, js::jit::Label*)':
/builddir/build/BUILD/firefox-45.1.0/firefox-45.1.0esr/js/src/jit/IonCaches.cpp:702:14: error: 'class js::jit::MacroAssembler' has no member named 'branch64'
         masm.branch64(Assembler::NotEqual,
              ^

It seems that it was cased by https://bugzilla.mozilla.org/show_bug.cgi?id=1246061 (this change http://hg.mozilla.org/mozilla-central/diff/967dcb05f347/js/src/jit/IonCaches.cpp#l703 ).

Firefox 45.0.2 ESR build under same conditions is fine.
(Assignee)

Comment 1

3 years ago
This is a problem in the backported patch, which lacks definition of the branch64 function in the none macro assembler.  I will make a patch in a few minutes.
(Assignee)

Updated

3 years ago
Blocks: 1246061
(Assignee)

Updated

3 years ago
status-firefox45: --- → unaffected
status-firefox46: --- → affected
status-firefox47: --- → unaffected
status-firefox48: --- → unaffected
status-firefox-esr38: --- → unaffected
status-firefox-esr45: --- → affected
Attachment #8743851 - Flags: review?(hv1989) → review+
(Assignee)

Comment 3

3 years ago
Comment on attachment 8743851 [details] [diff] [review]
Add branch64 functions to the none-backend MacroAssembler.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  Bug 1246061 caused a compilation issue on builds with disabled jit.
User impact if declined: Cannot compile.
Fix Landed on Version: N/A
Risk to taking this patch (and alternatives if risky): N/A
String or UUID changes made by this patch: N/A
Attachment #8743851 - Flags: approval-mozilla-esr45?
Attachment #8743851 - Flags: approval-mozilla-beta?
Assignee: nobody → nicolas.b.pierron
Comment on attachment 8743851 [details] [diff] [review]
Add branch64 functions to the none-backend MacroAssembler.

Taking it for 45.2.0.
It might be in 45.1.0 if we find a driver for a second build.

This missed 46 RC2, if we do a RC3, we could take it but this is Liz's call.
Attachment #8743851 - Flags: approval-mozilla-release?
Attachment #8743851 - Flags: approval-mozilla-esr45?
Attachment #8743851 - Flags: approval-mozilla-esr45+
Attachment #8743851 - Flags: approval-mozilla-beta?
Bah, sorry for missing these.  Backport was a little beastly here because of significant trunk code improvements, and as the trunk patch didn't (because of that) have none-arch changes, I didn't remember I needed to make them explicitly in backports.

I also didn't make these explicit changes in the ESR38 backport.  Is it necessary to fix that, or is ESR38 sufficiently Johnny-come-lately that the absence there won't hurt you?
(Reporter)

Comment 6

3 years ago
Thanks guys for swift reaction!
I missed this on Friday; we did an rc3 but i don't think this is in it.
tracking-firefox46: --- → +
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> I missed this on Friday; we did an rc3 but i don't think this is in it.

It's not, fwiw.
(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> This is a problem in the backported patch, which lacks definition of the
> branch64 function in the none macro assembler.  I will make a patch in a few
> minutes.

It doesn't look like the none macro assembler has a branch64 method in any branch (except esr45, now)
Flags: needinfo?(nicolas.b.pierron)
(Assignee)

Comment 11

3 years ago
(In reply to Mike Hommey [:glandium] from comment #10)
> It doesn't look like the none macro assembler has a branch64 method in any
> branch (except esr45, now)

Yes, this issue is caused by the backported patches from Bug 1246061.  Newer branches are using the generic MacroAssembler, with the "PER_ARCH" macro which replaces the need for the none macro assembler.

The status flags are correct.
Flags: needinfo?(nicolas.b.pierron)
Do we need this on 46.0.1 desktop (planned for this coming week)  
Or are you aiming it at the next esr?
Flags: needinfo?(nicolas.b.pierron)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> Do we need this on 46.0.1 desktop (planned for this coming week)  
> Or are you aiming it at the next esr?

It would be nice if it were fixed in both next esr and next point release. (is there a planned esr along 46.0.1?)
(Assignee)

Comment 14

3 years ago
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> Do we need this on 46.0.1 desktop (planned for this coming week)  
> Or are you aiming it at the next esr?

As Sylvestre mention, this is a nice to have but not enough to make a new release on its own.

So, yes, we should take it.
Flags: needinfo?(nicolas.b.pierron)
Comment on attachment 8743851 [details] [diff] [review]
Add branch64 functions to the none-backend MacroAssembler.

Approved for m-r uplift so it will go into 46.0.1. Otherwise debian/redhat users can't build firefox
Attachment #8743851 - Flags: approval-mozilla-release? → approval-mozilla-release+
Fixed by landing on the release and esr branches.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.