Closed
Bug 524230
Opened 15 years ago
Closed 15 years ago
NJ merge: adjust asm_ld_imm assertion
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: graydon, Assigned: graydon)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Assembler::asm_ld_imm in NativeARM.cpp does the following: - Writes the literal to *(_nSlot++) - Writes an offset-load to the literal to *(--_nIns) Explanation: ------------ - _nSlot is at a lower address and grows upwards towards _nIns - _nIins is at a higher address and grows downwards towards _nSlot - they grow together, and can get as close as touching, but must not pass each other - the instruction being written is a 12-bit offset from "PC" packed into a 32-bit word - PC points to "current instruction + 8" - but offset is calculated from _nIns - 2. IOW offset is just (_nSlot - _nIns) Diagram: -------- imm load [____|____|____|____|____|____|____] ^ ^ nslot nins Difference in code: ------------------- Our code differs from Adobe's only in an assert: - NanoAssert(isS12(offset) && (offset < 0)); + NanoAssert(isS12(offset) && (offset < -8)); Clearly the first assert is not right, as the offset needs to be at least -8. The second assert seems a little too tight though, as I think an offset of exactly -8 would still function, and is probably achievable in rare cases. So I'd set it to <= -8. But I'm willing to hear an argument to the contrary. Any opinions?
Assignee | ||
Comment 1•15 years ago
|
||
Discussed and reviewed on IRC, set to <= -8, tested, seems ok. Worst case it's just a wrong assert and won't harm any production builds anyway. http://hg.mozilla.org/tracemonkey/rev/47fddf8e644c Adobe folks: please either absorb this back or tell us how our logic's off here :)
Whiteboard: fixed-in-tracemonkey
Comment 2•15 years ago
|
||
Sounds good to me, will change tamarin code.
Comment 3•15 years ago
|
||
sounds ok
Comment 4•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/47fddf8e644c
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•