1260728 - Assertion failure: analyzedArgsUsage(), at js/src/jsscript.h:1515 with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

var g = newGlobal();
var dbg = new Debugger(g);
dbg.onNewScript = function(script) {
    fscript = script.getChildScripts()[0];
}
g.eval("function f(x) { arguments[0] = 3; return x }");
fscript.setBreakpoint(0, {hit:function(frame) {
    assertEq(frame.eval("assertEq(arguments, undefined)").return, 1);
}});
assertEq(g.f(1), 42);



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004687d2 in JSScript::needsArgsObj (this=<optimized out>) at js/src/jsscript.h:1515
#0  0x00000000004687d2 in JSScript::needsArgsObj (this=<optimized out>) at js/src/jsscript.h:1515
#1  0x0000000000b115ad in needsArgsObj (this=<optimized out>) at js/src/debug64/dist/include/js/RootingAPI.h:667
#2  isMissingArguments (scope=..., id=..., cx=0x7ffff6908800) at js/src/vm/ScopeObject.cpp:1928
#3  (anonymous namespace)::DebugScopeProxy::get (this=<optimized out>, cx=0x7ffff6908800, proxy=..., receiver=..., id=..., vp=...) at js/src/vm/ScopeObject.cpp:2197
#4  0x00000000009c01c9 in js::Proxy::get (cx=0x7ffff6908800, proxy=..., receiver_=..., id=..., vp=...) at js/src/proxy/Proxy.cpp:299
#5  0x000000000071c624 in GetProperty (vp=..., id=..., receiver=..., obj=..., cx=0x7ffff6908800) at js/src/vm/NativeObject.h:1476
#6  GetProperty (vp=..., id=..., receiver=..., obj=..., cx=0x7ffff6908800) at js/src/jsobj.h:830
#7  js::FetchName<false> (cx=0x7ffff6908800, obj=..., obj2=..., name=..., shape=..., vp=...) at js/src/vm/Interpreter-inl.h:191
#8  0x0000000000a8f458 in GetNameOperation (vp=..., pc=0x7ffff698c5c6 ";", fp=<optimized out>, cx=0x7ffff6908800) at js/src/vm/Interpreter.cpp:258
#9  Interpret (cx=cx@entry=0x7ffff6908800, state=...) at js/src/vm/Interpreter.cpp:2947
#10 0x0000000000a966a8 in js::RunScript (cx=cx@entry=0x7ffff6908800, state=...) at js/src/vm/Interpreter.cpp:426
#11 0x0000000000a99279 in js::ExecuteKernel (cx=cx@entry=0x7ffff6908800, script=..., script@entry=..., scopeChainArg=..., newTargetValue=..., evalInFrame=..., result=result@entry=0x7fffffffa8f0) at js/src/vm/Interpreter.cpp:684
#12 0x0000000000a0a0f2 in EvaluateInEnv (rval=..., lineno=<optimized out>, filename=<optimized out>, pc=<optimized out>, frame=..., env=..., cx=0x7ffff6908800, chars=...) at js/src/vm/Debugger.cpp:7233
#13 DebuggerGenericEval (cx=cx@entry=0x7ffff6908800, fullMethodName=fullMethodName@entry=0xefd435 "Debugger.Frame.prototype.eval", code=..., evalWithBindings=evalWithBindings@entry=EvalWithDefaultBindings, bindings=..., options=..., vp=..., dbg=dbg@entry=0x7ffff694f000, scope=..., scope@entry=..., iter=iter@entry=0x7fffffffac78) at js/src/vm/Debugger.cpp:7366
#14 0x0000000000a0b212 in DebuggerFrame_eval (cx=0x7ffff6908800, argc=<optimized out>, vp=<optimized out>) at js/src/vm/Debugger.cpp:7380
#15 0x0000000000a9a202 in js::CallJSNative (cx=0x7ffff6908800, native=0xa0afa0 <DebuggerFrame_eval(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
[...]
#43 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7443
rax	0x0	0
rbx	0x7ffff6908800	140737330055168
rcx	0x7ffff6ca5870	140737333844080
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffff9d90	140737488330128
rsp	0x7fffffff9d90	140737488330128
r8	0x7ffff7fdf7c0	140737354004416
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffff9b50	140737488329552
r11	0x7ffff6c27ee0	140737333329632
r12	0x7fffffff9f90	140737488330640
r13	0x7ffff5400000	140737308000256
r14	0x7fffffff9dd0	140737488330192
r15	0x7fffffff9df0	140737488330224
rip	0x4687d2 <JSScript::needsArgsObj() const+28>
=> 0x4687d2 <JSScript::needsArgsObj() const+28>:	movl   $0x5eb,0x0
   0x4687dd <JSScript::needsArgsObj() const+39>:	callq  0x4a9b00 <abort()>

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/29e5dcfb97f7
user:        Tom Schuster
date:        Tue Oct 06 17:04:09 2015 +0100
summary:     Bug 1211832 - Disable functions that can easily cause artificial OOMs. r=jonco

