Bugzilla@Mozilla – Bug 558618
TM: Crash [@ js_ValueToString] - Segfault when accessing out-of-range arguments on trace
Last modified: 2010-07-20 16:43:29 PDT
Summon comment box
/* vim: set ts=4 sw=4 tw=99 et: */ function f() { this.search = function(a, b, c) { arguments[3] = { } arguments.length = 4; for (var i = 0; i < 100; i++) { print(arguments[3]); } } } var o = new f(); o.search({x: -1, y: -1, w: 100600, h: 100600});
Created attachment 438309 [details] [review] fix basically another variant of bug 554670
Can you explain to me why we crash? I remember that we made this trace for a reason, so we are going to break some insane benchmark code.
We crash because the recorder state does not match the interpreter state. The interpreter is placing an object on the stack and the tracer is placing a void.
What object and why?
Is this the { } object there? Can we access that from trace?
When the JIT goes to box up the stack for Print(), it applies the interpreter's type (object) to the value in the JIT frame, which is an unboxed JSVAL_VOID - the result is Print() gets a vp with a "2" in it, NULL tagged as double. We don't access overridden arguments anywhere on trace, as comment #1 hinted this is a variant of a similar case that was fixed recently: > var a = arguments; > a[3] = { }; > a.length = 4; It's probably possible to trace it if we treat it as a generic GETELEM or something, but that would be a big patch. At least for 1.9.2 I'd prefer to just not trace it.
Comment on attachment 438309 [details] [review] fix Double check with sayrer that this doesn't regress that weird argc overshooting benchmark.
try this in a browser with the peacekeeper tests.
Giving this the same status (wanted, not blocking) as bug 554670, renominate for blocking if you feel that's inappropriate!
autoBisect confirms that this is probably related to bug 453730: The first bad revision is: changeset: 30020:c76558a87dd9 user: David Mandelin date: Wed Jul 08 11:16:41 2009 -0700 summary: Bug 453730: trace JSOP_ARGUMENTS, r=gal
Stack: Exception Type: EXC_BAD_ACCESS (SIGBUS) Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000000 Crashed Thread: 0 Dispatch queue: com.apple.main-thread Thread 0 Crashed: Dispatch queue: com.apple.main-thread 0 js-opt-32-tm-darwin 0x000dbd13 js_ValueToString + 211 1 js-opt-32-tm-darwin 0x00004406 Print(JSContext*, unsigned int, long*) + 166 2 ??? 0x003d2f84 0 + 4009860 3 js-opt-32-tm-darwin 0x00127480 js::ExecuteTree(JSContext*, js::TreeFragment*, unsigned int&, js::VMSideExit**) + 736 4 js-opt-32-tm-darwin 0x001427c8 js::MonitorLoopEdge(JSContext*, unsigned int&, js::RecordReason) + 1064 5 js-opt-32-tm-darwin 0x00062533 js_Interpret + 52291 6 js-opt-32-tm-darwin 0x00065653 js_Execute + 531 7 js-opt-32-tm-darwin 0x0000f33c JS_ExecuteScript + 60 8 js-opt-32-tm-darwin 0x00004daf Process(JSContext*, JSObject*, char*, int) + 1647 9 js-opt-32-tm-darwin 0x00008dea main + 1626 10 js-opt-32-tm-darwin 0x000028dd _start + 208 11 js-opt-32-tm-darwin 0x0000280c start + 40
Created attachment 440436 [details] [review] fix branch fix. does not regress peacekeeper.
http://hg.mozilla.org/tracemonkey/rev/7dcbfdfd7f48 I don't think this is exploitable on x86 because the low 32-bits will always be 0x2, and it'll act like a NULL deref/correctness bug if it escapes. On x64 the high 32-bits will be garbage. Well, at the least it's a pretty trivial crash to hit and experiment with so I won't mind if the bug stays closed/peach-coloured.
http://hg.mozilla.org/mozilla-central/rev/7dcbfdfd7f48
Comment on attachment 440436 [details] [review] fix a=LegNeato for 1.9.2.6. Please land this on mozilla-1.9.2 default.
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e4bf0cdd829e
Bustage fix (caused by the fact that I transplanted instead of pushing the other patch in the bug): http://hg.mozilla.org/releases/mozilla-1.9.2/rev/02eb574eb149
I want to confirm in the bug (because it is unclear)...is 1.9.1 affected? If so we'll mark it as such.
err, I mean we'll mark it as unaffected if this is not an issue on 1.9.1.
1.9.1 is not affected.
Thanks so much for the confirmation!
What is the STR for this for verification in 1.9.2? I tried the code in comment 0 in a JS Shell but it didn't crash (just gave me a bunch of objects).