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.
MediaTek-Labs · Aug 1, 2024 · d56083b · d56083b
1 parent 9422333
commit d56083b
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 60 deletions.
diff --git 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
@@ -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
@@ -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