Closed Bug 819610 Opened 12 years ago Closed 12 years ago

IonMonkey: Differential Testing: Getting different output w/without --ion-eager with gc

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 798670
Tracking Status
firefox20 --- affected

People

(Reporter: gkw, Unassigned)

References

Details

(Keywords: regression, testcase)

x = [] y = ArrayBuffer() x.unshift(this) try { f = (function() { y.byteLength }) x.unshift(1) x.sort(f) gc() y = 0 f() print(this) } catch (e) {} shows the following output in js opt shell on IonMonkey changeset 32638e411218 without --ion-eager: [object global] but shows no output with --ion-eager. s-s because gc is on the stack. Not sure what rating this should have. autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 111211:2583a19e59ef user: Kannan Vijayan date: Tue Oct 23 22:18:11 2012 -0400 summary: Bug 795801 - IC StrictPropertyOp setters in IonMonkey. (r=dvander)
Ignore the bisection range in comment 0 - that's for bug 819611. The bisection for this one isn't useful either - it points to the entire IonMonkey landing. :( (updating flags till we know more) Due to skipped revisions, the first bad revision could be any of: changeset: 106741:6cd206b37176 parent: 106740:b63bb39ed1c0 parent: 103644:a0240c1043ee user: David Anderson date: Wed Aug 29 17:51:24 2012 -0700 summary: Merge from mozilla-central. changeset: 106742:7bf95bb09233 parent: 106741:6cd206b37176 parent: 103794:706174d31a02 user: David Anderson date: Wed Aug 29 17:57:37 2012 -0700 summary: Merge from mozilla-central. changeset: 106747:003feda8a0b3 parent: 106742:7bf95bb09233 parent: 106746:630296b1c46d user: David Anderson date: Wed Aug 29 17:58:13 2012 -0700 summary: Merge. changeset: 106748:8f2d38db4b56 user: David Anderson date: Wed Aug 29 18:04:42 2012 -0700 summary: Fix merge bustage.
I'll let other more knowledgeable people assume the rating here..
Keywords: sec-critical
This sounds like a TI issue, when we fist execute f, we don't know y values, and produce a 100% fallible typebarrier. Then we observe y, monitor it and recompile. And the recompiled verison does not have any typebarrier and likely freeze "y". My guess is that "y = 0" does not change the typeset associated to "y". This is likely due to an interaction with the GC. So we enter byteLengthGetter with a This which is not a typed array, and this result in an exception which skip the rest of the execution. The following test case is even more intriguing as we are invalidated before the gc and as we are recompiling after the gc, which highlight the TI+GC issue. y = ArrayBuffer() try { f = (function() { y.byteLength }); f() // f is called and invalidated by the type barrier. gc() y = 0 f() // f is compiled against bad type info and throw an exception. print("good"); } catch (e) { print("bad"); } As this deal with TI, I will leave this bug as s-s for now.
(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #3) > This sounds like a TI issue, when we fist execute f, we don't know y values, > and produce a 100% fallible typebarrier. Then we observe y, monitor it and > recompile. And the recompiled verison does not have any typebarrier and > likely freeze "y". ... > As this deal with TI, I will leave this bug as s-s for now. bhackett, thoughts?
Flags: needinfo?(bhackett1024)
The type information is fine, if you call cx->compartment->types.print during the second compilation of f then the GETGNAME "y" is marked as producing either an int or array buffer (it generates the type directly instead of a type barrier since it knows the exact object being accessed). The problem seems to be that we then go through getPropTryCommonGetter and generate code to call the ArrayBuffer byteLength getter with that int32 value, which is wrong. I don't know whether the appropriate fix is to guard on the input being an object, or just not do the optimization if it might not be an object. Cc'ing efaust. Since the getter is being called with a Value I think this is just a correctness issue and not s-s, but don't know the DOM stuff well enough to say for sure.
Flags: needinfo?(bhackett1024)
Same issue as bug 798670, FWIW.
Group: core-security
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.