The default bug view has changed. See this FAQ.

IonMonkey: Assertion failure: !cx->runtime->hasIonReturnOverride(), at ion/Ion.cpp:980

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: decoder, Assigned: djvj)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Other Branch
x86
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
The following testcase asserts on ionmonkey revision a9a18824b4c1 (run with --ion -n -m):


var counter = 0;
var p = Proxy.create({
    has : function(id) {
	    ++counter;
	    if (counter == (12)) {
		obj.__proto__ = null;
	    return true;
	}
    },
    get : function(id) {
	if (id == 'xyz')
	    return 10;
    }
});
    obj = { xyz: 1};
    for (var i = 0; i != 100; ++i) {
	var s = obj.xyz;
	if (i == 10) {
	    delete obj.xyz;
	    Object.prototype.__proto__ = p;
    }
}
(Assignee)

Updated

5 years ago
Assignee: general → kvijayan
(Assignee)

Comment 1

5 years ago
Created attachment 621757 [details] [diff] [review]
Patch to fix the bug.

The issue here is that if a callVM call ends up invalidating the script (and setting ionReturnOverride on the runtime), and then returns false causing the engine to jump out to the interpreter (bypassing bailouts), then the ionReturnOverride is left dangling and uncleared.

In this case, this happens because the line "var s = obj.xyz", which (on iteration 20 of the loop) triggers a GetPropertyCache which resorts to JSObject::GetGeneric and eventually ends up invoking the "has" method on the handler for the proxy assigned to 'p'.  This resets the object's prototype, invalidating the script.  The GetGeneric subsequently calls the proxy handler's "get" method, which fails, causing the GetGeneric to return false, causing the GetPropertyCache to return false, causing an escape to the interpreter bypassing the bailouts.

Fix is to make HandleException clear the ionReturnOverride.
Attachment #621757 - Flags: review?(dvander)
Comment on attachment 621757 [details] [diff] [review]
Patch to fix the bug.

Review of attachment 621757 [details] [diff] [review]:
-----------------------------------------------------------------

Good bug - I suspect this would have been pretty serious.

::: js/src/ion/IonFrames.cpp
@@ +354,5 @@
> +    if (cx->runtime->hasIonReturnOverride()) {
> +        IonSpew(IonSpew_Invalidate, "HandleException: clearing dangling return override.");
> +        cx->runtime->takeIonReturnOverride();
> +    }
> +

Probably don't need the spew.
Attachment #621757 - Flags: review?(dvander) → review+
(Assignee)

Comment 3

5 years ago
Pushed.

http://hg.mozilla.org/projects/ionmonkey/rev/a2c4615bc595
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.