gas: gcfg: fix handling of non-local direct jmps in gcfg

The ginsn infrastructure in GAS includes the ability to create a GCFG
(ginsn CFG).  A GCFG is currently used for SCFI passes.

This patch fixes the following invalid assumptions / code blocks:
 - The function ginsn_direct_local_jump_p () was erroneously _not_
   checking whether the symbol is locally defined (i.e., within the
   scope of the code block for which GCFG is desired).  Fix the code
   to do so.
 - Similarly, the GCFG creation code, in gcfg_build () itself had an
   assumption that a GINSN_TYPE_JUMP to a non-local symbol will not be
   seen.  The latter can indeed be seen, and in fact, needs to be treated
   the same way as an exit from the function in terms of control-flow.

gas/
        * ginsn.c (ginsn_direct_local_jump_p): Check if the symbol
	is local to the code block or function being assembled.
        (add_bb_at_ginsn): Remove buggy assumption.
        (frch_ginsn_data_append): Direct jmps do not disqualify a stream
	of ginsns from GCFG creation.

gas/testsuite/
	* gas/scfi/x86_64/scfi-cfg-3.d: New test.
	* gas/scfi/x86_64/scfi-cfg-3.l: New test.
	* gas/scfi/x86_64/scfi-cfg-3.s: New test.
	* gas/scfi/x86_64/scfi-x86-64.exp: Add new test.

diff --git a/gas/ginsn.c b/gas/ginsn.c
index 661f51d23c59f732140c8f828b61d4126e9a6e4b..492e161876b0fb9705e29f14fbcbf43c425a2818 100644 (file)
--- a/gas/ginsn.c
+++ b/gas/ginsn.c
@@ -444,16 +444,26 @@ ginsn_indirect_jump_p (ginsnS *ginsn)
   return ret_p;
 }
 
+/* Return whether the GINSN is an unconditional jump to a label which is
+   defined locally in the scope of the block of insns, which are currently
+   being processed for GCFG creation.  */
+
 static bool
 ginsn_direct_local_jump_p (ginsnS *ginsn)
 {
-  bool ret_p = false;
+  bool local_p = false;
+  const symbolS *taken_label;
+
   if (!ginsn)
-    return ret_p;
+    return local_p;
 
-  ret_p |= (ginsn->type == GINSN_TYPE_JUMP
-           && ginsn->src[0].type == GINSN_SRC_SYMBOL);
-  return ret_p;
+  if (ginsn->type == GINSN_TYPE_JUMP
+      && ginsn->src[0].type == GINSN_SRC_SYMBOL)
+    {
+      taken_label = ginsn->src[0].sym;
+      local_p = (label_ginsn_map_find (taken_label) != NULL);
+    }
+  return local_p;
 }
 
 static char *
