1304551 - Assertion failure: v.isUndefined(), at js/src/jsstr.cpp:3050 with getBacktrace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

thisValue = {};
var t = function(a, b, c, d) {
    getBacktrace({
        locals: true,
    });
}
var f = t.bind(thisValue);
var res2 = new f(1, 2, 3, 4)


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x00000000009b84c8 in js::ToStringSlow<(js::AllowGC)1> (cx=cx@entry=0x7ffff695f000, arg=...) at js/src/jsstr.cpp:3050
#0  0x00000000009b84c8 in js::ToStringSlow<(js::AllowGC)1> (cx=cx@entry=0x7ffff695f000, arg=...) at js/src/jsstr.cpp:3050
#1  0x00000000008f6049 in js::ToString<(js::AllowGC)1> (cx=cx@entry=0x7ffff695f000, v=..., v@entry=...) at js/src/jsstr.h:172
#2  0x00000000009298d2 in FormatFrame (showThisProps=false, showLocals=true, showArgs=false, num=1, buf=<optimized out>, iter=..., cx=0x7ffff695f000) at js/src/jsfriendapi.cpp:902
#3  JS::FormatStackDump (cx=cx@entry=0x7ffff695f000, buf=<optimized out>, buf@entry=0x0, showArgs=false, showLocals=true, showThisProps=false) at js/src/jsfriendapi.cpp:1013
#4  0x0000000000c59676 in GetBacktrace (cx=cx@entry=0x7ffff695f000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:2469
#5  0x0000000000aec7c9 in js::CallJSNative (cx=cx@entry=0x7ffff695f000, native=0xc59620 <GetBacktrace(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
[...]
#18 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7663
rax	0x0	0
rbx	0x7ffff695f000	140737330409472
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffc400	140737488339968
rsp	0x7fffffffc3b0	140737488339888
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7fffffffc6c0	140737488340672
r13	0x7ffff04bb8c0	140737224882368
r14	0x7fffffffc790	140737488340880
r15	0x7ffff695f000	140737330409472
rip	0x9b84c8 <js::ToStringSlow<(js::AllowGC)1>(js::ExclusiveContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType)+712>
=> 0x9b84c8 <js::ToStringSlow<(js::AllowGC)1>(js::ExclusiveContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType)+712>:	movl   $0x0,0x0
   0x9b84d3 <js::ToStringSlow<(js::AllowGC)1>(js::ExclusiveContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType)+723>:	ud2

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20160112055644" and the hash "0af7319a6a4169d94453f8fcfd78384459c343db".
The "bad" changeset has the timestamp "20160112064943" and the hash "592fc90e655a1ebd3968300b5ed6261d24ed4065".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0af7319a6a4169d94453f8fcfd78384459c343db&tochange=592fc90e655a1ebd3968300b5ed6261d24ed4065

Comment 2

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

Till, would bug 1000780 from the regression window be a likely regressor?

Comment 3

[Till Schneidereit [:till]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2)
> Till, would bug 1000780 from the regression window be a likely regressor?

Yes, seems that way. I can reproduce and am looking into it.

Comment 4

[Till Schneidereit [:till]]

Attachment: Bug 1304551 - Properly handle bound functions in getBacktrace.

getBacktrace, when invoked with `{locals:true}`, prints the `this` value for each frame, where applicable. When constructing bound functions, the `this` value is magic, which FormatFrame doesn't handle. Since printing that `this` value isn't useful, we just skip it.

Review commit: https://reviewboard.mozilla.org/r/82104/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82104/

Comment 5

[Eric Faust [:efaust]]

Comment on attachment 8796158 [details]
Bug 1304551 - Properly handle bound functions in getBacktrace.

https://reviewboard.mozilla.org/r/82104/#review82582

Oh, this used to be safe, once upon a time, befroe the bind impl was self-hosted. Nice catch.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Till, is this good to land?

Comment 7

[Till Schneidereit [:till]]

It is, sorry for not doing so. Setting checkin-needed because the tree is closed right now, and I don't want to forget about this again.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0394048027e8
Properly handle bound functions in getBacktrace. r=efaust

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0394048027e8

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this worth possible risk of uplift to beta? If not, that's fine, but if you think it may be safe and help prevent particular crashes we're still in early beta in a long beta cycle.

Comment 11

[Till Schneidereit [:till]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #10)
> Is this worth possible risk of uplift to beta? If not, that's fine, but if
> you think it may be safe and help prevent particular crashes we're still in
> early beta in a long beta cycle.

While I don't see any particular reason not to uplift this, I also don't really see any reason to do so. This code is really only used by internal debugging functions. It's definitely not possible to trigger this from web content, and AFAICT, it's also not possible to do so in the JS debugger or platform/addons code - I'm only 95% sure of that, though. Even if it should turn out to be possible, the circumstances this would be triggered in are exceedingly unlikely to be hit in practice.