[webkit-reviews] review granted: [Bug 101909] Patching of jumps to stubs should use jump replacement rather than branch destination overwrite : [Attachment 173757] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 101909: Patching of jumps to stubs should use jump replacement rather than
branch destination overwrite
https://bugs.webkit.org/show_bug.cgi?id=101909

Attachment 173757: the patch
https://bugs.webkit.org/attachment.cgi?id=173757&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=173757&action=review


r=me

> Source/JavaScriptCore/assembler/X86Assembler.h:1922
> +#if CPU(X86_64)
> +    static void revertJumpTo_movq_i64r(void* instructionStart, int64_t imm,
RegisterID dst)
> +    {
> +	   const int rexBytes = 1;
> +	   const int opcodeBytes = 1;
> +	   ASSERT(rexBytes + opcodeBytes <= maxJumpReplacementSize());
> +	   uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
> +	   ptr[0] = PRE_REX | (1 << 3) | (dst >> 3);
> +	   ptr[1] = OP_MOV_EAXIv | (dst & 7);
> +	   
> +	   union {
> +	       uint64_t asWord;
> +	       uint8_t asBytes[8];
> +	   } u;
> +	   u.asWord = imm;
> +	   for (unsigned i = rexBytes + opcodeBytes; i <
static_cast<unsigned>(maxJumpReplacementSize()); ++i)
> +	       ptr[i] = u.asBytes[i - rexBytes - opcodeBytes];
> +    }
> +#endif
> +    
> +    static void revertJumpTo_cmpl_im_force32(void* instructionStart, int32_t
imm, int offset, RegisterID dst)
> +    {
> +	   ASSERT_UNUSED(offset, !offset);
> +	   const int opcodeBytes = 1;
> +	   const int modRMBytes = 1;
> +	   ASSERT(opcodeBytes + modRMBytes <= maxJumpReplacementSize());
> +	   uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
> +	   ptr[0] = OP_GROUP1_EvIz;
> +	   ptr[1] = (X86InstructionFormatter::ModRmMemoryNoDisp << 6) |
(GROUP1_OP_CMP << 3) | dst;
> +	   union {
> +	       uint32_t asWord;
> +	       uint8_t asBytes[4];
> +	   } u;
> +	   u.asWord = imm;
> +	   for (unsigned i = opcodeBytes + modRMBytes; i <
static_cast<unsigned>(maxJumpReplacementSize()); ++i)
> +	       ptr[i] = u.asBytes[i - opcodeBytes - modRMBytes];
> +    }

This would be a good thing to run by Gavin when he's back.