Closed Bug 622167 Opened 11 years ago Closed 11 years ago
GC related crash/asserts: is
Function Frame() / obj->num Slots() >= first + count
The attached two testcases trigger two (likely related) assertions with the shell: Assertion failure: isFunctionFrame(), at ../jsinterp.h:312 Assertion failure: obj->numSlots() >= first + count, at jsfun.cpp:1370 The second assertion isn't always triggered, sometimes also the first one shows up instead. For a reliable run, try running in valgrind also. Both versions will crash when continuing after the assertions. The bug might be related to 617139 because these examples also involve infinite recursion and GC triggering. The assertions are different though.
Taking, I've been working with code involving these assertions, so it's probably quickest for me to fix this...
Assignee: jimb → jwalden+bmo
What happens: * we call a heavyweight function, which recurs until just short of over-recursion, and then... * we create a Call object for the (very nested) frame * we set the Call object's private to that stack frame * we call js::Interpret, which immediately errors out from over-recursion * ...and ScriptEpilogue is never called, so js_PutCallObject never unsets that Call object's stack frame pointer! * now the Call object's stack frame pointer dangles, or points into newer frames, or whatever This doesn't look to be a problem in 192. See the putActivationObjects call that happens regardless whether js_Interpret succeeded or failed: http://hg.mozilla.org/releases/mozilla-1.9.2/file/c5b805f21466/js/src/jsinterp.cpp#l1386 It's only a recent problem likely introduced when we added ScriptEpilogue.
Attachment #501570 - Flags: review?(dvander)
Also not a problem in 191, which unconditionally called js_PutCallObject after calling js_Interpret.
Comment on attachment 501570 [details] [diff] [review] Patch Nice find.
Attachment #501570 - Flags: review?(dvander) → review+
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b9
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
How likely would a web attacker be able to pull off the conditions in comment 4? Is the impact here really "critical" or more "moderate"?
It's easy to pull off those conditions. What's harder is getting the now-dead call object to be traced. The recursion error happens early enough that the call object pointer never escapes into script, so either you need to keep it on the stack in a way that it gets reused, you have to "guess" what it is and get it on the stack somehow (easier said than done), or something. I don't know an easy way to do that, but I don't know there isn't an easy way. Given that the testcase here was consistently reproducible, I suspect there probably is an easy way. But that's only halfway: you still need to leverage the failed assertion further. It might or might not be possible to do that. But I suspect, given how many more slots you'd be tracing than would be okay, it's probably possible. I'd say this leans more to critical than moderate, but I could be mistaken. There's too much complexity of behavior here, none of it in "normal" semantics (well-defined by the relevant function interfaces, that is), to say without investigating a fair bit, I think.
Given the uncertainty bumping the severity down a notch to "high".
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:high?] fixed-in-tracemonkey
Fixed for a long time and not affecting old branches, opening this.
Automatically extracted testcase for this bug was committed: https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
You need to log in before you can comment on or make changes to this bug.