@@ -785,16 +795,13 @@ add_bb_at_ginsn (const symbolS *func, gcfgS *gcfg, ginsnS *ginsn, gbbS *prev_bb,
          || ginsn->type == GINSN_TYPE_JUMP_COND
          || ginsn->type == GINSN_TYPE_RETURN)
        {
-         /* Indirect Jumps or direct jumps to symbols non-local to the
-            function must not be seen here.  The caller must have already
-            checked for that.  */
+         /* Indirect jumps must not be seen here.  The caller must have
+            already checked for that.  */
          gas_assert (!ginsn_indirect_jump_p (ginsn));
-         if (ginsn->type == GINSN_TYPE_JUMP)
-           gas_assert (ginsn_direct_local_jump_p (ginsn));
 
-         /* Direct Jumps.  May include conditional or unconditional change of
-            flow.  What is important for CFG creation is that the target be
-            local to function.  */
+         /* Handle direct jumps.  For unconditional direct jumps, where the
+            target is not local to the function, treat them later as similar
+            to an exit from function (in the else block).  */
          if (ginsn->type == GINSN_TYPE_JUMP_COND
              || ginsn_direct_local_jump_p (ginsn))
            {
@@ -822,10 +829,14 @@ add_bb_at_ginsn (const symbolS *func, gcfgS *gcfg, ginsnS *ginsn, gbbS *prev_bb,
              /* Add the bb for the fall through path.  */
              find_or_make_bb (func, gcfg, ginsn->next, prev_bb, errp);
            }
-        else if (ginsn->type == GINSN_TYPE_RETURN)
+        else
           {
-            /* We'll come back to the ginsns following GINSN_TYPE_RETURN
-               from another path if they are indeed reachable code.  */
+            gas_assert (ginsn->type == GINSN_TYPE_RETURN
+                        || (ginsn->type == GINSN_TYPE_JUMP
+                            && !ginsn_direct_local_jump_p (ginsn)));
+            /* We'll come back to the ginsns following GINSN_TYPE_RETURN or
+               other (non-local) unconditional jmps from another path if they
+               are indeed reachable code.  */
             break;
           }
 
@@ -1083,9 +1094,7 @@ frch_ginsn_data_append (ginsnS *ginsn)
     {
       temp->id = ++id;
 
-      if (ginsn_indirect_jump_p (temp)
-         || (ginsn->type == GINSN_TYPE_JUMP
-             && !ginsn_direct_local_jump_p (temp)))
+      if (ginsn_indirect_jump_p (temp))
        frchain_now->frch_ginsn_data->gcfg_apt_p = false;
 
       if (listing & LISTING_GINSN_SCFI)

diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d
new file mode 100644 (file)
index 0000000..c5a9838
--- /dev/null
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d
@@ -0,0 +1,32 @@
+#as: --scfi=experimental -W
+#as:
+#objdump: -Wf
+#name: Synthesize CFI in presence of control flow 3
+#...
+Contents of the .eh_frame section:
+
+00000000 0+0014 0+0000 CIE
+  Version:               1
+  Augmentation:          "zR"
+  Code alignment factor: 1
+  Data alignment factor: -8
+  Return address column: 16
+  Augmentation data:     1b
+  DW_CFA_def_cfa: r7 \(rsp\) ofs 8
+  DW_CFA_offset: r16 \(rip\) at cfa-8
+  DW_CFA_nop
+  DW_CFA_nop
+
+0+0018 0+0010 0+001c FDE cie=00000000 pc=0+0000..0+001e
+  DW_CFA_nop
+#...
+
+0+002c 0+0018 0+0030 FDE cie=00000000 pc=0+001e..0+0029
+  DW_CFA_advance_loc: 1 to 0+001f
+  DW_CFA_def_cfa_offset: 16
+  DW_CFA_offset: r6 \(rbp\) at cfa-16
+  DW_CFA_advance_loc: 3 to 0+0022
+  DW_CFA_def_cfa_register: r6 \(rbp\)
+  DW_CFA_nop
+
+#pass

diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l
new file mode 100644 (file)
index 0000000..23ca734
--- /dev/null
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l
@@ -0,0 +1,2 @@
+.*Assembler messages:
+.*9: Warning: SCFI ignores most user-specified CFI directives

diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s
new file mode 100644 (file)
index 0000000..1c8f9b9
--- /dev/null
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s
@@ -0,0 +1,40 @@
+# Testcase with jmp to function instead of a call.
+# Also includes a jmp to label locally defined.
+# The CFG creation process is not expected to warn about
+# missing foo_handler_v1 or xstrdup.
+       .text
+       .globl  foo_handler
+       .type   foo_handler, @function
+foo_handler:
+       .cfi_startproc
+       movl    current_style(%rip), %eax
+       jmp     .L2
+.L2:
+       cmpl    $-1, %eax
+       je      .L5
+       testb   $4, %al
+       je      .L3
+       jmp     foo_handler_v1
+.L3:
+       xorl    %eax, %eax
+       ret
+.L5:
+       jmp     xstrdup
+       .cfi_endproc
+       .size   foo_handler, .-foo_handler
+
+# New function with unconditional jump to a label previously defined
+# in a different function.
+       .globl  bar
+       .type   bar, @function
+bar:
+       .cfi_startproc
+       pushq   %rbp
+       .cfi_def_cfa_offset 16
+       .cfi_offset %rbp, -16
+       movq    %rsp, %rbp
+       .cfi_def_cfa_register %rbp
+       movl    $0, %eax
+       jmp     .L2
+       .cfi_endproc
+       .size   bar, .-bar

diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp b/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
index 5324af386f8a0bf96d369f668b46595cef86e4df..c004d99b4ac3b53fb776f3e87c5753a783e96e11 100644 (file)
--- a/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
@@ -78,6 +78,8 @@ if  { ([istarget "x86_64-*-*"] && ![istarget "x86_64-*-linux*-gnux32"]) } then {
     run_list_test "scfi-cfg-1" "--scfi=experimental --warn"
     run_dump_test "scfi-cfg-2"
     run_list_test "scfi-cfg-2" "--scfi=experimental --warn"
+    run_dump_test "scfi-cfg-3"
+    run_list_test "scfi-cfg-3" "--scfi=experimental --warn"
     run_dump_test "scfi-asm-marker-1"
     run_list_test "scfi-asm-marker-1" "--scfi=experimental --warn"
     run_dump_test "scfi-asm-marker-2"