In at least some cases, a breakpoint on an 'if' statement alters the execution. That's not good. From Issue 1096: Adding breakpoint causes code to malfunction http://code.google.com/p/fbug/issues/detail?id=1096 Breakpoint on line 8 cause failure. 5 try { 6 throw new Error('Test Exception'); 7 } catch(ex) { 8 if(ex.fileName != null && ex.lineNumber != null) 9 alert('alert 1'); 10 else 11 alert('alert 2'); 12 } Issue 1096: Adding breakpoint causes code to malfunction http://code.google.com/p/fbug/issues/detail?id=1096 Breakpoint on line 20 causes bug 16 myLog('aObject.aString = "' + aObject.aString + '"'); 17 myLog('typeof(aObject.aString) = "' + typeof(aObject.aString) + '"'); 18 myLog('Equality Test: ' + (aObject.aString === "Hello World")); 19 myLog('Testing if aObject.aString("' + aObject.aString + '") === "Hello World"'); 20 if (aObject.aString === "Hello World") { 21 myLog('if says true'); 22 myLog('Equality Test: ' + (aObject.aString === "Hello World")); 23 } else { 24 myLog('if says false'); 25 myLog('Equality Test: ' + (aObject.aString === "Hello World")); 26 }

A test case using the 1096 example is here: http://fbug.googlecode.com/svn/chromebug/tests/jsdTests@johnjbarton.com/content/jsd/jsdIScript/bugzilla-463239.js The firefox extension: http://fbug.googlecode.com/svn/chromebug/tests/jsdTests@johnjbarton.com starts jsd and runs the test case. Tools -> Test Now or run firefox from the command line with -chrome chrome://jsdtest/content/testWebPage.html.

I'm curious if this is because of JITting. CC'ing Crowder for input.

I guess not since the original reports and tests were against FF3.0

Since 462704 got wanted1.9.1+ I'll ask for same since this one is way more important.

I'll try the blocking1.9.1 flag then

Any reduced js shell testcase using the shell's trap built-in? /be

I don't know the shell thing, but this test: http://fbug.googlecode.com/svn/chromebug/tests/jsdTests@johnjbarton.com/content/jsd/jsdIScript/bugzilla-463239.js is in a file tree that is kinda sorta similar to things I say in js tests somewhere.

Any progress? This bug is very embarrassing....

This bug is assigned to rginda, who is not active. If you want progress, get a real assignee. /be

Is this a regression?

I believe it's a regression from 3.0 to 3.1, yes. Brendan, do you have anybody that can work on this or knows the code? Crowder maybe?

(In reply to comment #11) > I believe it's a regression from 3.0 to 3.1, yes. comment #3 says the original tests and reports were against 3.0, which would indicate it didn't work there either, unless I misunderstand John's comment.

The earliest user report is Firefox 3.0.1 (2008071222) It took me a long time to figure out how to create a test case that did not require firebug.

This user report may be related. But it was against FF2! http://code.google.com/p/fbug/issues/detail?id=378

my mistake. I thought this was a much more recent thing.

testing w/ jsshell. js> t=18;function b463239(){try {throw new Error("T");}catch(ex){var c=ex.fileNa me&&ex.lineNumber;print(c?"+":"-")}};b463239();trap(b463239,t,"void(0)");b463239();untrap(b463239,t);b463239(); + + + js> t=21;function b463239(){try {throw new Error("T");}catch(ex){var c=ex.fileName&&ex.lineNumber;print(c?"+":"-")}}; b463239();trap(b463239,t,"void(0)");b463239();untrap(b463239,t);b463239(); + - + this is a bug in the engine.

so. I have no coverage near this range: (bug 365851) 11377:ef58dabbbe59..11813:7f8c737b196e (bug 416665) In my testing, it was already dead by the latter end. it was working in 11376:45530554f854 I updated to 11377 and applied the diff from 11812:11813, and then instead of crashing, i get the bad behavior. So either bug 365851 broke this part of traps, or the fix for bug 416665 did :) Either way. brendan wrote the patch :)

looks like you got some jsdb to work this over. brendan: does this sound possible?

fwiw, i have available: venkman, firebug, xpcshell, jsdb, and jsshell this was just jsshell. for testcases, if you can reduce it to jsshell, that's the one that the people who own spidermonkey are familiar w/. venkman/firebug have their own framesworks which sit on top of jsdI xpcshell is jsdI w/o those frameworks jsdI sits on top of jsd jsdb is a fairly straight wrapping of the jsd api jsd sits on top of jsdbgapi js just exposes a couple of basic jsdbgapi primitives via js basically if i can reproduce a problem in xpcshell, then it exonerates layout/xul, with jsdb, i can exonerate jsdI, and if i can reproduce it with js, I can exonerate jsd and leave this bug in this component. For this bug, crowder suggests that the property cache is the obvious related component.

Timeless: thanks for the shell reduction, but where is it? Can you attach it? The +\n+\n+\n ellipses in comment 16 and code fragments (or log messages?) between them leave me without enough info. /be

