737251 - Crash [@ JS_EncodeString] or "Assertion failure: JSID_IS_STRING(iden),"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stacks

a = {}
a.getOwnPropertyDescriptor = XML;
b = Proxy.create(a)
for (x in this)
try {
  (function() {
    "use strict";
    b[2] = x
  })()
} catch (e) {}

asserts js debug shell on m-c changeset 8414a5a38e56 without any CLI arguments at Assertion failure: JSID_IS_STRING(iden), and crashes js opt shell at JS_EncodeString

Seems like a null deref, but setting sg:critical just to play safe unless shown otherwise.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   89620:bba06c18d52d
user:        Bobby Holley
date:        Fri Mar 16 12:47:21 2012 -0700
summary:     Bug 596351 - Proxies should throw TypeErrors for assignments to read-only properties in ES5 strict mode.
 r=luke

Comment 1

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

> Seems like a null deref, but setting sg:critical just to play safe unless
> shown otherwise.

I certainly meant *s-s* to play safe, changing to sg:dos based on that assumption.

Comment 2

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

> I certainly meant *s-s* to play safe, changing to sg:dos based on that
> assumption.

I'm actually entirely not sure, I see similar crashes on Windows but with possibly different register values.

I'll leave it to someone knowledgeable to determine the rating.

Comment 3

[Daniel Veditz [:dveditz]]

Assigning to bobby based on the regression range.

Comment 4

[Bobby Holley (:bholley)]

Attachment: Reuse the throwing code in jsobj.cpp for jsproxy.cpp. v1

Attaching a patch. Flagging luke for review.

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 612277 [details] [diff] [review]
Reuse the throwing code in jsobj.cpp for jsproxy.cpp. v1

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

Nice

::: js/src/jsobj.cpp
@@ +1839,5 @@
> +    jsid idstr;
> +
> +    if (!js_ValueToStringId(cx, IdToValue(id), &idstr))
> +       return false;
> +    JSAutoByteString bytes(cx, JSID_TO_STRING(idstr));

Pre-existing, but can you replace JSID_TO_STRING(js_ValueToStringId(IdToValue)) with IdToString?

Comment 6

[Bobby Holley (:bholley)]

Sure.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=32ba0266b16c

Comment 7

[Bobby Holley (:bholley)]

Looks green enough. Pushed to m-i:

http://hg.mozilla.org/integration/mozilla-inbound/rev/36bae719a2d4

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/36bae719a2d4

Comment 9

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.