Closed
Bug 302439
(spandepfu)
Opened 19 years ago
Closed 19 years ago
spandep fu doesn't skip unused JSOP_TABLESWITCH jump table entries
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
People
(Reporter: brendan, Unassigned)
References
()
Details
(Keywords: js1.5)
Attachments
(2 files)
973 bytes,
patch
|
shaver
:
review+
brendan
:
approval1.8b4+
|
Details | Diff | Splinter Review |
85.18 KB,
text/plain
|
Details |
Easily fixed, dumb oversight. Surprised it hasn't come up. Clearly our tests and the real-world testcases (tellme.com, e.g.) have had only switches of density 1! /be
Reporter | ||
Comment 1•19 years ago
|
||
This is the canonical, regression-free, one-line fix to a crash bug. We want it everywhere. The only objection could be that it doesn't fix other bugs, but one bug at a time (and please report other bugs!). /be
Attachment #190798 -
Flags: review?(shaver)
Attachment #190798 -
Flags: approval1.8b4+
Attachment #190798 -
Flags: approval1.7.12?
Attachment #190798 -
Flags: approval-aviary1.0.7?
Reporter | ||
Updated•19 years ago
|
Flags: blocking1.8b4+
Flags: blocking1.7.12?
Flags: blocking-aviary1.0.7?
Reporter | ||
Updated•19 years ago
|
Alias: spandepfu
Updated•19 years ago
|
Flags: testcase?
Comment on attachment 190798 [details] [diff] [review] fix r=shaver.
Attachment #190798 -
Flags: review?(shaver) → review+
Reporter | ||
Comment 3•19 years ago
|
||
Reporter | ||
Comment 4•19 years ago
|
||
Someone please check in so I can stop worrying about this for 1.8b4. /be
Flags: testcase? → testcase+
Comment 5•19 years ago
|
||
I checked this fix into the trunk earlier today. Marking FIXED.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 6•19 years ago
|
||
*** Bug 304719 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Flags: testcase+ → testcase?
Reporter | ||
Comment 7•19 years ago
|
||
This bug's patch caused bug 308085. So much for simple one-line patches! My reviewers and I forgot that, per the comment in EmitSwitch: /* * Emit switchOp followed by switchSize bytes of jump or lookup table. * * If switchOp is JSOP_LOOKUPSWITCH or JSOP_TABLESWITCH, it is crucial * to emit the immediate operand(s) by which bytecode readers such as * BuildSpanDepTable discover the length of the switch opcode *before* * calling js_SetJumpOffset (which may call BuildSpanDepTable). It's * also important to zero all unknown jump offset immediate operands, * so they can be converted to span dependencies with null targets to * be computed later (js_EmitN zeros switchSize bytes after switchOp). */ So we can't EmitSpanDep only if off != 0 -- duh! Patch coming in bug 308085. /be
Reporter | ||
Updated•19 years ago
|
Attachment #190798 -
Flags: approval1.7.13?
Attachment #190798 -
Flags: approval-aviary1.0.8?
Reporter | ||
Updated•19 years ago
|
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?
Comment 8•19 years ago
|
||
Checking in regress-302439.js; /cvsroot/mozilla/js/tests/ecma_3/Statements/regress-302439.js,v <-- regress-302439.js initial revision: 1.1 done I tried reducing the test case, but kept losing the crash so this uses the original test source.
Flags: testcase? → testcase+
Comment 9•18 years ago
|
||
verified fixed 20060401 1.8.0.2, 1.8, 1.9a1 win/mac, 1.9a1 linux
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•