oh the +'s aren't ellipses, they're output from print(c?"+":"-") if you prefer T and F could be substituted: js> t=18;function b463239(){ try {throw new Error("T");}catch(ex){ var c=ex.fileName&&ex.lineNumber;print(c?"T":"F")} };b463239();trap(b463239,t,"void(0)");b463239();untrap(b463239,t);b463239(); T T T js> t=21;function b463239(){ try {throw new Error("T");}catch(ex){ var c=ex.fileName&&ex.lineNumber;print(c?"T":"F")} };b463239();trap(b463239,t,"void(0)");b463239();untrap(b463239,t);b463239(); T F T The goal of my paste was that there was a single line you could copy [js>] and 3 lines of output to check (+++/+-+). But of course bugzilla munched that horribly.

Taking this off Brendan's plate to take a swing at it.

So basically the bug here is that the exception doesn't include its "fileName" property because of the set trap. I think this should be pretty straightforward to fix: t=21; function b463239() { try { throw new Error("T"); } catch(ex) { var c = ex.fileName + ":" + ex.lineNumber; print(c); } } b463239(); trap(b463239,t,"void(0)"); b463239(); untrap(b463239,t); b463239(); Output: tmp.js:4 undefined:4 tmp.js:4

But how can something like this explain incorrect branching?

Incorrect branching might be a different bug; can you show me a testcase?

Actually, I'm wrong. The exception object itself is being built properly, but the property-cache is getting confused by the JSOP_TRAP. More soon.

(In reply to comment #25) > Incorrect branching might be a different bug; can you show me a testcase? The test case on comment 1 does not have exceptions.

The testcase from comment #1 is enormous, can you simplify?

? Are we looking at the same thing? It's two pages of JS. Most of it is logging to verify that it gets to the right state. Since the bug happens on a breakpoint, you need a jsdIScript to set the breakpoint. I guess you can get it from enumerateScripts instead of onScriptCreated.

So you're saying you can't simplify it?

I don't know what you want. If it were me I would put a four line function in an HTML file and run it under Firebug to set the breakpoint. Comment #1 was a lot of work to create a case that did not require Firebug. If you have a third idea of simple I need help understanding what it would be.

I'd love to have a testcase that works in the jsshell, just like the one timeless provided.

Sorry I don't know what jsshell is.

I worked the original bug report over. Very bizarre results. The statement "var y" must be present to trigger the bug. Breaking up the statement, adding more statements prevents the bug. function b463239(aObject) { var y = ('Equality Test: ' + (aObject.aString === "Hello World")); if (aObject.aString === 'Hello World') // set breakpoint here

Thanks for the simplified testcase. I think (without having tested, yet) this fits my theory though; here's why. If the property cache lookup returns a bogus result (ie., a different atom than "Hello World"), then the logic will naturally behave differently. The issue isn't so much that the branching behavior has changed, but that the data being referenced by the equality operator is bogus... so you end up on a different branch.

jjb, see https://developer.mozilla.org/en/Introduction_to_the_JavaScript_shell. Build with --enable-debug so you have access to the dis() function. See bug 476066 for an example of a jsshell trap testcase. Use the dis() function to find out which bytecode offsets are valid trap arguments, and then experiment to figure out which trap offset triggers the bug that Firebug experiences.

This is the slow version (ie., it should not be landed), just checking to see if I've got the idea... what do we want here, a static inline version of JS_GetTrapOpcode? This fixes the bug in timeless' testcase.

Eh, let's do this instead of worrying about inlining, etc.

Comment on attachment 360211 [details] [diff] [review] Better Woops, this has warnings, disregard.

Driving by: I think this idiom should be in an inline function. There are a few other places that should call it. I think Detecting should call it a few times.

Comment on attachment 360214 [details] [diff] [review] Better, and it builds Yes, we want a static inline GetOpFromPC helper that commons that code without adding out of line call overhead. /be

This adds an inline, js_GetOpcode, and uses it in several places.

My grepping for v1 didn't find expressions like `regs.pc[offset]`.

Comment on attachment 361538 [details] [diff] [review] v2 Argh, why all the jsdbgapi.h including? It should not be necessary to propagate that -- if nesting it in jsscript.h is not a good idea, suggest factoring JS_GetTrapOpcode into a js_GetTrapOpcode defined in jsscript.cpp, wrapped by the API entry point in jsdbgapi.cpp. /be

> Argh, why all the jsdbgapi.h including? ...Um, I have no idea what I was thinking. Just #including "jsdbgapi.h" from "jsscript.h" works fine.

Comment on attachment 362633 [details] [diff] [review] v3 One line looks like 105 chars, please wrap (&& and || go at end of line, other ops usually at start of continuation line): + JS_ASSERT(js_CodeSpec[js_GetOpcode(cx, script, regs.pc)].length == JSOP_CALL_LENGTH); /be

http://hg.mozilla.org/tracemonkey/rev/949d496b5a84 But see bug 479353 which this introduced.