Closed Bug 1304551 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- fix-optional
firefox52 --- fixed

People

(Reporter: decoder, Assigned: till)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

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
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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
Till, would bug 1000780 from the regression window be a likely regressor?
Blocks: 1000780
Flags: needinfo?(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.
Assignee: nobody → till
Status: NEW → ASSIGNED
Flags: needinfo?(till)
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.
Attachment #8796158 - Flags: review?(efaustbmo) → review+
Till, is this good to land?
Flags: needinfo?(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.
Flags: needinfo?(till)
Keywords: checkin-needed
Pushed by tschneidereit@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0394048027e8
Properly handle bound functions in getBacktrace. r=efaust
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0394048027e8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
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.
(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.
Flags: needinfo?(till)
You need to log in before you can comment on or make changes to this bug.