Assertion failure: !cx->isExceptionPending(), at js/src/jscntxtinlines.h:238 with saveBytecode

RESOLVED FIXED in Firefox 41

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: decoder, Assigned: bz)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla41
x86_64
Linux
assertion, regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
The following testcase crashes on mozilla-central revision 7d4ab4a9febd (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe):

test  = (function () {
  function f() {
    return !f.hasOwnProperty('length') && !f.hasOwnProperty('name')
  };
  return "var obj = { x : 2 };" + f.toSource() + "; f()";
})();
evalWithCache(test, { assertEqBytecode: true, assertEqResult : true });
function evalWithCache(code, ctx) {
  code = cacheEntry(code);
  ctx.global = newGlobal();
  ctx.isRunOnce = true;
  var res1 = evaluate(code, Object.create(ctx, {saveBytecode: { value: true } }));
  var res2 = evaluate(code, Object.create(ctx, {loadBytecode: { value: true }, saveBytecode: { value: true } }));
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x000000000068e8e3 in js::CallJSNative (cx=0x7ffff691b4e0, native=0x482d60 <Evaluate(JSContext*, unsigned int, jsval*)>, args=...) at js/src/jscntxtinlines.h:238
#0  0x000000000068e8e3 in js::CallJSNative (cx=0x7ffff691b4e0, native=0x482d60 <Evaluate(JSContext*, unsigned int, jsval*)>, args=...) at js/src/jscntxtinlines.h:238
#1  0x000000000067b85c in js::Invoke (cx=cx@entry=0x7ffff691b4e0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:703
#2  0x0000000000674c47 in Interpret (cx=cx@entry=0x7ffff691b4e0, state=...) at js/src/vm/Interpreter.cpp:2954
#3  0x000000000067b283 in js::RunScript (cx=cx@entry=0x7ffff691b4e0, state=...) at js/src/vm/Interpreter.cpp:652
#4  0x00000000006862ee in js::ExecuteKernel (cx=cx@entry=0x7ffff691b4e0, script=..., script@entry=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:887
#5  0x0000000000688531 in js::Execute (cx=cx@entry=0x7ffff691b4e0, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:927
#6  0x0000000000a86c15 in ExecuteScript (cx=cx@entry=0x7ffff691b4e0, obj=..., scriptArg=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4188
#7  0x0000000000a86ddb in JS_ExecuteScript (cx=cx@entry=0x7ffff691b4e0, scriptArg=..., scriptArg@entry=...) at js/src/jsapi.cpp:4210
#8  0x0000000000426ac1 in RunFile (compileOnly=false, file=0x7ffff69ac800, filename=0x7fffffffdfdc "min.js", cx=0x7ffff691b4e0) at js/src/shell/js.cpp:443
#9  Process (cx=cx@entry=0x7ffff691b4e0, filename=0x7fffffffdfdc "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:561
#10 0x00000000004724b1 in ProcessArgs (op=0x7fffffffda60, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:5749
#11 Shell (envp=<optimized out>, op=0x7fffffffda60, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:6018
#12 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6354
rax	0x0	0
rbx	0x7ffff691b4e0	140737330132192
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffc4f0	140737488340208
rsp	0x7fffffffc4a0	140737488340128
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffc260	140737488339552
r11	0x7ffff6c27960	140737333328224
r12	0x7ffff4cf4158	140737300611416
r13	0x0	0
r14	0x7fffffffc4b0	140737488340144
r15	0x482d60	4730208
rip	0x68e8e3 <js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)+643>
=> 0x68e8e3 <js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)+643>:	movl   $0xee,0x0
   0x68e8ee <js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)+654>:	callq  0x4933b0 <abort()>
(Reporter)

Updated

3 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
(Reporter)

Comment 1

3 years ago
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/796283553115
user:        Boris Zbarsky
date:        Wed Apr 01 12:05:29 2015 -0400
summary:     Bug 679939 part 5.  Stop using the compileAndGo script flag in the bytecode emitter.  r=luke

This iteration took 174.888 seconds to run.
Flags: needinfo?(bzbarsky)
OK, so the exception is getting set by this bit

1247                    JS_ReportErrorNumber(cx, my_GetErrorMessage, nullptr,
1248                                         JSSMSG_CACHE_EQ_CONTENT_FAILED);

in Evaluate in js.cpp.

In particular, both loadBuffer and saveBuffer have 693 bytes, and they differ in one byte: byte 472.  saveBuffer has a 12 there, while loadBuffer has a 4.

So there are two issues:

1)  The js.cpp code is just broken: it should return false after doing the JS_ReportErrorNumber.

2)  Why are our buffers ending up different?
Flags: needinfo?(bzbarsky)
So if I make checkRunOnceContext() always return false, the assert goes away.
Aha.  So the function that's being XDR-encoded has the 0x800 flag set on the second call but not the first one.

That's the RESOLVED_NAME flag.

Seems to me like XDR of a function should mask out the RESOLVED_NAME and RESOLVED_LENGTH flags, no?

So now we know what's going on: we're in a run-once context, so we just create a singleton function.  The first time the code is evaluated, the f.hasOwnProperty("length") call returns true, so f returns false and the hasOwnProperty("name") never happens.  Then we save the bytecode and save the RESOLVED_LENGTH flag.  Then we deserialize and run again.  This time f.hasOwnProperty("length") returns false because the flag is set so we don't resolve the property (bug!) and then we go on to do the hasOwnProperty("name") and when we do that one we set the RESOLVED_NAME flag.  Now we serialize again and get different XDR data.
Created attachment 8617393 [details] [diff] [review]
part 1.  Fix shell's Evaluate to actually throw when it detects save/load bytecode mismatches
Attachment #8617393 - Flags: review?(jwalden+bmo)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Created attachment 8617398 [details] [diff] [review]
part 2.  When XDR-encoding a function, don't incode temporary flags
Attachment #8617398 - Flags: review?(jwalden+bmo)

Comment 7

3 years ago
Comment on attachment 8617393 [details] [diff] [review]
part 1.  Fix shell's Evaluate to actually throw when it detects save/load bytecode mismatches

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

::: js/src/shell/js.cpp
@@ +1247,1 @@
>              } else if (!PodEqual(loadBuffer, saveBuffer.get(), loadLength)) {

if (saveLength != loadLength) {
    ...
    return false;
}

if (!PodEqual(...)) {
    ...
    return false;
}
Attachment #8617393 - Flags: review?(jwalden+bmo) → review+

Comment 8

3 years ago
Comment on attachment 8617393 [details] [diff] [review]
part 1.  Fix shell's Evaluate to actually throw when it detects save/load bytecode mismatches

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

::: js/src/shell/js.cpp
@@ +1247,1 @@
>              } else if (!PodEqual(loadBuffer, saveBuffer.get(), loadLength)) {

if (saveLength != loadLength) {
    ...
    return false;
}

if (!PodEqual(...)) {
    ...
    return false;
}

Comment 9

3 years ago
Comment on attachment 8617398 [details] [diff] [review]
part 2.  When XDR-encoding a function, don't incode temporary flags

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

::: js/src/jit-test/tests/xdr/function-flags.js
@@ +1,4 @@
> +load(libdir + 'bytecode-cache.js');
> +
> +// Ensure that if a function is encoded we don't encode its "name
> +// resolved" flag.

Seems like it'd be nice to also have one or more of these tests test behavior with the name/length deleted (that is, resolved but not present), just for comprehensiveness.
Attachment #8617398 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/53ea4904898e
https://hg.mozilla.org/mozilla-central/rev/62934f4d92c8
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox41: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.