gas: x86: ginsn: adapt the APX insn handling

A review comment on the SCFI V4 series was to bail out on APX
instructions differently:

Currently, GAS bails out at the mere sight of an APX instruction when
--scfi=experimental is in effect; this is implemented in form of an
early exit from x86_ginsn_new ().  It is preferable to not have to
handle APX instructions explicitly like that with an early exit,
but rather to use the existing infrastructure in the x86_ginsn_new ()
function to bail out only if the APX insn affects SCFI correctness.

FIXME -
1. There remains an outstanding issue with adcx etc ops
2. Add tests once above is fixed

gas/
	* config/tc-i386.c (ginsn_opsize_prefix_p): Rename and adjust to
	be ginsn_implicitstackop_opsize16_p instead.
	(x86_ginsn_enter): Use the new function.
	(x86_ginsn_leave): Likewise.
	(x86_ginsn_new): Likewise.  Also remove the stub for early exit
	for APX instructions.

gas/testsuite/
        * gas/scfi/x86_64/scfi-unsupported-insn-1.l:

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 9c4f3543f1a525cffa4224537ad966a4ed0ca000..dbc05c602698547dfb766c317af4326beb7fe5c3 100644 (file)
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5387,9 +5387,8 @@ x86_scfi_callee_saved_p (unsigned int dw2reg_num)
   return false;
 }
 
-/* Check whether an instruction prefix which affects operation size
-   accompanies.  For insns in the legacy space, setting REX.W takes precedence
-   over the operand-size prefix (66H) when both are used.
+/* Check whether the instruction affecting stack implicitly has a 16-bit
+   operation size.
 
    The current users of this API are in the handlers for PUSH, POP or other
    instructions which affect the stack pointer implicitly:  the operation size
@@ -5397,9 +5396,20 @@ x86_scfi_callee_saved_p (unsigned int dw2reg_num)
    incremented / decremented (2, 4 or 8).  */
 
 static bool
-ginsn_opsize_prefix_p (void)
+ginsn_implicitstackop_opsize16_p (void)
 {
-  return (!(i.prefix[REX_PREFIX] & REX_W) && i.prefix[DATA_PREFIX]);
+  bool opsize_prefix_p = false;
+
+  if (i.tm.opcode_modifier.operandconstraint != IMPLICIT_STACK_OP)
+    return opsize_prefix_p;
+
+  /* For insns in the legacy space, setting REX.W takes precedence
+     over the operand-size prefix (66H) when both are used.  */
+  opsize_prefix_p |= !(i.prefix[REX_PREFIX] & REX_W) && i.prefix[DATA_PREFIX];
+  opsize_prefix_p |= (i.tm.opcode_space == SPACE_EVEXMAP4
+                     && i.tm.opcode_modifier.opcodeprefix == PREFIX_0X66);
+
+  return opsize_prefix_p;
 }
 
 /* Get the DWARF register number for the given register entry.
@@ -5883,7 +5893,7 @@ x86_ginsn_enter (const symbolS *insn_end_sym)
     }
 
   /* Check if this is a 16-bit op.  */
-  if (ginsn_opsize_prefix_p ())
+  if (ginsn_implicitstackop_opsize16_p ())
     stack_opnd_size = 2;
 
   /* If the nesting level is 0, the processor pushes the frame pointer from
@@ -5920,7 +5930,7 @@ x86_ginsn_leave (const symbolS *insn_end_sym)
   int stack_opnd_size = 8;
 
   /* Check if this is a 16-bit op.  */
