887544 - Differential Testing: Different output message w/without --no-baseline involving eval

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

print(eval("for(let x = 0; x < 9; ++x) { true }"))

shows the following on a 64-bit deterministic threadsafe js shell on m-c rev 61c3c8b85563:

undefined

but shows the following with --no-baseline:

true


This seems to be an old bug. Setting needinfo for jandem as this may cloud other correctness bugs.

Comment 2

[Jan de Mooij [:jandem]]

Nice catch, when we OSR from the interpreter into baseline we did not copy the frame's return value. I fixed this in bug 866878, but didn't know it was also observable without try-finally. That means we only have to fix this on beta.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch for beta

This is from the patch I landed for bug 866878, so setting r+

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 768258 [details] [diff] [review]
Patch for beta

[Approval Request Comment]
> Bug caused by (feature/regressing bug #): 
Baseline compiler (23)

> User impact if declined:
Correctness bugs

> Testing completed (on m-c, etc.): 
On m-c and aurora for a while

> Risk to taking this patch (and alternatives if risky):
Very low risk.

> String or IDL/UUID changes made by this patch:
None

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> That means we only have to fix this on beta.

I'm not sure if this is entirely correct. I had reproduced the issue on m-c rev 61c3c8b85563, which means aurora and m-c are affected.

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #5)
> I'm not sure if this is entirely correct. I had reproduced the issue on m-c
> rev 61c3c8b85563, which means aurora and m-c are affected.

Rev 61c3c8b85563 is from June 21, bug 866878 landed June 22.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> Rev 61c3c8b85563 is from June 21, bug 866878 landed June 22.

d'oh! You're right, then. :)

Comment 8

[bhavana bajaj [:bajaj]]

Comment on attachment 768258 [details] [diff] [review]
Patch for beta

Low risk, looks ok to land on beta since we still have a few more to go.

Comment 9

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c4e9afa543f1