http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244403638.1244403731.21809.gz jsdbgapi.cpp: In function 'JSBool JS_EvaluateUCInStackFrame(JSContext*, JSStackFrame*, const jschar*, uintN, const char*, uintN, jsval*)': jsdbgapi.cpp:1268: error: cannot access JS_REQUIRES_STACK variable JSContext::fp jsdbgapi.cpp:1272: error: cannot access JS_REQUIRES_STACK variable JSContext::fp Blame for these lines points to bug 494235.
Created attachment 382074 [details] Crash stack I'm guessing this is the same problem I'm seeing even though the line number is off by one. I'm seeing it when using Venkman. > /* This API requires an active fp on cx, so fp2 can't go null here. */ I'm guessing it can go null. FAULTING_IP: js3250!JS_EvaluateUCInStackFrame+6f [e:\builds\moz2_slave\mozilla-central-win32-nightly\build\js\src\jsdbgapi.cpp @ 1273] 003d5d2c 8b4858 mov ecx,dword ptr [eax+58h] EXCEPTION_RECORD: ffffffff -- (.exr 0xffffffffffffffff) .exr 0xffffffffffffffff ExceptionAddress: 003d5d2c (js3250!JS_EvaluateUCInStackFrame+0x0000006f) ExceptionCode: c0000005 (Access violation) ExceptionFlags: 00000000 NumberParameters: 2 Parameter: 00000000 Parameter: 00000058 Attempt to read from address 00000058
(In reply to comment #1) > Created an attachment (id=382074) [details] > Crash stack Separate bug, please file it (nominate blocking), with a debug build's backtrace if possible. Thanks, /be
I'm not sure about the jsdbgapi invariants here. As far as I can tell, obtaining a JSStackFrame* can only happen through the JS_FrameIterator API. This leaves the trace. Then all the remaining dbgapi methods take a JSStackFrame* parameter. Can all these methods assume that we're no longer on trace? It's simple enough to js_LeaveTrace, but I'm not sure whether that's necessary.
We shouldn't add a js_LeaveTrace. What's the qualifier to use that says "we promise to be off-trace here, please don't turn red, oh dear static-analysis tbox"? /be
Created attachment 382175 [details] [diff] [review] fix Thanks to bsmedberg for irc advice. /be
http://hg.mozilla.org/tracemonkey/rev/27739a61e313 is a followup, reversing the order of the asserts so the analysis knows it's safe to assert cx->fp
I think this should be a blocker for reopening mozilla-central, but does not need to block 1.9.1
I've found a crasher filed at bug 496981. Could these be related?
(In reply to comment #9) > I've found a crasher filed at bug 496981. Could these be related? No, this bug is just about a false positive in the static analysis due to failure to annotate, which is now fixed in tracemonkey. Rob, if you could test Firebug on tracemonkey hourly builds that would be great. /be
This JS_ASSERT(cx->fp); makes Venkman crash in 1.9.1 SM debug builds for every breakpoint.
Why was this checked into the 1.9.1 branch? It isn't blocking, wanted, or approved...
(In reply to comment #15) > Why was this checked into the 1.9.1 branch? It isn't blocking, wanted, or > approved... ug. that must be mistake...
(In reply to comment #16) > (In reply to comment #15) > > Why was this checked into the 1.9.1 branch? It isn't blocking, wanted, or > > approved... > > ug. that must be mistake... must be *my* mistake, I meant to write.
The fix is debug-only and therefore mostly exempt from the normal rules... no harm done.
Harm definitely done -- we need to be able to debug 1.9.1-stream builds, which is why we take assertion and other debug-only fixes on maintenance stream. Can we get a fix for this?
Shaver, I don't understand. This *did* land on 1.9.1, even though it technically didn't have the proper approvals.
Yes, I mean that harm was done by the landing, since it took us from "static analysis tinderbox is red" to "can't use a JS debugger in a 1.9.1 debug build". We'll have to take bug 497998's patch somewhere to remedy that, hopefully after FF3.5 rc1 becomes FF3.5 :)