Closed
Bug 999358
Opened 10 years ago
Closed 10 years ago
MLambdaArrow should initialize the unused extended slot
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla31
Tracking | Status | |
---|---|---|
firefox29 | --- | unaffected |
firefox30 | --- | unaffected |
firefox31 | --- | fixed |
firefox-esr24 | --- | unaffected |
b2g-v1.2 | --- | unaffected |
b2g-v1.3 | --- | unaffected |
b2g-v1.3T | --- | unaffected |
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
(Keywords: csectype-uninitialized, regression, sec-high, Whiteboard: [qa-])
Attachments
(1 file)
3.16 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
Not initializing the second slot probably caused bug 991755 comment 13. I think we're also missing a post barrier atm, this patch fixes that too. Terrence, please double check this.
Assignee | ||
Comment 1•10 years ago
|
||
Forgot to attach the patch; see comment 0.
Attachment #8410154 -
Flags: review?(terrence)
Assignee | ||
Updated•10 years ago
|
Comment 2•10 years ago
|
||
Comment on attachment 8410154 [details] [diff] [review] Patch Review of attachment 8410154 [details] [diff] [review]: ----------------------------------------------------------------- Nice! r=me ::: js/src/jit/IonBuilder.cpp @@ +9442,5 @@ > current->add(ins); > current->push(ins); > > + if (NeedsPostBarrier(info(), thisDef)) > + current->add(MPostWriteBarrier::New(alloc(), ins, thisDef)); Actually, we shouldn't ever need a barrier here: LambdaArrow only generates a nursery allocation, so will either nursery allocate or take the ool path. If the lambda is only ever in the nursery then the |this| assignment can only create nursery->tenured edges, which we don't care about. The OOL path may allocate tenured, but it rejoins after initialization -- it will be barriered by the interpreter's normal mechanisms.
Attachment #8410154 -
Flags: review?(terrence) → review+
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Terrence Cole [:terrence] from comment #2) > Actually, we shouldn't ever need a barrier here: LambdaArrow only generates > a nursery allocation, so will either nursery allocate or take the ool path. Ah you're right. masm.newGCThing will only use the free list if initialHeap == gc::TenuredHeap, I was thinking about the nursery-disabled case but that doesn't affect this.
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9e4dac08b6e (No sec-approval, does not affect branches.)
Blocks: 988993
Updated•10 years ago
|
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b9e4dac08b6e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v1.2:
--- → unaffected
status-b2g-v1.3:
--- → unaffected
status-b2g-v1.3T:
--- → unaffected
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → fixed
status-firefox29:
--- → unaffected
status-firefox-esr24:
--- → unaffected
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•10 years ago
|
Group: javascript-core-security → core-security
Updated•10 years ago
|
Updated•10 years ago
|
tracking-firefox31:
+ → ---
Comment 6•10 years ago
|
||
Marking [qa-] due to lack of test and/or STR. Feel free to add that if you'd like QA verification, thanks.
Whiteboard: [qa-]
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•