1109964 - Assertion failure: !val.isMagic(), at js/src/jsobj.cpp:3637 with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision d7c76fe69e9a (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-debug, run with --no-threads --fuzzing-safe):

var evalInFrame = (function (global) {
  var dbgGlobal = newGlobal();
  var dbg = new dbgGlobal.Debugger();
  return function evalInFrame(upCount, code) {
    dbg.addDebuggee(global);
    var frame = dbg.getNewestFrame().older;
      frame = frame.older;
    var completion = frame.eval(code);
  };
})(this);
function i(save) {
    evalInFrame(1, "a.push(z)", save);
}
function f(b) {
    var a = arguments;
    i.apply(null, a);
}
f(true);



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000009abcd9 in js::ToObjectSlow (cx=0x1a07150, val=..., reportScanStack=true) at js/src/jsobj.cpp:3637
3637	    MOZ_ASSERT(!val.isMagic());
#0  0x00000000009abcd9 in js::ToObjectSlow (cx=0x1a07150, val=..., reportScanStack=true) at js/src/jsobj.cpp:3637
#1  0x0000000000a68e71 in ToObjectFromStack (vp=$jsmagic(JS_OPTIMIZED_ARGUMENTS), cx=0x1a07150) at js/src/jsobj.h:1178
#2  GetPropertyOperation (vp=$jsmagic(JS_OPTIMIZED_ARGUMENTS), lval=$jsmagic(JS_OPTIMIZED_ARGUMENTS), pc=<optimized out>, script=..., fp=0x7fffffff9830, cx=0x1a07150) at js/src/vm/Interpreter.cpp:246
#3  Interpret (cx=0x1a07150, state=...) at js/src/vm/Interpreter.cpp:2359
#4  0x0000000000a6ced7 in js::RunScript (cx=0x1a07150, state=...) at js/src/vm/Interpreter.cpp:434
#5  0x0000000000a6d282 in js::ExecuteKernel (cx=0x1a07150, script=..., scopeChainArg=(JSObject &) @0x7ffff7e5e140 [object Proxy], thisv=..., type=<optimized out>, evalInFrame=..., result=0x7fffffffa3a0) at js/src/vm/Interpreter.cpp:643
#6  0x0000000000a6d894 in js::EvaluateInEnv (cx=0x1a07150, env=(JSObject * const) 0x7ffff7e5e140 [object Proxy], thisv=$jsval((JSObject *) 0x7ffff7e5d060 [object global] delegate), frame=..., chars=..., filename=<optimized out>, lineno=<optimized out>, rval=JSVAL_VOID) at js/src/vm/Debugger.cpp:5639
#7  0x0000000000a81f12 in DebuggerGenericEval (cx=0x1a07150, fullMethodName=<optimized out>, code=..., evalWithBindings=EvalWithDefaultBindings, bindings=..., options=JSVAL_VOID, vp=$jsval((JSObject *) 0x7ffff7e83880 [object Function "eval"]), dbg=0x1af78c0, scope=0x0, iter=0x7fffffffa628) at js/src/vm/Debugger.cpp:5776
#8  0x0000000000a82a56 in DebuggerFrame_eval (cx=0x1a07150, argc=<optimized out>, vp=<optimized out>) at js/src/vm/Debugger.cpp:5790
#9  0x0000000000a8c4a5 in js::CallJSNative (cx=0x1a07150, native=0xa827a0 <DebuggerFrame_eval(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:231
#10 0x0000000000a6db47 in js::Invoke (cx=0x1a07150, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:484
#11 0x0000000000a6ef9d in js::Invoke (cx=0x1a07150, thisv=..., fval=..., argc=<optimized out>, argv=0x1a80690, rval=$jsval((JSObject *) 0x7ffff7e83880 [object Function "eval"])) at js/src/vm/Interpreter.cpp:540
#12 0x00000000009ec12e in js::DirectProxyHandler::call (this=<optimized out>, cx=0x1a07150, proxy=(JSObject * const) 0x7ffff7e5e120 [object Proxy], args=...) at js/src/proxy/DirectProxyHandler.cpp:75
#13 0x00000000009ec2a5 in js::CrossCompartmentWrapper::call (this=0x19bc540, cx=0x1a07150, wrapper=(JSObject * const) 0x7ffff7e5e120 [object Proxy], args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:296
#14 0x00000000009ea57f in js::Proxy::call (cx=0x1a07150, proxy=(JSObject * const) 0x7ffff7e5e120 [object Proxy], args=...) at js/src/proxy/Proxy.cpp:430
#15 0x00000000009ea66a in js::proxy_Call (cx=0x1a07150, argc=<optimized out>, vp=<optimized out>) at js/src/proxy/Proxy.cpp:812
#16 0x0000000000a8c4a5 in js::CallJSNative (cx=0x1a07150, native=0x9ea600 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:231
#17 0x0000000000a6ddec in js::Invoke (cx=0x1a07150, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:477
#18 0x0000000000a5fbf9 in Interpret (cx=0x1a07150, state=...) at js/src/vm/Interpreter.cpp:2541
#19 0x0000000000a6ced7 in js::RunScript (cx=0x1a07150, state=...) at js/src/vm/Interpreter.cpp:434
#20 0x0000000000a6dca5 in js::Invoke (cx=0x1a07150, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:503
#21 0x0000000000933324 in js_fun_apply (cx=0x1a07150, argc=<optimized out>, vp=0x1a80540) at js/src/jsfun.cpp:1319
#22 0x0000000000a8c4a5 in js::CallJSNative (cx=0x1a07150, native=0x932f90 <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:231
#23 0x0000000000a6db47 in js::Invoke (cx=0x1a07150, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:484
#24 0x0000000000a5fbf9 in Interpret (cx=0x1a07150, state=...) at js/src/vm/Interpreter.cpp:2541
#25 0x0000000000a6ced7 in js::RunScript (cx=0x1a07150, state=...) at js/src/vm/Interpreter.cpp:434
#26 0x0000000000a6d282 in js::ExecuteKernel (cx=0x1a07150, script=..., scopeChainArg=(JSObject &) @0x7ffff7e5d060 [object global] delegate, thisv=..., type=<optimized out>, evalInFrame=..., result=0x0) at js/src/vm/Interpreter.cpp:643
#27 0x0000000000a6d5eb in js::Execute (cx=0x1a07150, script=0x7ffff7e611a8, scopeChainArg=..., rval=0x0) at js/src/vm/Interpreter.cpp:680
#28 0x00000000008dd1bc in ExecuteScript (cx=0x1a07150, obj=(JSObject * const) 0x7ffff7e5d060 [object global] delegate, scriptArg=0x7ffff7e611a8, rval=0x0) at js/src/jsapi.cpp:4708
#29 0x0000000000417aa8 in RunFile (compileOnly=false, file=0x1aef460, filename=0x7fffffffef66 "min.js", obj=..., cx=0x1a07150) at js/src/shell/js.cpp:450
#30 Process (cx=0x1a07150, obj_=<optimized out>, filename=<optimized out>, forceTTY=<optimized out>) at js/src/shell/js.cpp:583
#31 0x0000000000424c29 in ProcessArgs (op=0x7fffffffe960, obj_=<optimized out>, cx=0x1a07150) at js/src/shell/js.cpp:5434
#32 Shell (op=0x7fffffffe960, cx=0x1a07150, envp=<optimized out>) at js/src/shell/js.cpp:5673
#33 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6020
rax	0x0	0
rbx	0x1a07150	27291984
rcx	0x7ffff6cb7910	140737333917968
rdx	0x0	0
rsi	0x7ffff6f8baa0	140737336883872
rdi	0x7ffff6f8a180	140737336877440
rbp	0x7fffffff9310	140737488327440
rsp	0x7fffffff92f0	140737488327408
r8	0x7ffff7fe8740	140737354041152
r9	0x72746e65632d616c	8247338199356891500
r10	0x7fffffff9080	140737488326784
r11	0x7ffff6c3fc90	140737333427344
r12	0x1a80730	27789104
r13	0xfffa000000000009	-1688849860263927
r14	0x1	1
r15	0x1a07168	27292008
rip	0x9abcd9 <js::ToObjectSlow(JSContext*, JS::HandleValue, bool)+313>
=> 0x9abcd9 <js::ToObjectSlow(JSContext*, JS::HandleValue, bool)+313>:	movl   $0x7b,0x0
   0x9abce4 <js::ToObjectSlow(JSContext*, JS::HandleValue, bool)+324>:	callq  0x4049f0 <abort@plt>

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Recover missing arguments when it gets assigned to another slot.

