This is... strange. I get the following crash when loading a web page with firebug enabled: Assertion failure: *pc == JSOP_GETARG, at /home/sfink/src/TM-singlestep/js/src/jsopcode.cpp:4994 Oddly, this *only* happens if I am connected to the network over a wired ethernet connection. If I am on wireless or off the net entirely, I do not see the crash. The code involved seems to be associated with Sync, which explains why I do not see the crash when off the network, but I don't know why it matters whether I'm using a wired ethernet vs wireless. I will attach a stack trace, but basically what's happening is that JSD is trying to decompile a script that is still trapped. Specifically, *pc == JSOP_TRAP, which is why the assertion is failing, and the "real" opcode is indeed JSOP_GETARG: (gdb) p JS_GetTrapOpcode(jp->sprinter.context, script, pc) $80 = JSOP_GETARG

The easiest way to fix this would probably be to use js_UntrapScriptCode (something like that) at the top of the function. DecompileExpression does this, that would probably be a good model. This introduces a bit of redundant work since other functions also untrap the code. AutoScriptRetrapper fixes this, but that's currently in the methodjit folder.

Yup, the js_Disassemble* and paths are missing an untrap. They all end up in js_Disassemble1, so I put it there. Note that although js_Disassemble1 appears to not use *pc (except at the very beginning), it really does via things like GetJumpOffset(). js_DecompileFunction also needs an untrap (that's what this bug hit). I did it with an RAII class called AutoScriptUntrapper. It differs from Retcon's AutoScriptRetrapper in that it only removes the traps and switches the script to use the new jsbytecode array, so it doesn't need to retrap like AutoScriptRetrapper does. Where would I put tests for this stuff?

Requesting blocking beta9. JSD will trigger random crashes without it.

Comment on attachment 497876 [details] [diff] [review] untrap in js_Dissassemble1 and js_DecompileFunction I'm a little uneasy modifying the code value of the script -- a lot of the engine depends on being able to hang on to PCs either as pointers or as (pc - code) offsets, and AutoScriptUntrapper (temporarily) violates that invariant. Would it be possible to use a solution similar to the others that doesn't involve modifying the script itself?

Huh? I used that approach because I thought it was already in use. The previous code was: - if (code != oldcode) { - jp->script->code = code; - jp->script->main = code + (oldmain - oldcode); - pc = code + (pc - oldcode); - } and - code = js_UntrapScriptCode(cx, script); - if (code != oldcode) { - script->code = code; - script->main = code + (oldmain - oldcode); - pc = code + (pc - oldcode); - }

Comment on attachment 497876 [details] [diff] [review] untrap in js_Dissassemble1 and js_DecompileFunction Nevermind, I was thinking of a different place in the code base when I read the replacement. This looks good. I would leave: >- MUST_FLOW_THROUGH("out"); That line in though, since it's still useful for the static analyzer (it would prevent a leak of pcstack).

I removed that MUST_FLOW_THROUGH("out") because pcstack has not yet been allocated. There is still a MUST_FLOW_THROUGH immediately after pcstack is successfully allocated. (Previously, the out: label also restored script->code, which is why the removed statement was there.)

Alright, disregard that as well.

And: http://hg.mozilla.org/tracemonkey/rev/df86d5999fef (I screwed up opt builds)