628334 - TM: Assertion failure: shape->hasDefaultSetter(), at ../jstracer.cpp:12207

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

--
var o = {};
o.watch("p", function() { });

for(var i=0; i<20; i++) {
    o.p = 123;
    delete o.p;
}
--
This asserts with -j:

Assertion failure: shape->hasDefaultSetter(), at ../jstracer.cpp:12207

Comment 1

[Jan de Mooij [:jandem]]

The first bad revision is:
changeset:   60418:33c58d16d911
user:        Jason Orendorff <[e-mail]>
date:        Fri Jan 14 16:18:53 2011 -0600
summary:     Bug 559653 - Record assignment before the interpreter goes, mostly. This reduces record_SetPropHit to a narrower callback, record_AddProperty. r=brendan.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Stacktrace

Comment 3

[Jason Orendorff [:jorendorff]]

Taking preliminarily, but it'll be hours before I get a chance to really look, so feel free to steal it. Looks like it might just be one of those silly "oh, i forgot watchpoints can happen" situations.

Comment 4

[David Mandelin [:dmandelin]]

For this test case, nothing happens in opt mode. I think the bug is that in this particular combo of watchpoints and tracing, a setter could get skipped. Thus, softblocker. Let me know if I missed something and it is more serious.

Comment 5

[Jason Orendorff [:jorendorff]]

Right. Actually the interpreter has a bug here too, bug 631305. Fortunately the watchpoint machinery is robust against it.

Fix coming.

Comment 6

[Jason Orendorff [:jorendorff]]

Attachment: v1

Straightforward.

Comment 7

[Brendan Eich [:brendan]]

> IsWatchedProperty(JSContext *cx, const Shape *shape)
> {
>     if (shape->hasSetterValue()) {
>         JSObject *funobj = shape->setterObject();
>         if (!funobj || !funobj->isFunction())
>             return false;
> 
>         JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj);

How about modernizing to use funobj->getFunctionPrivate() while you are here?

/be

Comment 8

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/ae1e00dc43b7

Comment 9

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/ae1e00dc43b7

Comment 10

[Christian Holler (:decoder)]

Bug in removed tracer code, setting in-testsuite- flag.