Explanation of bug in comments in the patch.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 8534758 [details] [diff] [review]
Recover missing arguments when it gets assigned to another slot.

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

::: js/src/vm/ScopeObject.cpp
@@ +1513,5 @@
> +    static bool isMagicMissingArgumentsValue(JSContext *cx, ScopeObject &scope, HandleValue v)
> +    {
> +        return v.isMagic() && v.whyMagic() == JS_OPTIMIZED_ARGUMENTS &&
> +               isFunctionScope(scope) &&
> +               !scope.as<CallObject>().callee().nonLazyScript()->needsArgsObj();

Why are the last two conjuncts necessary?  Is it even possible for there to be a JS_OPTIMIZED_ARGUMENTS magic value when needsArgsObj() is true?  I expect there is some terrible case where we only realize we need an args obj at runtime.  But, in that case, don't we still have a problem (we have a magic value).  Assuming it's *not* possible to have this case, then it'd be good to assert the latter two conjuncts instead.

@@ +1570,5 @@
> +        if (!createMissingArguments(cx, scope, &argsObj))
> +            return false;
> +
> +        if (!argsObj) {
> +            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE,

If createMissingArguments fails, it will have already reported an error, so this would be a double-report.  (Same comment for getMissingArguments.)

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 4

[Shu-yu Guo [:shu]]

(In reply to Luke Wagner [:luke] from comment #2)
> Comment on attachment 8534758 [details] [diff] [review]
> Recover missing arguments when it gets assigned to another slot.
> 
> Review of attachment 8534758 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/ScopeObject.cpp
> @@ +1513,5 @@
> > +    static bool isMagicMissingArgumentsValue(JSContext *cx, ScopeObject &scope, HandleValue v)
> > +    {
> > +        return v.isMagic() && v.whyMagic() == JS_OPTIMIZED_ARGUMENTS &&
> > +               isFunctionScope(scope) &&
> > +               !scope.as<CallObject>().callee().nonLazyScript()->needsArgsObj();
> 
> Why are the last two conjuncts necessary?  Is it even possible for there to
> be a JS_OPTIMIZED_ARGUMENTS magic value when needsArgsObj() is true?  I
> expect there is some terrible case where we only realize we need an args obj
> at runtime.  But, in that case, don't we still have a problem (we have a
> magic value).  Assuming it's *not* possible to have this case, then it'd be
> good to assert the latter two conjuncts instead.
> 

Good point, I'll assert the latter 2 conjuncts.

> @@ +1570,5 @@
> > +        if (!createMissingArguments(cx, scope, &argsObj))
> > +            return false;
> > +
> > +        if (!argsObj) {
> > +            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE,
> 
> If createMissingArguments fails, it will have already reported an error, so
> this would be a double-report.  (Same comment for getMissingArguments.)

There isn't a double reporting AIUI. The second JS_ReportErrorNumber is for when createMissingArguments returned *true* but didn't set the argsObj pointer, since there is no live scope corresponding to it. All cases where createMissingArguments return false, the caller also returns false immediately.

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 8534758 [details] [diff] [review]
Recover missing arguments when it gets assigned to another slot.

(In reply to Shu-yu Guo [:shu] from comment #4)
> There isn't a double reporting AIUI. The second JS_ReportErrorNumber is for
> when createMissingArguments returned *true* but didn't set the argsObj pointer

Hah, you're right.  r+ with the assertion.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fe70a6c9a374

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Fixed for Fx36 by the roll-up in bug 1114757.

Comment 8

[Shu-yu Guo [:shu]]

Attachment: Relax assertion in DebugScopeProxy::isMagicMissingArgumentsValue.

Oops, the assertion was too strict. Ion can emit OPTIMIZED_ARGUMENTS even when
the script !needsArgsObj(), such as in IonBuilder.cpp:877.

Comment 9

[Shu-yu Guo [:shu]]

Comment on attachment 8584901 [details] [diff] [review]
Relax assertion in DebugScopeProxy::isMagicMissingArgumentsValue.

Oops, attached to the wrong bug.