Closed Bug 792166 Opened 7 years ago Closed 7 years ago

IonMonkey: Assertion failure: !isOwn, at ion/IonBuilder.cpp:5509

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: decoder, Assigned: efaust)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update][ion:p1:fx18])

Attachments

(1 file)

The following testcase asserts on mozilla-central revision e4757379b99a (run with --ion-eager):


Object.defineProperty(Object.prototype, 'x', { 
    set: function() {} 
});
var obj = {};
for (var i = 0; i < 100 ; ++i) {
    obj.x = 1;
    delete obj.x;
}
Blocks: IonFuzz
Whiteboard: [jsbugmon:update]
Summary: Assertion failure: !isOwn, at ion/IonBuilder.cpp:5509 → IonMonkey: Assertion failure: !isOwn, at ion/IonBuilder.cpp:5509
This is me. Looking into it.
Assignee: general → efaustbmo
Status: NEW → ASSIGNED
deleting the property calls AddTypePropertyId(..., Unknown), which marks the typeset as own, which is wrong in the case of an inherited accessor.
In general, I think I am most interested in this case:

Object.defineProperty(Object.prototype, 'x', { 
    set: function() {print("got here");} 
});
var obj = {}; 
Object.defineProperty(obj, 'x', { 
    set : function() {print("got here instead");},
    configurable : true
});
for (var i = 0; i < 100 ; ++i) {
    obj.x = 1;
    // After the first deletion, we should be able to optimize to the function
    // on Object.prototype again right?
    delete obj.x;
}

TI should be able to keep track of what's actually going on in the face of deletions in the presence of inherited accessors.
Eric, is this something we need to fix for Firefox 18?
Yeah, it just doesn't work in the face of |delete|. What's worse is that if affects correctness in optimized builds. I will generate a patch early tomorrow morning and put it up for review. Who should even review it? You?
Fixed to cover both this and the Object.watch case.
Attachment #663425 - Flags: review?(kvijayan)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][ion:p1:fx18]
Here's another testcase for you to land:

try {
  __defineGetter__("eval", function() {
	this["__proto__"]
  })
  delete this["__proto__"]
  this["__proto__"]
} catch (e) {}
eval

also asserts js shell on m-c changeset f0c89cebf913 with --ion-eager.
Blocks: 349611
Keywords: regression
OS: Linux → All
Hardware: x86 → All
Comment on attachment 663425 [details] [diff] [review]
Fix: Once and for all.

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

Looks good.

::: js/src/ion/IonBuilder.cpp
@@ +5454,2 @@
>  
>          // Check here to make sure that everyone has Type Objects which known

nit: I assume you meant ".. Type Objects with known properties ..", not "which known properties".
Attachment #663425 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/298ef4e74849
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug792166-1.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.