"Assertion failure: [infer failure] Missing type in object [0x101f1a3a0] (index): <0x101f1d060>," with evalcx and gc

VERIFIED FIXED in Firefox 16

Status

()

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

People

(Reporter: gkw, Assigned: efaust)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla18
x86_64
Mac OS X
assertion, regression, sec-critical, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14 unaffected, firefox15 unaffected, firefox16 fixed, firefox17 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [fuzzblocker][advisory-tracking-])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 655259 [details]
stack

sandbox = newGlobal('')
evalcx("x=[]", sandbox)
evalcx("\
  x[0] = this;\
  Object.defineProperty(x, 0, {})\
", sandbox)
gc()
evalcx("x.shift()", sandbox)

asserts js debug shell on m-c changeset 29ca472bf2d2 with -m, -n and -a at Assertion failure: [infer failure] Missing type in object [0x101f1a3a0] (index): <0x101f1d060>,

s-s because type inference failures are usually bad, and gc is involved.
(Reporter)

Comment 1

5 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   103141:455ed4a415aa
user:        Eric Faust
date:        Wed Aug 22 22:05:21 2012 -0700
summary:     Bug 781855 - Fix incorrectly shadowing 'own' properties in the case of prototypal setters. (r=bhackett)
Blocks: 781855
(Reporter)

Comment 2

5 years ago
If the fix for bug 781855 lands on aurora, the fix for this bug should also land on aurora.
(Assignee)

Comment 3

5 years ago
OK, so:

The bug is that JM wasn't updated in 781855 to ensure that the property was an own property after an array setelem. So the (index) property is not marked as own. Then, at the gc(), the type information for that property is discarded and not rebuilt because it's not an own property, leading to the missing information at the next get.
(Assignee)

Comment 4

5 years ago
The way it worked before was that during analysis, when we saw the setelem, we would generate a PropertyAccess, which would call TypeObject::getProperty(...,assign = true), which /used/ to ensure that the property was marked as own. This seems the most plausible road to fixing the problem.
Assignee: general → efaustbmo
Status: NEW → ASSIGNED
(Reporter)

Comment 5

5 years ago
Setting [fuzzblocker] because the addresses in the inference failure messages change with different testcases, but it's always "Assertion failure: [infer failure] Missing type in object [<some address>] (index): <some address>"
Whiteboard: [fuzzblocker]
(Assignee)

Comment 6

5 years ago
Created attachment 655587 [details] [diff] [review]
Fix

Always mark void jsid property writes as own, to avoid mismarking own-ness of array properties. This should also resolve bugs 785749, 785824, and 785835.
Attachment #655587 - Flags: review?(bhackett1024)
Comment on attachment 655587 [details] [diff] [review]
Fix

Review of attachment 655587 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsinfer.cpp
@@ +1090,5 @@
>  
> +    /*
> +     * Capture the effects of a standard property access. Only mark void jsid
> +     * property as own, as it may have inherited accessors, but array accesses
> +     * are always own properties.

This needs a beefier comment.  How about:

Capture the effects of a standard property access.  For assignments, we do not automatically update the 'own' bit on accessed properties, except for indexed elements in dense arrays.  The latter exception allows for JIT fast paths to avoid testing the array's type when assigning to dense array elements.
Attachment #655587 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/efc2630b978a
(Assignee)

Comment 9

5 years ago
Comment on attachment 655587 [details] [diff] [review]
Fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 781855
User impact if declined: Sec-critical assertion (exploitable)
Testing completed (on m-c, etc.): Tested on tests in this bug as well as bugs 785749, 785824, and 789835. 
Risk to taking this patch (and alternatives if risky): Very Minor. Creates more restrictive type information. 
String or UUID changes made by this patch: None

To apply after the central->aurora uplift.
Attachment #655587 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
status-firefox14: --- → unaffected
status-firefox15: --- → unaffected
status-firefox16: --- → affected
status-firefox17: --- → affected
https://hg.mozilla.org/mozilla-central/rev/efc2630b978a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Duplicate of this bug: 785749
Duplicate of this bug: 785824
Duplicate of this bug: 785835
Duplicate of this bug: 785736
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/d77906f8ceee
status-firefox16: affected → fixed
Attachment #655587 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/9c05ad62c3f6
status-firefox17: affected → fixed

Updated

5 years ago
status-firefox-esr10: --- → unaffected
Keywords: verifyme
Whiteboard: [fuzzblocker] → [fuzzblocker][advisory-tracking-]
Group: core-security
Keywords: verifyme → sec-critical
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.