Closed Bug 610026 Opened 10 years ago Closed 10 years ago
_BLOCKCHAIN processing does not handle JSOP _INDEXBASE* ops
Large programs are possible (need to fix a JSParseNode recycling leak first to get the demo to work). Maybe be s-s, marking that way for now. This is a regression from the patch for bug 535912. /be
The test fails without this patch as follows in a release build: ./js1_8_5/regress/regress-610026.js:60: Error: Assertion failed: got (new ReferenceError("x is not defined", "./js1_8_5/regress/regress-610026.js", 55)), expected 42 In an unpatched debug shell it fails like so: Assertion failure: invalid opcode for fast block chain access, at ../jsinterp.cpp:271 because of the unexpected JSOP_RESETBASE. We should really simplify our bytecode to use a single immediate format for short and long jumps, regular and "big" literal indexes, etc. It might not even slow the interpreter down. Followup fodder. /be
This deals with GetBlockChain in a different way.
Thanks Bill -- I will use that GetBlockChain approach (tried it but wanted to kill the too-many-overcomplicated-immediate-encodings bird, will defer that to another bug), merged with other changes and tests. The jsparse.cpp change is for yet another bug. /be
Attachment #489073 - Flags: review? → review?(wmccloskey)
Comment on attachment 489073 [details] [diff] [review] fix, v2 Great, thanks. It might be good to add a comment to js_AdvanceOverBlockchain along the lines of "watch out for INDEXBASE instructions".
Attachment #489073 - Flags: review?(wmccloskey) → review+
I renamed js_AdvanceOverBlockchain js::AdvanceOverBlockchainOp (the second lowercase-c in "Blockchain" is better with "Op" after to clarify this refers to JSOP_*BLOCKCHAIN; normal CamelCaps spelling would want BlockChain but that seemed less obviously about the two bytecodes). /be
Zapped a stray print, and commented the magic numbers. /be
Status: NEW → ASSIGNED
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.