Closed Bug 633069 Opened 9 years ago Closed 9 years ago
"Assertion failure: JS
_THREAD _DATA(cx)->profiling Compartment == NULL", including in test _form _submission _cap2 .html
This testcase triggers an assertion in MonitorTracePoint: Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL, at js/src/jstracer.cpp:17038 This assertion was added as part of a large patch: changeset: ee3a4f429942 user: Bill McCloskey date: Tue Feb 01 10:18:06 2011 -0800 summary: Bug 623297 - Make JS_TRACE_MONITOR more robust by distinguishing callers (r=gal) Security-sensitive for now because bug 623297 was security-sensitive.
Thanks, Jesse! I think I know what this is.
Assignee: general → wmccloskey
Bill, we take a patch for this I think if you can fix is quickly.
What's the security implications of this assert? Compartment leakage? exploitable crash?
I'm 90% sure that this is not security sensitive. I don't think it will lead to a crash or any sort of compartment problems. We are breaking an invariant that I had hoped to preserve (that we don't do nested profiling), but I don't think we depend on this invariant for correctness. I'd rather not take out the assertion, since it's much harder to reason about control flow in the tracer if we can't rely on the invariant. There's a simple one-line fix that causes the invariant to be preserved. However, the fix regresses SunSpider. I'm working on figuring out how to fix it without messing up SS. Bottom line: shouldn't block; probably not security sensitive.
Opening this up based on the above comment, and discussion with Bill.
http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1297477875.1297478604.6242.gz Pretty much more or less similarish stack leading up to the same assertion, I'm hoping it'll manage to stay a very rarely intermittent while running test_form_submission_cap2.html, so I won't have to take over your bug and fill it with tbplbot spam :)
Sorry, looks like someone made it pretty common starting yesterday, here comes the spam. http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1297556299.1297556886.31142.gz Rev3 MacOSX Snow Leopard 10.6.2 tracemonkey debug test mochitests-5/5 on 2011/02/12 16:18:19 s: talos-r3-snow-026 7691 INFO TEST-PASS | /tests/toolkit/components/satchel/test/test_form_submission_cap2.html | checking unsaved value 60 Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL, at /builds/slave/tm-osx64-dbg/build/js/src/jstracer.cpp:6500
Summary: "Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL" → "Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL", including in test_form_submission_cap2.html
And that first one being Rev3 MacOSX Leopard 10.5.8 tracemonkey debug test mochitests-5/5 on 2011/02/11 18:31:15 s: talos-r3-leopard-011 apparently made it pretty common only on OS X.
This fixes the fuzzbug. I'm pretty sure it should also fix the orange. The problem is that we were asserting that we don't profile while we're recording or on trace. However, the lack of a profiler abort in JaegerShot causes us to violate this property. The easy fix would be to stick the abort in JaegerShot. However, that regresses our SunSpider score. It seems like we get the best performance if we allow outer profiler invocations to proceed and to abort inner profiler invocations (the opposite of what we do for recording). This is what this patch does. I've eliminated the profiler abort in Interpret. Instead, when we're about to run the profiler, the patch checks if we're already profiling and doesn't profile if we are. I wasn't sure how to deal with the case of recording or running on trace while profiling. These seem safe, but to be conservative, the patch aborts profiling whenever we're about to do either of these things. We can loosen these restrictions later if they hurt performance. (They are already looser than what is happening now on trunk.) Performance on all the usual benchmarks seems to be unaffected.
Attachment #512214 - Flags: review?(dvander)
9 years ago
Attachment #512214 - Flags: review?(dvander) → review?(gal)
Whiteboard: [orange] → [orange][fixed-in-tracemonkey]
Those were all from earlier in the day, we must have been backed up and suddenly a whole lot of run finished.
cdleary-bot mozilla-central merge info: http://hg.mozilla.org/mozilla-central/rev/0d78f2804085
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [orange][fixed-in-tracemonkey] → [fixed-in-tracemonkey]
Filter on qa-project-auto-change: Bug in removed tracer code, setting in-testsuite- flag.
You need to log in before you can comment on or make changes to this bug.