[Darwin] Fix PR57438 by avoiding empty function bodies and trailing labels.

A.
Empty function bodies causes two problems for Darwin's linker (i) zero-length
FDEs and (ii) coincident label addresses that might point to items of
differing weakness.

B.
Trailing local labels can be problematic when they end a function because
similarly they might apparently point to a following weak function, leading
to the linker concluding that there's a pointer-diff to a weak symbol
(which is not allowed).

Both conditions arise from __builtin_unreachable() lowering to a barrier.

The solution for both is to emit some finite amount of code; in the case of A
a trap is emitted, in the case of B a nop.

gcc/

2016-11-27  Iain Sandoe  <iain@codesourcery.com>

	PR target/57438
	* config/i386/i386.c (ix86_code_end): Note that we emitted code
	where the function might otherwise appear empty for picbase thunks.
	(ix86_output_function_epilogue): If we find a zero-sized function
	assume that reaching it is UB and trap.  If we find a trailing label
	append a nop.
	* config/rs6000/rs6000.c (rs6000_output_function_epilogue): If we
	find a zero-sized function assume that reaching it is UB and trap.
	If we find a trailing label, append a nop.

gcc/testsuite/

2016-11-27  Iain Sandoe  <iain@codesourcery.com>

	PR target/57438
	* gcc.dg/pr57438-1.c: New Test.
	* gcc.dg/pr57438-2.c: New Test.

From-SVN: r242897

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index ba2526c99c34fa024250c19de5bb23369e60a00d..a57eaab8b20e300da75bd9bda1fadf8f280608b6 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,15 @@
+2016-11-27  Iain Sandoe  <iain@codesourcery.com>
+
+       PR target/57438
+       * config/i386/i386.c (ix86_code_end): Note that we emitted code
+       where the function might otherwise appear empty for picbase thunks.
+       (ix86_output_function_epilogue): If we find a zero-sized function
+       assume that reaching it is UB and trap.  If we find a trailing label
+       append a nop.
+       * config/rs6000/rs6000.c (rs6000_output_function_epilogue): If we
+       find a zero-sized function assume that reaching it is UB and trap.
+       If we find a trailing label, append a nop.
+
 2016-11-27  Iain Sandoe  <iain@codesourcery.com>
 
        PR target/71767

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 1dd5669bfea9623cb508e16364f975f5c440574f..5018ccb633aef756c13c273b70b65365fd40bc6a 100644 (file)
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11920,6 +11920,9 @@ ix86_code_end (void)
       current_function_decl = decl;
       allocate_struct_function (decl, false);
       init_function_start (decl);
+      /* We're about to hide the function body from callees of final_* by
+        emitting it directly; tell them we're a thunk, if they care.  */
+      cfun->is_thunk = true;
       first_function_block_is_cold = false;
       /* Make sure unwind info is emitted for the thunk if needed.  */
       final_start_function (emit_barrier (), asm_out_file, 1);
@@ -14625,36 +14628,68 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED, HOST_WIDE_INT)
   if (pic_offset_table_rtx
       && !ix86_use_pseudo_pic_reg ())
     SET_REGNO (pic_offset_table_rtx, REAL_PIC_OFFSET_TABLE_REGNUM);
-#if TARGET_MACHO
-  /* Mach-O doesn't support labels at the end of objects, so if
-     it looks like we might want one, insert a NOP.  */
-  {
-    rtx_insn *insn = get_last_insn ();
-    rtx_insn *deleted_debug_label = NULL;
-    while (insn
-          && NOTE_P (insn)
-          && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL)
-      {
-       /* Don't insert a nop for NOTE_INSN_DELETED_DEBUG_LABEL
-          notes only, instead set their CODE_LABEL_NUMBER to -1,
-          otherwise there would be code generation differences
-          in between -g and -g0.  */
-       if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL)
-         deleted_debug_label = insn;
+
+  if (TARGET_MACHO)
+    {
+      rtx_insn *insn = get_last_insn ();
+      rtx_insn *deleted_debug_label = NULL;
+
+      /* Mach-O doesn't support labels at the end of objects, so if
+         it looks like we might want one, take special action.
+        First, collect any sequence of deleted debug labels.  */
+      while (insn
+            && NOTE_P (insn)
+            && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL)
+       {
+         /* Don't insert a nop for NOTE_INSN_DELETED_DEBUG_LABEL
+            notes only, instead set their CODE_LABEL_NUMBER to -1,
+            otherwise there would be code generation differences
+            in between -g and -g0.  */
+         if (NOTE_P (insn) && NOTE_KIND (insn)
+             == NOTE_INSN_DELETED_DEBUG_LABEL)
+           deleted_debug_label = insn;
+         insn = PREV_INSN (insn);
+       }
+
+      /* If we have:
+        label:
+           barrier
+         then this needs to be detected, so skip past the barrier.  */
+
+      if (insn && BARRIER_P (insn))
        insn = PREV_INSN (insn);
-      }
-    if (insn
-       && (LABEL_P (insn)
-           || (NOTE_P (insn)
-               && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)))
-      fputs ("\tnop\n", file);
-    else if (deleted_debug_label)
-      for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn))
-       if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL)
-         CODE_LABEL_NUMBER (insn) = -1;
-  }
-#endif
 
