JS_REQUIRES_STACK violation in JS_EvaluateUCInStackFrame

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
blocker
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

({fixed1.9.1, regression})

Trunk
x86
Linux
fixed1.9.1, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
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.
Flags: blocking1.9.1?
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[0]: 00000000
   Parameter[1]: 00000058
Attempt to read from address 00000058
(Assignee)

Comment 2

9 years ago
(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

Comment 3

9 years ago
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.
(Assignee)

Comment 4

9 years ago
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
(Assignee)

Comment 5

9 years ago
Created attachment 382175 [details] [diff] [review]
fix

Thanks to bsmedberg for irc advice.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #382175 - Flags: review?(mrbkap)

Updated

9 years ago
Attachment #382175 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/tracemonkey/rev/2d8dd0494e64

/be
Whiteboard: fixed-in-tracemonkey

Comment 7

9 years ago
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

Comment 8

9 years ago
I think this should be a blocker for reopening mozilla-central, but does not need to block 1.9.1
Flags: blocking1.9.1? → blocking1.9.1-
I've found a crasher filed at bug 496981. Could these be related?
(Assignee)

Comment 10

9 years ago
(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
wilco

Comment 12

9 years ago
http://hg.mozilla.org/mozilla-central/rev/2d8dd0494e64
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 13

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fc298bb83af7
Keywords: fixed1.9.1

Comment 14

9 years ago
This 
  JS_ASSERT(cx->fp);
makes Venkman crash in 1.9.1 SM debug builds for every breakpoint.

Updated

9 years ago
Blocks: 498232
Why was this checked into the 1.9.1 branch? It isn't blocking, wanted, or approved...

Comment 16

9 years ago
(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...

Comment 17

9 years ago
(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.

Comment 18

9 years ago
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?

Comment 20

9 years ago
Shaver, I don't understand. This *did* land on 1.9.1, even though it technically didn't have the proper approvals.
(Assignee)

Comment 21

9 years ago
Fixed by patch in bug 497998.

/be
Depends on: 497998
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 :)
You need to log in before you can comment on or make changes to this bug.