430205 - jsdIScript.pcToLine() crashes FF3pre

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[John J. Barton]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042106 Minefield/3.0pre

Running this loop
----
     for (var i = 0; i <= 10*lineCount; i++)
     {
         var lineFromPC = script.pcToLine(i, this.pcmap_type)
         this.outerScriptLineMap[lineFromPC] = script;
         if (lineFromPC >= lineCount) break;
     }
---
crashes with a stack:
0  	js3250.dll  	js3250.dll@0x48968  	
1 	xul.dll 	jsdScript::PcToLine 	mozilla/js/jsd/jsd_xpc.cpp:1385
2 	xul.dll 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
3 	xul.dll 	XPCWrappedNative::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2369

http://crash-stats.mozilla.com/report/index/4436b3e8-1022-11dd-ac44-001cc45a2c28
4 times identical
The crash happens in a script with ~7700 lines as the PC gets to lines ~350. This jsdIScript is the "outer script", top-level, where the globals are defined (not that this matters for the crash I think).

I've never has pcToLine() crash on me, but then it is usually called with a pc from a stack frame, rather than a value I created.

Its a shame because it looks like pcToLine() is way faster than IsLineExecutable(). I'm trying to handle scripts with ~40kloc and with isLineExecutable() I can't get through the source before the run-away script timer goes off.

I don't know how to create a test case for this, all the apparatus of jsd has to be up.  Any chance the stack trace holds enough info?

Comment 1

[John J. Barton]

Marking firebug-p3, nice to have but, more important, very nice to know it won't be biting us later when pcToLine() gets called with frame.pc during debugging.

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Are there breakpoints set on |script| in this case?  Could be another JSOP_TRAP-vs-decompilation/etc. bug.

Comment 3

[timeless]

luser: does this build have symbols for js?

john: please try using windbg http://developer.mozilla.org/en/docs/How_to_get_a_stacktrace_with_WinDbg

lm (and !lmi) will tell you whether there are symbols for js3250 (!sym noisy before .reload /f would too). And assuming you have symbols, windbg sometimes can give better stack traces (!analyze -v -f).

fwiw, the public entrypoints jsdScript::PcToLine and JSD_GetClosestLine don't sanity check their input, which is essentially a violation of everyone else's APIs, so for now, please be a polite consumer and be very certain that the values you're passing in are valid :).

Comment 4

[(not currently active) Ted Mielczarek]

Yep, shaver just poked me about this crash. That address is just in the middle of nowhere in js3250.dll. This is the closest function:
FUNC 46d51 14 0 _SEH_epilog4

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

John: do you have a way to know if the PC is in range?  I'm not sure where "lineCount * 10" is coming from, and I would not be at all surprised to discover that we do unfortunate things if you ask us to look off the end of the script.  A patch to check that case and throw might be pretty easy, I'll try to whip that up today.

Could we end up in SEH from a bad pointer deref somewhere?  I confess I'm not sure exactly how it's triggered.

Comment 6

[timeless]

we only offer jsdScript::GetBaseLineNumber and jsdScript::GetLineExtent
http://mxr.mozilla.org/seamonkey/source/js/jsd/idl/jsdIDebuggerService.idl#941

I don't think that's sufficient to enable what he's trying to do.

especially given that a line might have thousands of operations.

sadly it doesn't look like JSD exposes anything relevant. Perhaps no one asked for it, or JSD thought that data was obvious (it isn't...).... jsdI is just a straight-forward reflection of JSD.

http://mxr.mozilla.org/seamonkey/source/js/src/jsscript.h#90

The data is private, so you probably need to add bits to:
http://mxr.mozilla.org/seamonkey/source/js/src/jsdbgapi.h

and then we can add bits to JSD and jsdI and ...

Comment 7

[John J. Barton]