Tom, is bug 1211832 a likely regressor?

Comment 2

[Tom S [:evilpie]]

I don't think so. Nothing here uses OOMAtAllocation, OOMAfterAllocations or GCParameter.

Comment 3

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

Moving over to our Debugger folks then.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I can reproduce.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

First Debugger frame on the stack was last touched Thu Jan 21 20:01:18 2016, and all the other frames' blames are even older than that (2012-early 2015):

  f0f2c29d Bug 1234845 part 10 - Remove ExecuteType and InitialFrameFlags enums. r=luke

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Ensure that the script has had its arguments usage analyzed before we get arguments in the debugger

Ok so I am not really sure what the root cause is here, so this is a pretty
blunt approach. But it seems to work. If there is a finer, more precise
approach, please point me in the correct direction... I have this nagging
feeling that this patch is fixing one specific symptom arising from a larger
bug.

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d795e6a87476

Comment 8

[Shu-yu Guo [:shu]]

Comment on attachment 8758873 [details] [diff] [review]
Ensure that the script has had its arguments usage analyzed before we get arguments in the debugger

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

::: js/src/vm/ScopeObject.cpp
@@ +2161,5 @@
>          Rooted<DebugScopeObject*> debugScope(cx, &proxy->as<DebugScopeObject>());
>          Rooted<ScopeObject*> scope(cx, &proxy->as<DebugScopeObject>().scope());
>  
> +        if (!ensureHasAnalyzedArgsUsage(cx, *scope))
> +            return false;

You're right that calling ensureHasAnalyzedArgsUsage on the script is the thing to do, but DebugScopeProxy::get isn't the place to do it.

I see that we call the function in DebuggerArguments_getArg right now and we're missing it in eval-in-frame. I don't think performance will be an issue here, so it'll be more reassuring to aggressively call ensureHasAnalyzedArgsUsage on the scripts of all frames we reflect via Debugger.Frame. Mind doing that?

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Shu-yu Guo [:shu] from comment #8)
> Comment on attachment 8758873 [details] [diff] [review]
> Ensure that the script has had its arguments usage analyzed before we get
> arguments in the debugger
> 
> Review of attachment 8758873 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/ScopeObject.cpp
> @@ +2161,5 @@
> >          Rooted<DebugScopeObject*> debugScope(cx, &proxy->as<DebugScopeObject>());
> >          Rooted<ScopeObject*> scope(cx, &proxy->as<DebugScopeObject>().scope());
> >  
> > +        if (!ensureHasAnalyzedArgsUsage(cx, *scope))
> > +            return false;
> 
> You're right that calling ensureHasAnalyzedArgsUsage on the script is the
> thing to do, but DebugScopeProxy::get isn't the place to do it.
> 
> I see that we call the function in DebuggerArguments_getArg right now and
> we're missing it in eval-in-frame. I don't think performance will be an
> issue here, so it'll be more reassuring to aggressively call
> ensureHasAnalyzedArgsUsage on the scripts of all frames we reflect via
> Debugger.Frame. Mind doing that?

Thanks, this was exactly the kind of review I was hoping for!

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Ensure that the script has had its arguments usage analyzed before we get arguments in the debugger

Much more satisfying!

Comment 11

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff9eea6d0d5c

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/403b82279b76
Ensure that the script has had its arguments usage analyzed before we get arguments in the debugger; r=shu

Comment 13

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision e27fe24a746f).

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/403b82279b76