Closed Bug 633069 Opened 11 years ago Closed 11 years ago

"Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL", including in test_form_submission_cap2.html


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
blocking2.0 --- .x+


(Reporter: jruderman, Assigned: billm)



(4 keywords, Whiteboard: [fixed-in-tracemonkey])


(3 files)

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.
Attached file stack trace
blocking2.0: --- → ?
Thanks, Jesse! I think I know what this is.
Assignee: general → wmccloskey
blocking2.0: ? → .x
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.
Group: core-security

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.
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
Blocks: 438871
Summary: "Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL" → "Assertion failure: JS_THREAD_DATA(cx)->profilingCompartment == NULL", including in test_form_submission_cap2.html
Whiteboard: [orange]
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.
Attached patch fixSplinter Review
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)
Attachment #512214 - Flags: review?(dvander) → review?(gal)
Attachment #512214 - Flags: review?(gal) → review+
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.
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 634436
Whiteboard: [orange][fixed-in-tracemonkey] → [fixed-in-tracemonkey]
Filter on qa-project-auto-change:

Bug in removed tracer code, setting in-testsuite- flag.
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.