Closed
Bug 899572
Opened 10 years ago
Closed 10 years ago
IonMonkey: SetElementCache shouldn't add a dense (add) stub when there are setters set
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: h4writer, Assigned: h4writer)
References
Details
Attachments
(1 file)
2.26 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•10 years ago
|
||
Assignee: general → hv1989
Attachment #783185 -
Flags: review?(nicolas.b.pierron)
Comment 2•10 years ago
|
||
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+
Assignee | ||
Comment 3•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a1e0eb4539c
Comment 4•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4a1e0eb4539c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in
before you can comment on or make changes to this bug.
Description
•