1468629 - Assertion failure: shape->writable(), at js/src/vm/Interpreter.cpp:4543 with bytecode caching

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 87b46cd1c941 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

function lfEvalInCache(lfCode, lfIncremental = false, lfRunOnce = false) {
  let ctx = {};
  let code = cacheEntry(lfCode);
  ctx_save = Object.create(ctx, { saveIncrementalBytecode: { value: true } });
  try { evaluate(code, ctx_save); } catch(exc) {}
  try { evaluate(code, Object.create(ctx_save, { loadBytecode: { value: true } })); } catch(exc) {}
}
lfEvalInCache(`
  function q() {}
  Object.freeze(this);
`, false, true);


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x000000000059d8b6 in js::DefFunOperation (cx=<optimized out>, script=..., envChain=..., fun=...) at js/src/vm/Interpreter.cpp:4543
#0  0x000000000059d8b6 in js::DefFunOperation (cx=<optimized out>, script=..., envChain=..., fun=...) at js/src/vm/Interpreter.cpp:4543
#1  0x00000000005a8445 in Interpret (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:3634
#2  0x00000000005aecdd in js::RunScript (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:421
#3  0x00000000005b1e6d in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7ffff4809150) at js/src/vm/Interpreter.cpp:704
#4  0x00000000005b21f9 in js::Execute (cx=<optimized out>, cx@entry=0x7ffff5f17000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7ffff4809150) at js/src/vm/Interpreter.cpp:737
#5  0x0000000000a4bb03 in ExecuteScript (cx=cx@entry=0x7ffff5f17000, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x7ffff4809150) at js/src/jsapi.cpp:4738
#6  0x0000000000a4c0a3 in ExecuteScript (cx=0x7ffff5f17000, envChain=..., scriptArg=..., rval=0x7ffff4809150) at js/src/jsapi.cpp:4757
#7  0x0000000000a4c14a in JS_ExecuteScript (cx=<optimized out>, envChain=..., scriptArg=..., scriptArg@entry=..., rval=...) at js/src/jsapi.cpp:4778
#8  0x00000000004758a3 in Evaluate (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:2027
#9  0x00000000005ba3ec in js::CallJSNative (cx=0x7ffff5f17000, native=0x474d50 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:274
[...]
#23 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9373
rax	0x0	0
rbx	0x7fffffffba40	140737488337472
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbb40	140737488337728
rsp	0x7fffffffb9f0	140737488337392
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7fffffffbac0	140737488337600
r13	0x7fffffffbae0	140737488337632
r14	0x1	1
r15	0x7fffffffbea0	140737488338592
rip	0x59d8b6 <js::DefFunOperation(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JSFunction*>)+1382>
=> 0x59d8b6 <js::DefFunOperation(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JSFunction*>)+1382>:	movl   $0x0,0x0
   0x59d8c1 <js::DefFunOperation(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JSFunction*>)+1393>:	ud2

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/3dee28e17417
user:        Tooru Fujisawa
date:        Fri Jun 08 10:06:13 2018 +0900
summary:     Bug 1467052 - Use BindingName tag to distinguish between var and top-level function, instead of offset range. r=shu

This iteration took 310.625 seconds to run.

Comment 2

[Jan de Mooij [:jandem]]

NI per comment 1.

Comment 3

[Tooru Fujisawa [:arai]]

I forgot to handle isTopLevelFunction bit in XDR.
I'll fix shortly

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Propery encode/decode BindingName flags in XDR.

Moved the `flags` bit pattern generation into BindingName from XDR part (actually just passes the FlagMask part of bits_), so that any new flags are reflected properly on XDR.
also Added BindingName::FromXDR to re-create BindingName instance from atom and flags.

I'm worrying about the similarity of the signatures between 2 ctors (bool/uint8_t).
let me know if you have better idea how to create `bits_` from atom and flags.

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 8985432 [details] [diff] [review]
Propery encode/decode BindingName flags in XDR.

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

::: js/src/vm/Scope.cpp
@@ +216,5 @@
>      RootedAtom atom(cx, bindingName->name());
>      bool hasAtom = !!atom;
>  
> +    uint8_t flags = bindingName->flagsForXDR();
> +    MOZ_ASSERT(!(flags & 0x80));

I would probably write this as ((flags << 1) >> 1) rather than create a necessary implicit correspondence between uint8_t and 0x80 here.  The 1 as magic constant isn't too ideal either, I guess -- could be a global static constexpr just above these functions, I guess.

::: js/src/vm/Scope.h
@@ +133,5 @@
> +  private:
> +    // For FromXDR.
> +    BindingName(JSAtom* name, uint8_t flags)
> +      : bits_(uintptr_t(name) | flags)
> +    { }

Maybe add an assert that (flags < alignof(JSAtom)), or with FlagsMask instead?

@@ +136,5 @@
> +      : bits_(uintptr_t(name) | flags)
> +    { }
> +
> +  public:
> +    static BindingName FromXDR(JSAtom* name, uint8_t flags) {

My general sense is we camelCaps class static functions usually, not apply to them the convention we apply to static standalone functions.

Comment 6

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31c7ebf5b9499a227923bc6483db7c3abb2bd340
Bug 1468629 - Propery encode/decode BindingName flags in XDR. r=jwalden

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/31c7ebf5b949