1236548 - Assertion failure: data.s.payload.why == why, at js/src/debug64/dist/include/js/Value.h:1218 with ES6 Classes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision d7a0ad85d9fb (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-extra-checks --ion-offthread-compile=off --baseline-eager):

function f(thunk) {
  try {
    thunk()
  } catch (e) {}
}
class foo extends null {
  constructor() {
    class Array {}
  }
}
for (i = 0; i < 10000; ++i)
  f(() => new foo)



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004492aa in JS::Value::isMagic (why=JS_IS_CONSTRUCTING, this=0x7fffffffc590) at js/src/debug64/dist/include/js/Value.h:1218
#0  0x00000000004492aa in JS::Value::isMagic (why=JS_IS_CONSTRUCTING, this=0x7fffffffc590) at js/src/debug64/dist/include/js/Value.h:1218
#1  0x00000000007fea71 in isMagic (why=JS_IS_CONSTRUCTING, this=0x7fffffffc590) at js/src/jscntxt.h:686
#2  isMagic (why=JS_IS_CONSTRUCTING, this=0x7fffffffc580) at js/src/debug64/dist/include/js/Value.h:1747
#3  js::jit::InvokeFunction (cx=0x7ffff6907800, obj=..., constructing=<optimized out>, argc=<optimized out>, argv=<optimized out>, rval=...) at js/src/jit/VMFunctions.cpp:81
#4  0x00007ffff7ff04df in ?? ()
#5  0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff6907800	140737330051072
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffc520	140737488340256
rsp	0x7fffffffc520	140737488340256
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffc2e0	140737488339680
r11	0x7ffff6c27960	140737333328224
r12	0x7fffffffc5e0	140737488340448
r13	0x7fffffffc580	140737488340352
r14	0x1	1
r15	0x7fffffffc5a0	140737488340384
rip	0x4492aa <JS::Value::isMagic(JSWhyMagic) const+28>
=> 0x4492aa <JS::Value::isMagic(JSWhyMagic) const+28>:	movl   $0x4c2,0x0
   0x4492b5 <JS::Value::isMagic(JSWhyMagic) const+39>:	callq  0x4a4a90 <abort()>

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

ni efaust as classes are involved

Comment 2

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20151009120049" and the hash "2c91f257f53d9c2d1e0df1578ccc0fcf9c740bf3".
The "bad" changeset has the timestamp "20151009120613" and the hash "250cd0bf3ce07627f08057cc7a38bc2d67174f9f".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2c91f257f53d9c2d1e0df1578ccc0fcf9c740bf3&tochange=250cd0bf3ce07627f08057cc7a38bc2d67174f9f

Comment 3

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

Wait, this probably isn't accurate. It can't probably be fixed by something from before the rev in comment 0.

Comment 4

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

Ignore comment 3, my bad.

Eric, is bug 1105463 a likely regressor?

Comment 5

[Eric Faust [:efaust]]

Uhm, it's a regressor inasmuch as the testcase wouldn't parse before then, but I don't think it's at fault, no. Anyway, I have a fix.

Comment 6

[Eric Faust [:efaust]]

Attachment: fixMagicConstructAssert.patch

We have both JS_IS_CONSTRUCTING and JS_UNINITIALIZED_LEXICAL flowing validly through this space. Both are valid to use the construction code without creating a new object.

Comment 7

[Jeff Walden [:Waldo]]

Comment on attachment 8706677 [details] [diff] [review]
fixMagicConstructAssert.patch

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

Add a testcase, ideally the ones I put in the comments.  Beware that inIon() allegedly throws if Ion is disabled, or something (i.e. tryserver carefully).

::: js/src/jit/VMFunctions.cpp
@@ +75,5 @@
>              cargs[i].set(argvWithoutThis[i]);
>          RootedValue newTarget(cx, argvWithoutThis[argc]);
> +        // If |this| hasn't been created, or is JS_UNINITIALIED_LEXICAL,
> +        // we can use normal construction code without creating an extraneous
> +        // object.

IZED

Add an assert that the magic-value is *only* one of these two values, so if more flow through in the future we learn about it (as the current code pointed out another one does flow through here now).

I'd like to see demos of the relevant triggering cases.  This language, disconnected from actual testcases, is very inscrutable, even considering I just touched this like three months ago.  These should work:

// JS_IS_CONSTRUCTING
var g = newGlobal();
do {
  new g.String(); // jit::CreateThis passes JS_IS_CONSTRUCTING
} while (!inIon());

// JS_UNINITIALIZED_LEXICAL
class B {};
class D extends B {
  constructor() { super(); }
};
do {
  new D(); // jit::CreateThis passes JS_UNINITIALIZED_LEXICAL
} while (!inIon());

Comment 8

[Pulsebot]

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

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10164991b6cc

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b8e4d6e226bd
https://hg.mozilla.org/mozilla-central/rev/10164991b6cc