Skip to content

Commit

Permalink
fix insnCodeGen::modifyData's 64-bit conversion
Browse files Browse the repository at this point in the history
On RHEL6 with a prelinked `/lib64/libc-2.12.so`, all of tests involving
fork instrumentation were getting SIGSEGV in the mutatee.  This worked
in 9.1, and it also works fine after `prelink -u` to undo libc.  Using
git-bisect found 2b86eb4 as the point of regression.

It seems prelink ends up with libc sitting far away from the relocation
buffer, more than a 32-bit displacement, so `insnCodeGen::modifyData`
decides to rewrite that to a 64-bit immediate.  To do this, it has to
emit additional instructions first.  But after the commit above, part of
the rewritten instruction has already been written when we're trying to
emit those extras, and things gets clobbered.

This patch emits those preamble instructions first, before any part of
the newly rewritten instruction is copied out.
  • Loading branch information
cuviper committed Aug 30, 2016
1 parent a032945 commit 4ca18d9
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions dyninstAPI/src/codegen-x86.C
Expand Up @@ -1196,10 +1196,6 @@ bool insnCodeGen::modifyData(Address targetAddr, instruction &insn, codeGen &gen

/* get the prefix count */
size_t pref_count = instruct.getSize();

/* copy the prefix */
memcpy(newInsn, origInsn, pref_count);
newInsn += pref_count;
origInsn += pref_count;

/* Decode the opcode */
Expand All @@ -1208,35 +1204,38 @@ bool insnCodeGen::modifyData(Address targetAddr, instruction &insn, codeGen &gen

/* Calculate the amount of opcode bytes */
size_t opcode_len = instruct.getSize() - pref_count;

/* Copy the opcode bytes */
memcpy(newInsn, origInsn, opcode_len);
newInsn += opcode_len;
origInsn += opcode_len;

/* Get the value of the Mod/RM byte */
unsigned char mod_rm = *origInsn;
origInsn++;
unsigned char mod_rm = *origInsn++;

#if defined(arch_x86_64)
if (!is_disp32(newDisp+insnSz) && !is_addr32(targetAddr))
{
// Case C: replace with 64-bit.
// This preamble must come before any writes to newInsn!
is_data_abs64 = true;
#if defined(arch_x86_64)
pointer_reg = (mod_rm & 0x38) != 0 ? 0 : 3;
SET_PTR(newInsn, gen);
emitPushReg64(pointer_reg, gen);
emitMovImmToReg64(pointer_reg, targetAddr, true, gen);
REGET_PTR(newInsn, gen);
}
#endif

/* copy the prefix and opcode bytes */
memcpy(newInsn, origInsnStart, pref_count + opcode_len);
newInsn += pref_count + opcode_len;

if (is_data_abs64)
{
// change ModRM byte to use [pointer_reg]: requires
// us to change last three bits (the r/m field)
// to the value of pointer_reg

mod_rm = (mod_rm & 0xf8) | pointer_reg;
/* Set the new ModR/M byte of the new instruction */
*newInsn++ = mod_rm;
#endif
} else if (is_disp32(newDisp + insnSz))
{
/* Instruction can remain a 32 bit instruction */
Expand Down

0 comments on commit 4ca18d9

Please sign in to comment.