From: Indu Bhagat Date: Thu, 1 Aug 2024 17:07:07 +0000 (-0700) Subject: gas: x86: ginsn: handle previously missed indirect call and jmp ops X-Git-Tag: gdb-16-branchpoint~1250 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d56083b5047b8e7cc9eda2f867bd2b75724920a1;p=thirdparty%2Fbinutils-gdb.git gas: x86: ginsn: handle previously missed indirect call and jmp ops Some flavors of indirect call and jmp instructions were not being handled earlier, leading to a GAS error (#1): (#1) "Error: SCFI: unhandled op 0xff may cause incorrect CFI" Not handling jmp/call (direct or indirect) ops is an error (as shown above) because SCFI needs an accurate CFG to synthesize CFI correctly. Recall that the presence of indirect jmp/call, however, does make the CFG ineligible for SCFI. In other words, generating the ginsns for them now, will eventually cause SCFI to bail out later with an error (#2) anyway: (#2) "Error: untraceable control flow for func 'XXX'" The first error (#1) gives the impression of missing functionality in GAS. So, it seems cleaner to synthesize a GINSN_TYPE_JUMP / GINSN_TYPE_CALL now in the backend, and let SCFI machinery complain with the error as expected. The handling for these indirect jmp/call instructions is similar, so reuse the code by carving out a function for the same. Adjust the testcase to include the now handled jmp/call instructions as well. gas/ * config/tc-i386-ginsn.c (x86_ginsn_indirect_branch): New function. (x86_ginsn_new): Refactor out functionality to above. gas/testsuite/ * gas/scfi/x86_64/ginsn-cofi-1.l: Adjust the output. * gas/scfi/x86_64/ginsn-cofi-1.s: Add further varieties of jmp/call opcodes. --- diff --git a/gas/config/tc-i386-ginsn.c b/gas/config/tc-i386-ginsn.c index dccd6758f0a..b9dc9c10cbb 100644 --- a/gas/config/tc-i386-ginsn.c +++ b/gas/config/tc-i386-ginsn.c @@ -468,6 +468,61 @@ x86_ginsn_jump (const symbolS *insn_end_sym, bool cond_p) return ginsn; } +static ginsnS * +x86_ginsn_indirect_branch (const symbolS *insn_end_sym) +{ + ginsnS *ginsn = NULL; + const reg_entry *mem_reg; + unsigned int dw2_regnum; + + ginsnS * (*ginsn_func) (const symbolS *sym, bool real_p, + enum ginsn_src_type src_type, unsigned int src_reg, + const symbolS *src_ginsn_sym); + + /* Other cases are not expected. */ + gas_assert (i.tm.extension_opcode == 4 || i.tm.extension_opcode == 2); + + if (i.tm.extension_opcode == 4) + /* 0xFF /4 (jmp r/m). */ + ginsn_func = ginsn_new_jump; + else if (i.tm.extension_opcode == 2) + /* 0xFF /2 (call r/m). */ + ginsn_func = ginsn_new_call; + + if (i.reg_operands) + { + dw2_regnum = ginsn_dw2_regnum (i.op[0].regs); + ginsn = ginsn_func (insn_end_sym, true, + GINSN_SRC_REG, dw2_regnum, NULL); + ginsn_set_where (ginsn); + } + else if (i.mem_operands) + { + /* Handle jump/call near, absolute indirect, address. + E.g., jmp/call *imm(%rN), jmp/call *sym(,%rN,imm) + or jmp/call *sym(%rN) etc. */ + mem_reg = i.base_reg ? i.base_reg : i.index_reg; + /* Generate a ginsn, even if it is with TBD_GINSN_INFO_LOSS. Otherwise, + the user gets the impression of missing functionality due to this + being a COFI and alerted for via the x86_ginsn_unhandled () workflow + as unhandled operation (which can be misleading for users). + + Indirect branches make the code block ineligible for SCFI; Hence, an + approximate ginsn will not affect SCFI correctness: + - Use dummy register if no base or index + - Skip symbol information, if any. + Note this case of TBD_GINSN_GEN_NOT_SCFI. */ + dw2_regnum = (mem_reg + ? ginsn_dw2_regnum (mem_reg) + : GINSN_DW2_REGNUM_RSI_DUMMY); + ginsn = ginsn_func (insn_end_sym, true, + GINSN_SRC_REG, dw2_regnum, NULL); + ginsn_set_where (ginsn); + } + + return ginsn; +} + static ginsnS * x86_ginsn_enter (const symbolS *insn_end_sym) { @@ -977,50 +1032,8 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode) ginsn_set_where (ginsn_next); gas_assert (!ginsn_link_next (ginsn, ginsn_next)); } - else if (i.tm.extension_opcode == 4) - { - /* jmp r/m. E.g., notrack jmp *%rax. */ - if (i.reg_operands) - { - dw2_regnum = ginsn_dw2_regnum (i.op[0].regs); - ginsn = ginsn_new_jump (insn_end_sym, true, - GINSN_SRC_REG, dw2_regnum, NULL); - ginsn_set_where (ginsn); - } - else if (i.mem_operands && i.index_reg) - { - /* jmp *0x0(,%rax,8). */ - dw2_regnum = ginsn_dw2_regnum (i.index_reg); - ginsn = ginsn_new_jump (insn_end_sym, true, - GINSN_SRC_REG, dw2_regnum, NULL); - ginsn_set_where (ginsn); - } - else if (i.mem_operands && i.base_reg) - { - dw2_regnum = ginsn_dw2_regnum (i.base_reg); - ginsn = ginsn_new_jump (insn_end_sym, true, - GINSN_SRC_REG, dw2_regnum, NULL); - ginsn_set_where (ginsn); - } - } - else if (i.tm.extension_opcode == 2) - { - /* 0xFF /2 (call). */ - if (i.reg_operands) - { - dw2_regnum = ginsn_dw2_regnum (i.op[0].regs); - ginsn = ginsn_new_call (insn_end_sym, true, - GINSN_SRC_REG, dw2_regnum, NULL); - ginsn_set_where (ginsn); - } - else if (i.mem_operands && i.base_reg) - { - dw2_regnum = ginsn_dw2_regnum (i.base_reg); - ginsn = ginsn_new_call (insn_end_sym, true, - GINSN_SRC_REG, dw2_regnum, NULL); - ginsn_set_where (ginsn); - } - } + else if (i.tm.extension_opcode == 4 || i.tm.extension_opcode == 2) + ginsn = x86_ginsn_indirect_branch (insn_end_sym); break; case 0xc2: /* ret imm16. */ diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l index ab6b50d47e8..3261b76a5fd 100644 --- a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l +++ b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l @@ -1,8 +1,7 @@ .*: Assembler messages: -.*:20: Error: untraceable control flow for func 'foo' +.*:26: Error: untraceable control flow for func 'foo' GAS LISTING .* - 1 # Testcase with a variety of "change of flow instructions" 2 # 3 # This test does not have much going on wrt synthesis of CFI; @@ -22,17 +21,34 @@ GAS LISTING .* 12 ginsn: JMP %r0, 13 \?\?\?\? 41FFD0 call \*%r8 13 ginsn: CALL - 14 \?\?\?\? 67E305 jecxz .L179 - 14 ginsn: JCC - 15 \?\?\?\? FF6730 jmp \*48\(%rdi\) - 15 ginsn: JMP %r5, - 16 \?\?\?\? 7000 jo .L179 - 16 ginsn: JCC - 17 .L179: - 17 ginsn: SYM .L179 - 18 \?\?\?\? C3 ret - 18 ginsn: RET - 19 .LFE0: - 19 ginsn: SYM .LFE0 - 20 .size foo, .-foo - 20 ginsn: SYM FUNC_END + 14 \?\?\?\? FF14C500 call \*cost_arr\(,%rax,8\) + 14 000000 + 14 ginsn: CALL + 15 \?\?\?\? FF149500 call \*\(,%rdx, 4\) + 15 000000 + 15 ginsn: CALL + 16 \?\?\?\? FF142500 call \*symbol\+1 + 16 000000 + 16 ginsn: CALL + 17 \?\?\?\? 67E316 jecxz .L179 + 17 ginsn: JCC + 18 \?\?\?\? 41FFE0 jmp \*%r8 + 18 ginsn: JMP %r8, + 19 \?\?\?\? FF6730 jmp \*48\(%rdi\) + 19 ginsn: JMP %r5, + 20 \?\?\?\? FF24C500 jmp \*cost_arr\(,%rax,8\) + 20 000000 + 20 ginsn: JMP %r0, + 21 \?\?\?\? FF242500 jmp \*symbol\+1 + 21 000000 + 21 ginsn: JMP %r4, + 22 \?\?\?\? 7000 jo .L179 + 22 ginsn: JCC + 23 .L179: + 23 ginsn: SYM .L179 + 24 \?\?\?\? C3 ret + 24 ginsn: RET + 25 .LFE0: + 25 ginsn: SYM .LFE0 + 26 .size foo, .-foo + 26 ginsn: SYM FUNC_END diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s index 0a63910e046..5ab66ba5c26 100644 --- a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s +++ b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s @@ -11,8 +11,14 @@ foo: loop foo notrack jmp *%rax call *%r8 + call *cost_arr(,%rax,8) + call *(,%rdx, 4) + call *symbol+1 jecxz .L179 + jmp *%r8 jmp *48(%rdi) + jmp *cost_arr(,%rax,8) + jmp *symbol+1 jo .L179 .L179: ret