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

VERIFIED FIXED in Firefox 16

Status

()

defect
--
critical
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: gkw, Assigned: efaust)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla18
x86_64
macOS
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

7 years ago
Posted file 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

7 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

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

Comment 3

7 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

7 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

7 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

7 years ago
Posted patch FixSplinter Review
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 9

7 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?
https://hg.mozilla.org/mozilla-central/rev/efc2630b978a
Status: ASSIGNED → RESOLVED
Closed: 7 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
Attachment #655587 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [fuzzblocker] → [fuzzblocker][advisory-tracking-]
Group: core-security
Keywords: verifymesec-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.