+      /* Up to now we've only seen notes or barriers.  */
+      if (insn)
+       {
+         if (LABEL_P (insn)
+             || (NOTE_P (insn)
+                 && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
+           /* Trailing label.  */
+           fputs ("\tnop\n", file);
+         else if (cfun && ! cfun->is_thunk)
+           {
+             /* See if we have a completely empty function body, skipping
+                the special case of the picbase thunk emitted as asm.  */
+             while (insn && ! INSN_P (insn))
+               insn = PREV_INSN (insn);
+             /* If we don't find any insns, we've got an empty function body;
+                I.e. completely empty - without a return or branch.  This is
+                taken as the case where a function body has been removed
+                because it contains an inline __builtin_unreachable().  GCC
+                declares that reaching __builtin_unreachable() means UB so
+                we're not obliged to do anything special; however, we want
+                non-zero-sized function bodies.  To meet this, and help the
+                user out, let's trap the case.  */
+             if (insn == NULL)
+               fputs ("\tud2\n", file);
+           }
+       }
+      else if (deleted_debug_label)
+       for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn))
+         if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL)
+           CODE_LABEL_NUMBER (insn) = -1;
+    }
 }
 
 /* Return a scratch register to use in the split stack prologue.  The

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index fce4e39128a45bd8e9e0ebc8abe70390af6c1132..6c28e6aaf6543a03b5db0a91e35b452d1eb47b98 100644 (file)
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -30234,11 +30234,15 @@ rs6000_output_function_epilogue (FILE *file,
 {
 #if TARGET_MACHO
   macho_branch_islands ();
-  /* Mach-O doesn't support labels at the end of objects, so if
-     it looks like we might want one, insert a NOP.  */
+
   {
     rtx_insn *insn = get_last_insn ();
     rtx_insn *deleted_debug_label = NULL;
+
+    /* Mach-O doesn't support labels at the end of objects, so if
+       it looks like we might want one, take special action.
+
+       First, collect any sequence of deleted debug labels.  */
     while (insn
           && NOTE_P (insn)
           && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL)
@@ -30251,11 +30255,40 @@ rs6000_output_function_epilogue (FILE *file,
          deleted_debug_label = insn;
        insn = PREV_INSN (insn);
       }
-    if (insn
-       && (LABEL_P (insn)
+
+    /* Second, if we have:
+       label:
+        barrier
+       then this needs to be detected, so skip past the barrier.  */
+
+    if (insn && BARRIER_P (insn))
+      insn = PREV_INSN (insn);
+
+    /* Up to now we've only seen notes or barriers.  */
+    if (insn)
+      {
+       if (LABEL_P (insn)
            || (NOTE_P (insn)
-               && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)))
-      fputs ("\tnop\n", file);
+               && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
+         /* Trailing label: <barrier>.  */
+         fputs ("\tnop\n", file);
+       else
+         {
+           /* Lastly, see if we have a completely empty function body.  */
+           while (insn && ! INSN_P (insn))
+             insn = PREV_INSN (insn);
+           /* If we don't find any insns, we've got an empty function body;
+              I.e. completely empty - without a return or branch.  This is
+              taken as the case where a function body has been removed
+              because it contains an inline __builtin_unreachable().  GCC
+              states that reaching __builtin_unreachable() means UB so we're
+              not obliged to do anything special; however, we want
+              non-zero-sized function bodies.  To meet this, and help the
+              user out, let's trap the case.  */
+           if (insn == NULL)
+             fputs ("\ttrap\n", file);
+         }
+      }
     else if (deleted_debug_label)
       for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn))
        if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index bd2d5405ba71d1ea9a1a4887ff23f5bf0d91747e..d7eacc3543e12647f1890108b2accd4086bd1586 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2016-11-27  Iain Sandoe  <iain@codesourcery.com>
+
+       PR target/57438
+       * gcc.dg/pr57438-1.c: New Test.
+       * gcc.dg/pr57438-2.c: New Test.
+
 2016-11-27  Dominique d'Humieres  <dominiq@lps.ens.fr>
            Iain Sandoe  <iain@codesourcery.com>
 

diff --git a/gcc/testsuite/gcc.dg/pr57438-1.c b/gcc/testsuite/gcc.dg/pr57438-1.c
new file mode 100644 (file)
index 0000000..9bfd8b9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr57438-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target *-*-darwin* } } */
+/* { dg-options "-O1" } */
+/* { dg-additional-options "-mdynamic-no-pic" { target powerpc*-*-darwin* } }
+
+/* This is testing that a completely empty function body results in the
+   insertion of a ud2/trap instruction to prevent a zero-sized FDE, and/or
+   the function label apparently pointing to following code.  */
+
+__attribute__((noinline))
+void foo (void)
+{
+  __builtin_unreachable();
+}
+
+/* { dg-final { scan-assembler "ud2" { target  { i?86-*-darwin*  x86_64-*-darwin* } } } } */
+/* { dg-final { scan-assembler "trap" { target { powerpc*-*-darwin* } } } } */

diff --git a/gcc/testsuite/gcc.dg/pr57438-2.c b/gcc/testsuite/gcc.dg/pr57438-2.c
new file mode 100644 (file)
index 0000000..f3ff1dc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr57438-2.c
@@ -0,0 +1,23 @@
+/* { dg-do compile { target *-*-darwin* } } */
+/* { dg-options "--param case-values-threshold=3 -O2" } */
+/* { dg-additional-options "-funwind-tables" { target powerpc*-*-darwin* } }
+
+/* This is testing that a trailing local label is followed by a
+   nop where required.  */
+   
+int foo (int x)
+{
+  switch (x)
+    {
+      case 0:
+        return 10;
+      case 3:
+        return -1;
+      case 5:
+        return 29;
+      default:
+        __builtin_unreachable();
+    }
+}
+
+/* { dg-final { scan-assembler "nop\\nLFE.*" { target  { *-*-darwin* } } } } */