JS Correctness: Another issue with/without TI and defineProperty/prototype

VERIFIED FIXED in Firefox 10

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: decoder, Assigned: bhackett)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla10
x86_64
Linux
testcase
Points:
---

Firefox Tracking Flags

(firefox9+, firefox10+ fixed, firefox11+ verified)

Details

(Whiteboard: [qa!] js-triage-needed)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
tracking-firefox10: --- → +
tracking-firefox11: --- → +
tracking-firefox9: --- → +
(Assignee)

Comment 1

6 years ago
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.
Assignee: general → bhackett1024
Attachment #580459 - Flags: review?(luke)

Updated

6 years ago
Attachment #580459 - Flags: review?(luke) → review+
(Assignee)

Comment 2

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/35d59a6e829d
Brian, do you think we should nominate this for 10?
(Assignee)

Comment 4

6 years ago
Comment on attachment 580459 [details] [diff] [review]
patch

Sure.
Attachment #580459 - Flags: approval-mozilla-aurora?

Comment 5

6 years ago
https://hg.mozilla.org/mozilla-central/rev/35d59a6e829d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Comment 6

6 years ago
(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!
(Assignee)

Comment 7

6 years ago
The problem here is similar to bug 706808, with a similar (small) risk.

Comment 8

6 years ago
Comment on attachment 580459 [details] [diff] [review]
patch

Approved for mozilla-aurora. Please land today.
Attachment #580459 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 9

6 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/2bd5ab8de0f0
Target Milestone: mozilla11 → mozilla10

Updated

6 years ago
status-firefox10: --- → fixed
status-firefox11: --- → fixed
Whiteboard: js-triage-needed → [qa+] js-triage-needed
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.
Status: RESOLVED → VERIFIED
status-firefox11: fixed → verified
Whiteboard: [qa+] js-triage-needed → [qa!] js-triage-needed
You need to log in before you can comment on or make changes to this bug.