diff options
author | Indu Bhagat <indu.bhagat@oracle.com> | 2024-08-01 10:07:07 -0700 |
---|---|---|
committer | Indu Bhagat <indu.bhagat@oracle.com> | 2024-08-01 10:07:07 -0700 |
commit | d56083b5047b8e7cc9eda2f867bd2b75724920a1 (patch) | |
tree | 4127e5e370ad25d6de0306e3d90ba9b12d9741d1 | |
parent | 94223333026f06dc5d78266dd9b6082544641f07 (diff) | |
download | gdb-d56083b5047b8e7cc9eda2f867bd2b75724920a1.zip gdb-d56083b5047b8e7cc9eda2f867bd2b75724920a1.tar.gz gdb-d56083b5047b8e7cc9eda2f867bd2b75724920a1.tar.bz2 |
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.
-rw-r--r-- | gas/config/tc-i386-ginsn.c | 101 | ||||
-rw-r--r-- | gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l | 48 | ||||
-rw-r--r-- | gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s | 6 |
3 files changed, 95 insertions, 60 deletions
diff --git a/gas/config/tc-i386-ginsn.c b/gas/config/tc-i386-ginsn.c index dccd675..b9dc9c1 100644 --- a/gas/config/tc-i386-ginsn.c +++ b/gas/config/tc-i386-ginsn.c @@ -469,6 +469,61 @@ x86_ginsn_jump (const symbolS *insn_end_sym, bool cond_p) } 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) { ginsnS *ginsn = NULL; @@ -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 ab6b50d..3261b76 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 0a63910..5ab66ba 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 |