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

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: 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.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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)

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

If the fix for bug 781855 lands on aurora, the fix for this bug should also land on aurora.

Comment 3

[Eric Faust [:efaust]]

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.

Comment 4

[Eric Faust [:efaust]]

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.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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>"

Comment 6

[Eric Faust [:efaust]]

Attachment: 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.

Comment 7

[Brian Hackett [Laid off!]]

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.

Comment 8

[Eric Faust [:efaust]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efc2630b978a

Comment 9

[Eric Faust [:efaust]]

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.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/efc2630b978a

Comment 11

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 16

[Eric Faust [:efaust]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d77906f8ceee

Comment 17

[Eric Faust [:efaust]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9c05ad62c3f6

Comment 18

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929