Last Comment Bug 743129 - IonMonkey: Assertion failure: !cx->runtime->hasIonReturnOverride(), at ion/Ion.cpp:980
: IonMonkey: Assertion failure: !cx->runtime->hasIonReturnOverride(), at ion/Io...
Status: RESOLVED FIXED
[jsbugmon:update]
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Linux
: -- major (vote)
: ---
Assigned To: Kannan Vijayan [:djvj]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-04-05 18:55 PDT by Christian Holler (:decoder)
Modified: 2012-05-08 07:49 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to fix the bug. (1023 bytes, patch)
2012-05-07 15:20 PDT, Kannan Vijayan [:djvj]
dvander: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-04-05 18:55:23 PDT
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;
    }
}
Comment 1 Kannan Vijayan [:djvj] 2012-05-07 15:20:09 PDT
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.
Comment 2 David Anderson [:dvander] 2012-05-07 15:22:51 PDT
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.
Comment 3 Kannan Vijayan [:djvj] 2012-05-08 07:49:29 PDT
Pushed.

http://hg.mozilla.org/projects/ionmonkey/rev/a2c4615bc595

Note You need to log in before you can comment on or make changes to this bug.