Closed Bug 899572 Opened 6 years ago Closed 6 years ago

IonMonkey: SetElementCache shouldn't add a dense (add) stub when there are setters set

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: h4writer, Assigned: h4writer)

References

Details

Attachments

(1 file)

SetElementCache doesn't check for "setters" in the addition case. With the patch of bug 899051 applied, the following testcase fails:

var gTestcases = new Array();
var gTc = gTestcases.length;
var setterCalled = false;
function TestCase() {
    gTestcases[gTc++] = this;
}
for(var i = 0; i < 13; ++i) {
    var testcase = new TestCase();
}
Array.prototype.__defineSetter__(32, function() { setterCalled = true; });
for(var i = 0; i < 20; ++i) {
    var testcase = new TestCase();
}
assertEq(setterCalled, true);

(It should probably be possible to create a testcase without the need of bug 899051 applied)
Blocks: 774006
Blocks: 899051
Assignee: general → hv1989
Attachment #783185 - Flags: review?(nicolas.b.pierron)
Comment on attachment 783185 [details] [diff] [review]
bug899572-cache-setter

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

::: js/src/ion/IonCaches.cpp
@@ +2658,5 @@
> +    // The object may have a setter definition,
> +    // either directly, or via a prototype, or via the target object for a prototype
> +    // which is a proxy, that handles a particular integer write.
> +    // Scan the prototype and shape chain to make sure that this is not the case.
> +    RootedObject curObj(cx, obj);

I think you can get rid of the cx and use a plain JSObject pointer instead.

19:29:28 <nbp> can isNative gc?
19:29:30 <mrgiggles> No, nothing matching |isNative| can GC. Matches are: http://pastebin.mozilla.org/2745714
19:29:51 <nbp> can isIndexed gc?
19:29:51 <mrgiggles> No, |uint8 JSObject::isIndexed() const| cannot GC
19:30:17 <nbp> can getProto gc?
19:30:18 <mrgiggles> Maybe, it depends on which 'getProto' you mean: see http://pastebin.mozilla.org/2745716
19:30:44 <nbp> can JSObject::getProto() gc?
19:30:44 <mrgiggles> No, |JSObject* JSObject::getProto() const| cannot GC
Attachment #783185 - Flags: review?(nicolas.b.pierron) → review+
https://hg.mozilla.org/mozilla-central/rev/4a1e0eb4539c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.