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.
---
[Changes in V2]
- Rebase to latest master.
- Rename function to x86_ginsn_indirect_branch. Adjust commit log.
- Fix code comments. Mention TBD_GINSN_GEN_NOT_SCFI.
- Add new insns: call *(,%rdx, 4) and jmp *%r8 to
ginsn-cofi-1.s testcase.
[End of changes in V2]
gas/config/tc-i386-ginsn.c | 101 +++++++++++--------
gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l | 48 ++++++---
gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s | 6 ++
3 files changed, 95 insertions(+), 60 deletions(-)
On 01.08.2024 09:57, Indu Bhagat wrote:
> 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.
Okay.
Jan
@@ -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. */
@@ -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
@@ -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