1032208 - Assertion failure: v.isObject(), at json.cpp:508 or Crash [@ GetObjectClass] or Crash [@ js::ObjectClassIs] with Symbol and JSON

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision b6408c32a170 (run with --fuzzing-safe --ion-eager):


JSON.stringify(Symbol());

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

Opt-crash trace:


Program received signal SIGSEGV, Segmentation fault.
0x00000000007e1458 in GetObjectClass (obj=0x7ffff7e1d040) at js/src/jsfriendapi.h:600
600         return reinterpret_cast<const shadow::Object*>(obj)->type->clasp;
#0  0x00000000007e1458 in GetObjectClass (obj=0x7ffff7e1d040) at js/src/jsfriendapi.h:600
#1  IsProxy (obj=0x7ffff7e1d040) at js/src/jsproxy.h:362
#2  is<js::ProxyObject> (this=0x7ffff7e1d040) at js/src/vm/ProxyObject.h:118
#3  js::ObjectClassIs (obj=..., classValue=js::ESClass_Array, cx=0x161a6e0) at js/src/jsobjinlines.h:1016
#4  0x00000000007ca9ca in Str (cx=0x161a6e0, v=..., scx=0x7fffffffc5c0) at js/src/json.cpp:513
#5  0x00000000007cb083 in Str (cx=<optimized out>, v=..., scx=0x7fffffffc5c0) at js/src/json.cpp:491
#6  0x00000000007cbe45 in js_Stringify (cx=<optimized out>, vp=..., replacer_=<optimized out>, space_=..., sb=...) at js/src/json.cpp:685
#7  0x00000000007ccbdc in json_stringify (cx=<optimized out>, argc=<optimized out>, vp=0x7fffffffcfc8) at js/src/json.cpp:873
rax     0x0     0
=> 0x7e1458 <js::ObjectClassIs(JS::HandleObject, js::ESClassValue, JSContext*)+8>:      mov    (%rax),%rcx



Although the opt-crash trace here seems to show a crash at 0x0, I've found that the test changes it's crash address when being passed strings to Symbol. I also saw crash addresses 0x13, 0xf and 0x1d so I must assume that one can control/influence the crash address somehow using the arguments. S-s and sec-high because of that.

Comment 3

[Christian Holler (:decoder)]

Similar/related assertion (as requested by jorendorff):

Assertion failure: v.isString() || v.isObject(), at vm/TypedArrayObject.cpp:916

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: bug-1032208-part-1-json-v1.patch

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: bug-1032208-part-2-typed-arrays-v1.patch

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8448221 [details] [diff] [review]
bug-1032208-part-2-typed-arrays-v1.patch

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

::: js/src/tests/ecma_6/Symbol/typed-arrays.js
@@ +1,2 @@
> +/* Any copyright is dedicated to the Public Domain.
> + * http://creativecommons.org/licenses/publicdomain/ */

Personally I prefer jit-tests, because we run them on TBPL with --ion-eager, --baseline-eager etc.

@@ +5,5 @@
> +
> +var tests = [
> +    {T: Int16Array, result: 0},
> +    {T: Uint8Array, result: 0},
> +    {T: Float32Array, result: NaN}

Nit: use Uint8ClampedArray instead of Uint8Array (or test both).

Comment 8

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f81a509fc014
https://hg.mozilla.org/integration/mozilla-inbound/rev/03f72dc8cbb1

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f81a509fc014
https://hg.mozilla.org/mozilla-central/rev/03f72dc8cbb1

Comment 10

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 11

[Al Billings [:abillings - ex-MoCo]]

This didn't go through sec-approval before going in. How far back does this go?

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Was ESR31 affected by this?

Comment 13

[Christian Holler (:decoder)]

I don't think so since Symbol was a very recent addition at that time, so the bugs we filed were Nightly only.