Last Comment Bug 709067 - JS Correctness: Another issue with/without TI and defineProperty/prototype
: JS Correctness: Another issue with/without TI and defineProperty/prototype
[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)
: Jason Orendorff [:jorendorff]
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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image 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(",");

$ $JS -m -a min.js

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

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

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 User image Brian Hackett (:bhackett) 2011-12-15 08:53:45 PST
Comment 3 User image David Mandelin [:dmandelin] 2011-12-15 17:09:30 PST
Brian, do you think we should nominate this for 10?
Comment 4 User image Brian Hackett (:bhackett) 2011-12-15 17:26:34 PST
Comment on attachment 580459 [details] [diff] [review]

Comment 5 User image Ed Morley [:emorley] 2011-12-16 06:14:46 PST
Comment 6 User image 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 User image 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 User image christian 2011-12-19 11:53:51 PST
Comment on attachment 580459 [details] [diff] [review]

Approved for mozilla-aurora. Please land today.
Comment 9 User image Brian Hackett (:bhackett) 2011-12-19 12:26:48 PST
Comment 10 User image 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.