Closed Bug 614907 Opened 9 years ago Closed 9 years ago
The shape guard in Bind
Name is not linked to the exit sequence .
http://hg.mozilla.org/tracemonkey/file/2000c21b7910/js/src/methodjit/PolyIC.cpp#l1551 The 'shapeTest' jump is not linked, so the shape guard in the BindName stub never actually does anything. On x86 and amd64, an unpatched branch points to the next instruction, so this fails silently. On ARM, an unpatched branch jumps to an invalid address. I have produced a patch for this as part of the PIC porting effort [bug 588021]: http://hg.mozilla.org/users/cleary_mozilla.com/tm-arm-pic-mq/file/32a00b4bdfc9/fix-bindname-stub-shape-jump It might be worth pushing the fix before PICs are ready, so I'm raising a separate bug. (Note that I don't have a test-case that catches this problem.) We should probably also consider making the default jump on x86 and amd64 branch to an invalid address, so we can catch this kind of error more easily. Is this feasible? The jumps are relative and the code location isn't known at assemble-time, so making the jumps fail reliably might be tricky. A jump-to-self might work. Infinite loops can be a nuisance but they're obvious in a debugger, at least.
Attachment #493344 - Flags: review?(dvander)
Comment on attachment 493344 [details] [diff] [review] Link the bindname stub shape guards to the exit sequence. Nice catch. Yeah, I think that's a great idea, we've made these mistakes a few times and they're always a pain to track down. It'd be really nice if a Jump's destructor could assert that it was patched. It might not be too hard if in debug mode a Jump also stored its assembler.
Attachment #493344 - Flags: review?(dvander) → review+
blocking2.0: --- → final+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.