Last Comment Bug 709067 - JS Correctness: Another issue with/without TI and defineProperty/prototype
: JS Correctness: Another issue with/without TI and defineProperty/prototype
Status: VERIFIED FIXED
[qa!] js-triage-needed
: testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla10
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2011-12-09 07:06 PST by Christian Holler (:decoder)
Modified: 2012-03-07 01:53 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+
fixed
+
verified


Attachments
patch (1.94 KB, patch)
2011-12-09 10:29 PST, Brian Hackett (:bhackett)
luke: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2011-12-09 07:06:21 PST
The following test produces different output with options "-m -a" vs. "-m -a -n" on mozilla-central revision 63bff373cb94:


Object.defineProperty(Object.prototype, 0, {set: function() { this.abstract; }});
function testStringify() {
    var t = true;
    var a = [];
    a[0] = "" + t;
    return a.join(",");
}
print(testStringify());


Output:
$ $JS -m -a min.js

$ $JS -m -a -n min.js
true


The test looks like bug 706808 but that is fixed already.
Comment 1 Brian Hackett (:bhackett) 2011-12-09 10:29:03 PST
Created attachment 580459 [details] [diff] [review]
patch

Similar issue to bug 706808, but affecting indexed array properties --- setters were only getting called when directly on Array.prototype, and non-writable prototype properties were getting ignored entirely.
Comment 2 Brian Hackett (:bhackett) 2011-12-15 08:53:45 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/35d59a6e829d
Comment 3 David Mandelin [:dmandelin] 2011-12-15 17:09:30 PST
Brian, do you think we should nominate this for 10?
Comment 4 Brian Hackett (:bhackett) 2011-12-15 17:26:34 PST
Comment on attachment 580459 [details] [diff] [review]
patch

Sure.
Comment 5 Ed Morley [:emorley] 2011-12-16 06:14:46 PST
https://hg.mozilla.org/mozilla-central/rev/35d59a6e829d
Comment 6 Alex Keybl [:akeybl] 2011-12-16 12:55:51 PST
(In reply to Brian Hackett (:bhackett) from comment #4)
> Comment on attachment 580459 [details] [diff] [review]
> patch
> 
> Sure.

[Triage Comment]
Can you help us understand the risk involved with this patch? Thanks!
Comment 7 Brian Hackett (:bhackett) 2011-12-16 13:23:09 PST
The problem here is similar to bug 706808, with a similar (small) risk.
Comment 8 christian 2011-12-19 11:53:51 PST
Comment on attachment 580459 [details] [diff] [review]
patch

Approved for mozilla-aurora. Please land today.
Comment 9 Brian Hackett (:bhackett) 2011-12-19 12:26:48 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/2bd5ab8de0f0
Comment 10 Mihaela Velimiroviciu (:mihaelav) 2012-03-07 01:53:54 PST
Ubuntu 11.04 64 bit

I built Spidermonkey for the latest beta (rev d46a4577a631) and run the test from comment #0: same output is produced in both cases (-m -a -n and -m -a):
./js -m -a -n test709067 


./js -m -a test709067 


Marking verified for Firefox 11.

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