1628719 - Wrong inherited static methods called

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Maxime]

Attachment: test.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

I have a for loop inside which I call inherited static methods on several different classes but after some iterations, the calls become mixed up and methods from wrong parent classes are being called.

In the test case file, the script loops through classes and calls the static "build" method, which is inherited from another base class. It loops through TimeLine, picks the relevant class from ClassMap, then calls build() on it, and then build() outputs the name of the base class. The TimeLine looks like [2, 1, .... , 1, 2], so ClassTwo which extends BaseTwo will be picked first, then ClassOne several times, and then it'll end with ClassTwo again.

The output should be:
2 BaseTwo
1 BaseOne (28 times)
2 BaseTwo

But Firefox shows
2 BaseTwo
1 BaseOne (28 times)
2 BaseOne

Calling run() from the console several more time eventually makes Firefox only call build() from the BaseOne class or sometimes only from BaseTwo. This only seem to happen when inside the for loop.

This started to happen in FF75.
Works fine in Chrome.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Tom S [:evilpie]]

Thank you very much for the reproducible test case!

Regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=517de066cc6f8b6823b64b694204ee423511613b&tochange=df596657bebcb96b917d75ff452316bbe8140a1a

Sadly seems like mozregression can't find individual builds for mozilla-central. This is either Bug 1378189 or Bug 1605143.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1628719 - Part 1: Move prototype modification code for cloned functions into JSObject.cpp. r=jandem!

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1628719 - Part 2: Call ReshapeForProtoMutation for cloned function prototype modification. r=jandem!

Depends on D70407

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1628719 - Part 3: Add a test case. r=jandem!

Depends on D70408

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d132daad6b22
Part 1: Move prototype modification code for cloned functions into JSObject.cpp. r=jandem
https://hg.mozilla.org/integration/autoland/rev/54ba3aee25c6
Part 2: Call ReshapeForProtoMutation for cloned function prototype modification. r=jandem
https://hg.mozilla.org/integration/autoland/rev/89d574cfc905
Part 3: Add a test case. r=jandem

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d132daad6b22
https://hg.mozilla.org/mozilla-central/rev/54ba3aee25c6
https://hg.mozilla.org/mozilla-central/rev/89d574cfc905

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:anba, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

We're basically out of Betas this cycle. Looks like this is going to have to ride 77 to release.

Comment 11

[Sebastian Müller]

Please do reconsider uplifting. A broken JavaScript engine with no feasible workaround is worth even a bugfix release (in 75!). We cannot wait for another two releases and tell users to simply ignore broken or malfunctioning web applications in the meantime.

Comment 12

[Markus Schlegel]

Our Web-Applications (mix of Es2015, ES-Modules and built with Rollup) are broken with FF 75/76 because of this Bug. We have to advice our users now to use Chrome until FF77 is officially out. No way to ship fix earlier?

Comment 13

[ernest.mueller]

Hey, we just realized today our entire Web app no longer works in Firefox (76) as well due to this issue. I'd also like to recommend this go into a bugfix release.

Comment 14

[Jason Orendorff [:jorendorff]]

We dropped the ball on this. The fix should have been backported and shipped in a point release. Now it is very likely too late for FF76, although if we do ship a FF76.0.2 point release, I'll make sure this is in it.

For a Web developer faced with this bug, the options are:

• avoid using static methods in base classes

• put a static method with a unique name on each affected subclass (to make SM see that they are all different).

These are not good workarounds. Both are absurdly burdensome. I expect Firefox will just be broken on some sites until FF76 is gone.

Comment 15

[gillespie.ethan]

Let me join the choir of "please release this as an asap bug fix". June is far too long to wait for this.

Comment 16

[Sebastian Müller]

Could someone please comment on my question (which was in the original thread), whether maybe only "real" Es6 classes are affected? Would full babeling to ES5 be a workaround? Because the talk is all about "classes" in the workaround options comment - are prototype based "classes" also affected or maybe definitely not? Thanks.

Comment 17

[André Bargull [:anba]]

Yes, only ES6 classes are affected.

Comment 18

[Sebastian Müller]

Thanks for the confirmation.
So if you can transpile your app down to ES5 (if you need to support IE, anyway), that might actually be the easiest workaround compared to the alternatives given above! Just set FF75/76 === IE 9 :-)

Comment 19

[Alin Ilea, Desktop QA]

Reproduced the initial issue in release version 76 using Windows 10.
Verified - Fixed in Beta 77.0b7 (build id: 20200515125749) and latest Nightly build 78.0a1 (build id: 20200518214824) on Windows 10 and Ubuntu 18.04. The result in Web Console is:
2 BaseTwo
1 BaseOne (28 times)
2 BaseTwo