(In reply to comment #2)
> Are there breakpoints set on |script| in this case?  Could be another
> JSOP_TRAP-vs-decompilation/etc. bug.

No, the only breakpoint was set on PC=0 and removed just before the code that crashes.  The code is called in the breakpoint handler if that matters.

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Yeah, and there's no JSOP_TRAP badness here.

Does this happen with 1.2a21?  If so, can you give manual STR, or even attach the test case?

Getting a stack out of windbg would be very helpful also, so that we're not just guessing about where the crash actually was.  Timeless' MDC link has great steps for it.

Comment 9

[John J. Barton]

(In reply to comment #3)
> fwiw, the public entrypoints jsdScript::PcToLine and JSD_GetClosestLine don't
> sanity check their input, which is essentially a violation of everyone else's
> APIs, so for now, please be a polite consumer and be very certain that the
> values you're passing in are valid :).

I don't think this is possible since there is no API giving the valid PC values. I am almost certainly giving invalid PC values, my only heuristic is to stop the iteration when the line count exceeds lineExtent.

Comment 10

[John J. Barton]

(In reply to comment #3)
> luser: does this build have symbols for js?
> 
> john: please try using windbg
> http://developer.mozilla.org/en/docs/How_to_get_a_stacktrace_with_WinDbg
> 
> lm (and !lmi) will tell you whether there are symbols for js3250 (!sym noisy
> before .reload /f would too). And assuming you have symbols, windbg sometimes
> can give better stack traces (!analyze -v -f).

I'll look in this later today.

Just to be clear, my goal is to insure that this isn't a bug that affects calls using jsdIStackFrame.pc that we've not hit yet.  Making the loop work would be a bonus, but not a show stopper. So if you have information that, eg the PC is not valid in the range between 0 and its maximum for a script (which we don't know), then we can stop now and save this bit of fun for Moz2.

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I strongly suspect you're over-reaching the script's extent, which isn't possible for jsdIStackFrame.pc, but a stack trace and some poking in the source server to see what the script's details are would tell the tale.

Comment 12

[timeless]

btw, once you actually get windbg up, you might as well have a way to build useful traces, run this before you start the code you have that will trigger the crash:

bp xul!jsdScript::PcToLine "kp 3;g"
bp jsd3250!JSD_GetClosestLine "kp 3;g"
bp js3250!JS_GetScriptLineExtent "kp 3;g"
bp js3250!JS_PCToLineNumber "kp 3;g"

that should give you enough breakpoints so that you can run and see where you are before you die. you can lop off the "kp 3;g" parts if you like, they're "give me 3 lines of a moderately useful stack trace and continue".

Comment 13

[John J. Barton]

(In reply to comment #12)
> btw, once you actually get windbg up, you might as well have a way to build
> useful traces, run this before you start the code you have that will trigger
> the crash:
> 
> bp xul!jsdScript::PcToLine "kp 3;g"
> bp jsd3250!JSD_GetClosestLine "kp 3;g"
> bp js3250!JS_GetScriptLineExtent "kp 3;g"
> bp js3250!JS_PCToLineNumber "kp 3;g"
> 
With these installed FF3 does not crash. Takes a heck of a long time to print tho ;-) 

Comment 14

[John J. Barton]

(In reply to comment #3)
> can give better stack traces (!analyze -v -f).
> 
STACK_TEXT:  
0012ddb8 608274a2 0034c7e0 00001e17 0361ac05 js3250!js_PCToLineNumber+0x45e85
0012ddcc 60649e03 0360dc40 0000016d 00000001 xul!jsdScript::PcToLine+0x2b [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\jsd\jsd_xpc.cpp @ 1385]
0012ddf0 60502fbe 0360dc40 0000001a 00000003 xul!NS_InvokeByIndex_P+0x27 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp @ 102]
0012de2c 604f6d69 0012dea0 0012dff8 00000003 xul!XPCWrappedNative::CallMethod+0x55e [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp @ 2371]

Comment 15

[John J. Barton]

See also bug 513556, a 64bit crash at js_PCToLineNumber

Comment 16

[Rob Campbell [:rc] (:robcee)]

John, can you still reproduce this?

Comment 17

[Jan de Mooij [:jandem]]

Please file a new bug if this is still a problem.