Currently AbstractMacroAssembler::CachedTempRegister() are initialized with a value of 0 and that the contents are valid. Instead they should be mark as invalid as we don't know the prior contents before the first use.
<rdar://problem/16214527>
Created attachment 225738 [details] Patch
Comment on attachment 225738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:881 > + , m_validBit(0 << static_cast<unsigned>(registerID)) Why not just "m_validBit(0)”?
(In reply to comment #3) > (From update of attachment 225738 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > > > Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:881 > > + , m_validBit(0 << static_cast<unsigned>(registerID)) > > Why not just "m_validBit(0)”? Actually, the is the wrong value to clear. Updated patch coming.
Comment on attachment 225738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > Source/JavaScriptCore/ChangeLog:12 > + - Changed the setting of the valid bit to 0 (not valid) in the constructor as > + we don't know the contents of the register at the entry to the block we > + are going to generate code for. Are you assuming that all JITs -- including the baseline JIT, the DFG, and the CSS selector JIT -- instantiate a new MacroAssembler at the head of each basic block?
Created attachment 225741 [details] Updated patch Updated after Mark caught that I was invalidating incorrectly. (In reply to comment #5) > (From update of attachment 225738 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > > > Source/JavaScriptCore/ChangeLog:12 > > + - Changed the setting of the valid bit to 0 (not valid) in the constructor as > > + we don't know the contents of the register at the entry to the block we > > + are going to generate code for. > > Are you assuming that all JITs -- including the baseline JIT, the DFG, and the CSS selector JIT -- instantiate a new MacroAssembler at the head of each basic block? Poor choice of words. I changed "block" to "code". This fix has nothing to do with basic blocks. Whenever we create a MacroAssembler, we need to start with the cached register values as invalid. In the case that triggered this issue, we were generating small stubs and used the temporary registers to materialize constants that fit in 16 bits.
Committed r165038: <http://trac.webkit.org/changeset/165038>
Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue.