back jump not detected in node

[honggyukim]

$ uftrace record -P v8::internal::Heap::CreateFillerObjectAt --no-libcall ./node -e ''

back jump not detected

Thread 2.1 "node" received signal SIGILL, Illegal instruction.
0x00005555562f0aca in v8::internal::Heap::CreateFillerObjectAt(unsigned long, int, v8::internal::ClearFreedMemoryMode) ()

 1├>Dump of assembler code for function _ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE:
 2│    0x00005555562f0ac0 <+0>:     endbr64
 3│    0x00005555562f0ac4 <+4>:     call   0x5555578eeff0
 4│    0x00005555562f0ac9 <+9>:     nop    DWORD PTR [rax+0x0]
 5│    0x00005555562f0ad0 <+16>:    sub    rdi,0xd2b8
 6│    0x00005555562f0ad7 <+23>:    cmp    edx,0x8
 7│    0x00005555562f0ada <+26>:    je     0x5555562f0b40 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+128>
 8│    0x00005555562f0adc <+28>:    cmp    edx,0x10
 9│    0x00005555562f0adf <+31>:    je     0x5555562f0b20 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+96>
10│    0x00005555562f0ae1 <+33>:    mov    rax,QWORD PTR [rdi+0x1f0]
11│    0x00005555562f0ae8 <+40>:    mov    QWORD PTR [rsi],rax
12│    0x00005555562f0aeb <+43>:    mov    rax,rdx
13│    0x00005555562f0aee <+46>:    shl    rax,0x20
14│    0x00005555562f0af2 <+50>:    mov    QWORD PTR [rsi+0x8],rax
15│    0x00005555562f0af6 <+54>:    test   ecx,ecx
16│    0x00005555562f0af8 <+56>:    jne    0x5555562f0ac8 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+8>

[honggyukim]

The original instructions before dynamic patching looks as follows.

0000000000ef0ac0 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE>:
  ef0ac0:       f3 0f 1e fa             endbr64
  ef0ac4:       85 d2                   test   %edx,%edx
  ef0ac6:       75 08                   jne    ef0ad0 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+0x10>
  ef0ac8:       c3                      ret
  ef0ac9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)
  ef0ad0:       48 81 ef b8 d2 00 00    sub    $0xd2b8,%rdi
  ef0ad7:       83 fa 08                cmp    $0x8,%edx
  ef0ada:       74 64                   je     ef0b40 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+0x80>
  ef0adc:       83 fa 10                cmp    $0x10,%edx
  ef0adf:       74 3f                   je     ef0b20 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+0x60>
  ef0ae1:       48 8b 87 f0 01 00 00    mov    0x1f0(%rdi),%rax
  ef0ae8:       48 89 06                mov    %rax,(%rsi)
  ef0aeb:       48 89 d0                mov    %rdx,%rax
  ef0aee:       48 c1 e0 20             shl    $0x20,%rax
  ef0af2:       48 89 46 08             mov    %rax,0x8(%rsi)
  ef0af6:       85 c9                   test   %ecx,%ecx
  ef0af8:       75 ce                   jne    ef0ac8 <_ZN2v88internal4Heap20CreateFillerObjectAtEmiNS0_20ClearFreedMemoryModeE+0x8>

The ret instruction was patched, which is weird.

ef0ac8:       c3                      ret

[honggyukim]

The following fixes the problem.

diff --git a/arch/x86_64/mcount-insn.c b/arch/x86_64/mcount-insn.c
index 4097f753..3cad90c9 100644
--- a/arch/x86_64/mcount-insn.c
+++ b/arch/x86_64/mcount-insn.c
@@ -40,6 +40,7 @@ enum fail_reason {
        INSTRUMENT_FAIL_RELJMP = (1U << 1),
        INSTRUMENT_FAIL_RELCALL = (1U << 2),
        INSTRUMENT_FAIL_PIC = (1U << 3),
+       INSTRUMENT_FAIL_RET = (1U << 4),
 };

 enum branch_group {
@@ -59,6 +60,9 @@ void print_instrument_fail_msg(int reason)
        if (reason & INSTRUMENT_FAIL_PIC) {
                pr_dbg3("prologue has PC-relative addressing\n");
        }
+       if (reason & INSTRUMENT_FAIL_RET) {
+               pr_dbg3("prologue has return instruction\n");
+       }
 }

 static int x86_reg_index(int capstone_reg)
@@ -461,6 +465,14 @@ static int check_instrumentable(struct mcount_disasm_engine *disasm, cs_insn *in
                        check_branch = OP_GROUP_CALL;
                else if (detail->groups[i] == CS_GRP_JUMP)
                        check_branch = OP_GROUP_JMP;
+               else if (detail->groups[i] == CS_GRP_RET)
+                       return INSTRUMENT_FAIL_RET;
+               else if (detail->groups[i] == CS_GRP_INT ||
+                        detail->groups[i] == CS_GRP_IRET ||
+                        detail->groups[i] == CS_GRP_PRIVILEGE ||
+                        detail->groups[i] == CS_GRP_BRANCH_RELATIVE) {
+                       return INSTRUMENT_FAIL_NO_DETAIL;
+               }
        }

        x86 = &insn->detail->x86;

/// Common instruction groups - to be consistent across all architectures.
typedef enum cs_group_type {
	CS_GRP_INVALID = 0,  ///< uninitialized/invalid group.
	CS_GRP_JUMP,    ///< all jump instructions (conditional+direct+indirect jumps)
	CS_GRP_CALL,    ///< all call instructions
	CS_GRP_RET,     ///< all return instructions
	CS_GRP_INT,     ///< all interrupt instructions (int+syscall)
	CS_GRP_IRET,    ///< all interrupt return instructions
	CS_GRP_PRIVILEGE,    ///< all privileged instructions
	CS_GRP_BRANCH_RELATIVE, ///< all relative branching instructions
} cs_group_type;

https://github.com/capstone-engine/capstone/blob/3aff9546cef55c4b85d846fc64959806f584edfe/include/capstone/capstone.h#L244-L254

[honggyukim]

We better use switch statement for detail->groups[i] handling.

[honggyukim]

diff --git a/arch/x86_64/mcount-insn.c b/arch/x86_64/mcount-insn.c
index 4097f753..95ededd7 100644
--- a/arch/x86_64/mcount-insn.c
+++ b/arch/x86_64/mcount-insn.c
@@ -40,6 +40,7 @@ enum fail_reason {
        INSTRUMENT_FAIL_RELJMP = (1U << 1),
        INSTRUMENT_FAIL_RELCALL = (1U << 2),
        INSTRUMENT_FAIL_PIC = (1U << 3),
+       INSTRUMENT_FAIL_RET = (1U << 4),
 };

 enum branch_group {
@@ -59,6 +60,9 @@ void print_instrument_fail_msg(int reason)
        if (reason & INSTRUMENT_FAIL_PIC) {
                pr_dbg3("prologue has PC-relative addressing\n");
        }
+       if (reason & INSTRUMENT_FAIL_RET) {
+               pr_dbg3("prologue has return instruction\n");
+       }
 }

 static int x86_reg_index(int capstone_reg)
@@ -457,10 +461,24 @@ static int check_instrumentable(struct mcount_disasm_engine *disasm, cs_insn *in
        detail = insn->detail;

        for (i = 0; i < detail->groups_count; i++) {
-               if (detail->groups[i] == CS_GRP_CALL)
+               switch (detail->groups[i]) {
+               case CS_GRP_CALL:
                        check_branch = OP_GROUP_CALL;
-               else if (detail->groups[i] == CS_GRP_JUMP)
+                       break;
+               case CS_GRP_JUMP:
                        check_branch = OP_GROUP_JMP;
+                       break;
+               case CS_GRP_RET:
+               case CS_GRP_IRET:
+                       fprintf(stderr, "RET\n");
+                       break;
+                       return INSTRUMENT_FAIL_RET;
+               case CS_GRP_PRIVILEGE:
+               case CS_GRP_BRANCH_RELATIVE:
+                       return INSTRUMENT_FAIL_NO_DETAIL;
+               default:
+                       /* do nothing for legit instructions. */
+               }
        }

        x86 = &insn->detail->x86;
@@ -608,6 +626,7 @@ int disasm_check_insns(struct mcount_disasm_engine *disasm, struct mcount_dynami
                };

                status = check_instrumentable(disasm, &insn[i]);
+               pr_dbg4("check_instrumentable: %s %s (status = %d)\n", insn[i].mnemonic, insn[i].op_str, status);
                if (status > 0)
                        size = manipulate_insns(&insn[i], insns_byte, &status, mdi, info);
                else