1138195 - Crash [@ js::FrameIter::callObj] with getBacktrace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

function f(x) {
    for (var i = 0; i < 40; ++i) {
	var stack = getBacktrace({args: true});
	(function() { g = x;});
    }
}
f(1);



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
js::FrameIter::callObj (this=this@entry=0x7fffffffb210, cx=cx@entry=0x1a25490) at js/src/vm/Stack.cpp:1194
1194	    while (!pobj->is<CallObject>())
#0  js::FrameIter::callObj (this=this@entry=0x7fffffffb210, cx=cx@entry=0x1a25490) at js/src/vm/Stack.cpp:1194
#1  0x0000000000aae8a2 in FormatFrame (cx=cx@entry=0x1a25490, iter=..., buf=0x1b33c70 "0 f(", buf@entry=0x0, num=num@entry=0, showArgs=showArgs@entry=true, showLocals=showLocals@entry=false, showThisProps=showThisProps@entry=false) at js/src/jsfriendapi.cpp:739
#2  0x0000000000aaeff5 in JS::FormatStackDump (cx=cx@entry=0x1a25490, buf=buf@entry=0x0, showArgs=<optimized out>, showLocals=<optimized out>, showThisProps=<optimized out>) at js/src/jsfriendapi.cpp:869
#3  0x00000000004a8029 in GetBacktrace (cx=0x1a25490, argc=<optimized out>, vp=0x7fffffffba98) at js/src/builtin/TestingFunctions.cpp:1963
#4  0x00007ffff7fdc74f in ?? ()
#5  0x00007ffff7e576d0 in ?? ()
#6  0x00007fffffffba70 in ?? ()
#7  0x0000000000000000 in ?? ()
rax	0x7ffff7e58318	140737352401688
rbx	0x199a520	26846496
rcx	0x100	256
rdx	0x0	0
rsi	0x7fffffffae60	140737488334432
rdi	0x7fffffffb2c0	140737488335552
rbp	0x7fffffffaec0	140737488334528
rsp	0x7fffffffae90	140737488334480
r8	0x7fffffffb2c0	140737488335552
r9	0x2	2
r10	0x1	1
r11	0x0	0
r12	0x0	0
r13	0x198a0a0	26779808
r14	0x19a58e0	26892512
r15	0x198a5a0	26781088
rip	0x66d0cf <js::FrameIter::callObj(JSContext*) const+175>
=> 0x66d0cf <js::FrameIter::callObj(JSContext*) const+175>:	mov    0x8(%r12),%rax
   0x66d0d4 <js::FrameIter::callObj(JSContext*) const+180>:	mov    (%rax),%rbx

Comment 1

[Christian Holler (:decoder)]

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/549b4ef82544
user:        Steve Fink
date:        Tue Jun 10 15:10:19 2014 -0700
summary:     Bug 1015339 - Add functions for getting and dumping the current backtrace, r=jandem

This iteration took 174.227 seconds to run.

Comment 2

[Christian Holler (:decoder)]

Needinfo from sfink based on comment 1 :)

Comment 3

[Christian Holler (:decoder)]

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

Comment 4

[Steve Fink [:sfink] [:s:]]

That changeset just added a test function that exposed the problem. I'll try ni? nbp because he touched the line of code that seems to be triggering the problem.

nbp: If you run the script in comment 0 with --no-threads --ion-eager (a debug build is fine), it will crash when it hits this line in FormatFrame:

                        arg = iter.callObj(cx).aliasedVar(fi);

callObj(cx) calls scopeChain(cx), which returns the global object, which causes the following loop (searching for a CallObject) to fail with a null pointer deref. (global->enclosingScope() is nullptr, which gets dereferenced for pobj->is<CallObject>()).

That's just gdb monkey work. I don't know enough about how this stuff works to make any sense of it.

Comment 5

[Nicolas B. Pierron [:nbp]]

The problem comes from the fact that we have an argument "x" which is aliased by an anonymous functions which is not called.  When we compile the function the bytecode emitter does not generate any code for the anonymous function:

00000:  zero
00001:  setlocal 0
00005:  pop
00006:  goto 53 (+47)
00011:  loophead
00012:  getgname "getBacktrace"
00017:  gimplicitthis "getBacktrace"
00022:  newobject ({args:(void 0)})
00027:  true
00028:  initprop "args"
00033:  call 1
00036:  setlocal 1
00040:  pop
00041:  getlocal 0
00045:  pos
00046:  one
00047:  add
00048:  setlocal 0
00052:  pop
00053:  loopentry 129
00055:  getlocal 0
00059:  int8 40
00061:  lt
00062:  ifne 11 (-51)
00067:  retrval

This implies that when we compile the function in IonBuilder [1], the BytecodeAnalysis claim that we do not need any call object on our scope chain.

When getBacktrace is called, the variable "x" is still flagged as being aliased [2], even after the removal of the dead-function.

This cause us to get look for a call object which does not exists, and thus fail.

I think we can hack something to expose the result of the BytecodeAnalysis once baseline is done, as we can query this to do the proper thing in this case.  On the other hand, I feel that we should better do that as part of the Bytecode emitter instead, in order to have consistent information.

Jason, what do you think?

[1] https://dxr.mozilla.org/mozilla-central/source/js/src/jit/IonBuilder.cpp#1176
[2] https://dxr.mozilla.org/mozilla-central/source/js/src/jsfriendapi.cpp#739

Comment 6

[Jan de Mooij [:jandem]]

I think BytecodeAnalysis should just set usesScopeChain_ = true if script->funHasAnyAliasedFormal().

Comment 7

[Nicolas B. Pierron [:nbp]]

Jandem, good idea I will write a patch to do that.

Comment 8

[Nicolas B. Pierron [:nbp]]

Attachment: Ensure that the bytecode analysis is consistent with the bindings.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8597962 [details] [diff] [review]
Ensure that the bytecode analysis is consistent with the bindings.

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

Looks good.

::: js/src/jit/BytecodeAnalysis.cpp
@@ +44,5 @@
>  {
>      if (!infos_.growByUninitialized(script_->length()))
>          return false;
>  
> +    // We need a scope chain if any of the bindings is aliased.

Nit: are aliased

Comment 10

[Pulsebot]

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

Comment 11

[Nicolas B. Pierron [:nbp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c30055471659

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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