-  if (ginsn_opsize_prefix_p ())
+  if (ginsn_implicitstackop_opsize16_p ())
     stack_opnd_size = 2;
 
   /* The 'leave' instruction copies the contents of the RBP register
@@ -6097,16 +6107,6 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
 
   opcode = i.tm.base_opcode;
 
-  /* Until it is clear how to handle APX NDD and other new opcodes, disallow
-     them from SCFI.  */
-  if (is_apx_rex2_encoding ()
-      || (i.tm.opcode_modifier.evex && is_apx_evex_encoding ()))
-    {
-      as_bad (_("SCFI: unsupported APX op %#x may cause incorrect CFI"),
-             opcode);
-      return ginsn;
-    }
-
   switch (opcode)
     {
 
@@ -6136,7 +6136,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
        break;
       dw2_regnum = ginsn_dw2_regnum (i.op[0].regs);
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       ginsn = ginsn_new_sub (insn_end_sym, false,
                             GINSN_SRC_REG, REG_SP, 0,
@@ -6157,7 +6157,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
        break;
       dw2_regnum = ginsn_dw2_regnum (i.op[0].regs);
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       ginsn = ginsn_new_load (insn_end_sym, false,
                              GINSN_SRC_INDIRECT, REG_SP, 0,
@@ -6177,7 +6177,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
       /* push reg.  */
       dw2_regnum = ginsn_dw2_regnum (i.op[0].regs);
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       ginsn = ginsn_new_sub (insn_end_sym, false,
                             GINSN_SRC_REG, REG_SP, 0,
@@ -6201,7 +6201,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
                              GINSN_DST_REG, dw2_regnum);
       ginsn_set_where (ginsn);
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       ginsn_next = ginsn_new_add (insn_end_sym, false,
                                  GINSN_SRC_REG, REG_SP, 0,
@@ -6216,7 +6216,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
       if (i.tm.opcode_space != SPACE_BASE)
        break;
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       /* Skip getting the value of imm from machine instruction
         because this is not important for SCFI.  */
@@ -6285,7 +6285,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
                              GINSN_DST_INDIRECT, dw2_regnum);
       ginsn_set_where (ginsn);
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       ginsn_next = ginsn_new_add (insn_end_sym, false,
                                  GINSN_SRC_REG, REG_SP, 0,
@@ -6300,7 +6300,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
        break;
       /* pushf / pushfq.  */
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       ginsn = ginsn_new_sub (insn_end_sym, false,
                             GINSN_SRC_REG, REG_SP, 0,
@@ -6323,7 +6323,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
        break;
       /* popf / popfq.  */
       /* Check if operation size is 16-bit.  */
-      if (ginsn_opsize_prefix_p ())
+      if (ginsn_implicitstackop_opsize16_p ())
        stack_opnd_size = 2;
       /* FIXME - hardcode the actual DWARF reg number value.  As for SCFI
         correctness, although this behaves simply a placeholder value; its
@@ -6348,7 +6348,7 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
       if (i.tm.extension_opcode == 6)
        {
          /* Check if operation size is 16-bit.  */
-         if (ginsn_opsize_prefix_p ())
+         if (ginsn_implicitstackop_opsize16_p ())
            stack_opnd_size = 2;
          ginsn = ginsn_new_sub (insn_end_sym, false,
                                 GINSN_SRC_REG, REG_SP, 0,

diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-insn-1.l b/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-insn-1.l
index d66a4c461c371f77066807cb3e2ef95f83497252..ed7399d11ed1cae1de5c2fa26c0be50ff0a4ec5c 100644 (file)
--- a/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-insn-1.l
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-insn-1.l
@@ -1,7 +1,8 @@
 .*Assembler messages:
-.*7: Error: SCFI: unsupported APX op 0x8f may cause incorrect CFI
-.*8: Error: SCFI: unsupported APX op 0x8f may cause incorrect CFI
-.*9: Error: SCFI: unsupported APX op 0xff may cause incorrect CFI
-.*10: Error: SCFI: unsupported APX op 0xff may cause incorrect CFI
-.*11: Error: SCFI: unsupported APX op 0x11 may cause incorrect CFI
+.*7: Error: SCFI: unhandled op 0x8f may cause incorrect CFI
+.*8: Error: SCFI: unhandled op 0x8f may cause incorrect CFI
+.*9: Error: SCFI: unhandled op 0xff may cause incorrect CFI
+.*10: Error: SCFI: unhandled op 0xff may cause incorrect CFI
 .*13: Error: SCFI: hand-crafting instructions not supported
+.*11: Error: SCFI: unsupported stack manipulation pattern
+.*16: Error: SCFI: forward pass failed for func 'foo'