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 accurately 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.c (x86_ginsn_indirect_jump_call): 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.c b/gas/config/tc-i386.c
index 7467cef18cdf796e3c9b9b9cc5a5de42da842076..9c4f3543f1a525cffa4224537ad966a4ed0ca000 100644 (file)
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5815,6 +5815,54 @@ x86_ginsn_jump (const symbolS *insn_end_sym, bool cond_p)
   return ginsn;
 }
 
+static ginsnS *
+x86_ginsn_indirect_jump_call (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);
+
+  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).  */
+    ginsn_func = ginsn_new_call;
+  else
+    /* Other cases are not expected.  Caller must screen.  */
+    return ginsn;
+
+  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)
+    {
+      mem_reg = i.base_reg ? i.base_reg : i.index_reg;
+      /* Use dummy register if no base or index.  Unlike other opcodes,
+        where we simply return NULL, we must try to generate a ginsn here.
+        Otherwise, the user gets the impression of missing functionality:
+          - a call insn is an IMPLICIT_STACK_OP.
+          - jmp insns are necessary for accurate control flow.  */
+      dw2_regnum = (mem_reg
+                   ? ginsn_dw2_regnum (mem_reg)
+                   : GINSN_DW2_REGNUM_RSI_DUMMY);
+      /* jmp/call *sym(,%rN,imm)  or  jmp/call *sym(%rN).  */
+      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)
 {
@@ -6324,50 +6372,9 @@ 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_jump_call (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..f6632445c5d99968a0ee8e25802abb35b8cc6aa3 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'
+.*:24: 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,29 @@ 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
+  14 \?\?\?\? FF14C500                 call    \*cost_arr\(,%rax,8\)
+  14      000000
+  14                   ginsn: CALL
+  15 \?\?\?\? FF142500                 call    \*symbol\+1
+  15      000000
+  15                   ginsn: CALL
+  16 \?\?\?\? 67E313                   jecxz   .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
+  17 \?\?\?\? FF6730                   jmp     \*48\(%rdi\)
+  17                   ginsn: JMP %r5, 
+  18 \?\?\?\? FF24C500                 jmp     \*cost_arr\(,%rax,8\)
+  18      000000
+  18                   ginsn: JMP %r0, 
+  19 \?\?\?\? FF242500                 jmp     \*symbol\+1
+  19      000000
+  19                   ginsn: JMP %r4, 
+  20 \?\?\?\? 7000                     jo      .L179
+  20                   ginsn: JCC 
+  21                   .L179:
+  21                   ginsn: SYM .L179
+  22 \?\?\?\? C3                       ret
+  22                   ginsn: RET
+  23                   .LFE0:
+  23                   ginsn: SYM .LFE0
+  24                           .size   foo, .-foo
+  24                   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..300634462007d2a308031cc4d6fc42dd3cfdc204 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,12 @@ foo:
        loop    foo
        notrack jmp     *%rax
        call    *%r8
+       call    *cost_arr(,%rax,8)
+       call    *symbol+1
        jecxz   .L179
        jmp     *48(%rdi)
+       jmp     *cost_arr(,%rax,8)
+       jmp     *symbol+1
        jo      .L179
 .L179:
        ret