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 dccd6758f0aaf25b24f27ee3073897032fb9bc9b..b9dc9c10cbbc5a08df52be70fbeba36da39d2996 100644 (file)
--- 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 ab6b50d47e8f574d287adb59d1eefd277c1f97a8..3261b76a5fdb4b29fd3e0f9f927e8db0b1996cae 100644 (file)
--- 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 0a63910e0460e2ce36b5028932d2ddf54435e5ea..5ab66ba5c264a3cfa7839a83ad37ff7198ad1605 100644 (file)
--- 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