diff mbox

[_eh,dawin,Version2] Allow targets to suppress epilogues in _eh frames.

Message ID 20100816041534.GA10370@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Aug. 16, 2010, 4:15 a.m. UTC
On Sun, Aug 15, 2010 at 10:31:13PM -0400, Jack Howarth wrote:
> Iain,
>    I am also seeing the failure...
> 
> FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test
> 
> at -m64 under this second revision of your _eh darwin
> patch. This represents a regression from both current
> gcc trunk and your previous _eh darwin patch...
> 
> http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01037.html
> 
> when I tested against either the compatibility or compact
> unwinders on x86_64-apple-darwin10 or against the compatibility
> unwinder on i386-apple-darwin10.
> 
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01454.html
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01429.html
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01573.html
> 
> It would appear that the second patch doesn't exactly reproduce
> the behavior of the first. Can you break up the changes between
> the first and second patches into sections to figure out which
> part triggered the gfortran testsuite failure?
>                     Jack

Iain,
   In case it helps, below is the diff between current gcc trunk
with your first and second patches applied so that the differences
between the two versions are more clearly seen.
                     Jack

Comments

Iain Sandoe Aug. 16, 2010, 9:52 a.m. UTC | #1
On 16 Aug 2010, at 05:15, Jack Howarth wrote:

>>   I am also seeing the failure...
>>
>> FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test

Hm.  This test does not contain any (overt) exception handling, I  
guess we must be seeing an interaction with the fortran runtime
(or some aspect of the optimize for smallest code which is unexpected  
--- note that m32 forces frame pointers on for optimize_size).

The salient difference between trunk and the patched version is that  
there are no unwind frames emitted for the patched version.
Un-patched trunk sets -fasynchronous-unwind-tables.

The test passes if   either

-fno-omit-frame-pointers

or

-funwind-tables

is given.

this can be replicated on un-patched trunk by passing '-fno- 
asynchronous-frame-tables'.

Given that -fno-asynchronous-frame-tables does not sit well with  
dawin...
I could arrange either to default to  "-funwind-tables" on m64 (at  
least for fortran) but please note the following:

======
apple_local 4.2.1 (5646) gcc/config/i386/i386.c:

   if (TARGET_64BIT)
     {
       /* Mach-O doesn't support omitting the frame pointer for now.  */
       if (flag_omit_frame_pointer == 2)
	flag_omit_frame_pointer = (TARGET_MACHO ? 0 : 1);

but we have recently changed trunk to this:

   if (TARGET_64BIT)
     {
       if (flag_zee == 2)
         flag_zee = 1;
       if (flag_omit_frame_pointer == 2)
	flag_omit_frame_pointer = 1;


------

If you know that the "does not support" is no longer true for Darwin10  
- then, fine, we can arrange the target over-rides to switch it off  
for Darwin10.

Otherwise, I'm not sure how we can justify omitting frame pointers by  
default for m64 OSX
Mike?

cheers,
Iain
Jack Howarth Aug. 16, 2010, 10:48 a.m. UTC | #2
On Mon, Aug 16, 2010 at 10:52:39AM +0100, IainS wrote:
>
> On 16 Aug 2010, at 05:15, Jack Howarth wrote:
>
>>>   I am also seeing the failure...
>>>
>>> FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test
>
> Hm.  This test does not contain any (overt) exception handling, I guess 
> we must be seeing an interaction with the fortran runtime
> (or some aspect of the optimize for smallest code which is unexpected  
> --- note that m32 forces frame pointers on for optimize_size).
>
> The salient difference between trunk and the patched version is that  
> there are no unwind frames emitted for the patched version.
> Un-patched trunk sets -fasynchronous-unwind-tables.

This seems to be in line with the comments I found in...

http://llvm.org/bugs/show_bug.cgi?id=4945

in particular comment 2 there which indicates that -fasynchronous-unwind-tables
should be defaulted on with -fomit-frame-pointer on x86-32 (for gdb at least).

>
> The test passes if   either
>
> -fno-omit-frame-pointers
>
> or
>
> -funwind-tables
>
> is given.
>
> this can be replicated on un-patched trunk by passing '-fno- 
> asynchronous-frame-tables'.
>
> Given that -fno-asynchronous-frame-tables does not sit well with  
> dawin...
> I could arrange either to default to  "-funwind-tables" on m64 (at least 
> for fortran) but please note the following:
>
> ======
> apple_local 4.2.1 (5646) gcc/config/i386/i386.c:
>
>   if (TARGET_64BIT)
>     {
>       /* Mach-O doesn't support omitting the frame pointer for now.  */
>       if (flag_omit_frame_pointer == 2)
> 	flag_omit_frame_pointer = (TARGET_MACHO ? 0 : 1);
>
> but we have recently changed trunk to this:
>
>   if (TARGET_64BIT)
>     {
>       if (flag_zee == 2)
>         flag_zee = 1;
>       if (flag_omit_frame_pointer == 2)
> 	flag_omit_frame_pointer = 1;
>
>
> ------
>
> If you know that the "does not support" is no longer true for Darwin10 - 
> then, fine, we can arrange the target over-rides to switch it off for 
> Darwin10.
>
> Otherwise, I'm not sure how we can justify omitting frame pointers by  
> default for m64 OSX
> Mike?
>
> cheers,
> Iain
diff mbox

Patch

diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/config/darwin-protos.h gcc-4.6-20100815/gcc/config/darwin-protos.h
--- gcc-4.6-20100815.old_eh_patch/gcc/config/darwin-protos.h	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/config/darwin-protos.h	2010-08-15 16:44:14.000000000 -0400
@@ -83,15 +83,10 @@ 
 extern void machopic_output_stub (FILE *, const char *, const char *);
 extern void darwin_globalize_label (FILE *, const char *);
 extern void darwin_assemble_visibility (tree, int);
-
-extern bool darwin_asm_suppress_eh_epilogue_p (void);
-extern void darwin_asm_output_dwarf_section_start_label (FILE *file, 
-							section *sect);
 extern void darwin_asm_output_dwarf_delta (FILE *, int, const char *,
 					   const char *);
 extern void darwin_asm_output_dwarf_offset (FILE *, int, const char *,
 					    section *);
-
 extern void darwin_asm_declare_constant_name (FILE *, const char *,
 					      const_tree, HOST_WIDE_INT);
 extern bool darwin_binds_local_p (const_tree);
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.c gcc-4.6-20100815/gcc/config/darwin.c
--- gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.c	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/config/darwin.c	2010-08-15 16:51:53.000000000 -0400
@@ -1666,36 +1666,6 @@ 
 	     "not supported in this configuration; ignored");
 }
 
-/* For compatibility with OSX versions that do not emit epilogues in _eh
-   frames we suppress them.  This is made a predicate function to permit 
-   us to add an OSX/FSF compatibility switch should that be required.  */
-
-bool
-darwin_asm_suppress_eh_epilogue_p (void)
-{
-  return true;
-}
-
-/* So that we can compute dwarf offsets within sections, we emit a known
-   section marker at the begining of the section.  This is distinct from
-   the ones emitted by dwarf2out.  The label is constructed by extracting
-   sectname from __DWARF,__sectname,etc,etc.  The hook should be invoked
-   once, after the first switch to the section.  */
-   
-void
-darwin_asm_output_dwarf_section_start_label (FILE *file, section *sect)
-{
-  const char *dnam;
-  int namelen;
-  gcc_assert (sect && (sect->common.flags & (SECTION_NAMED|SECTION_DEBUG)));
-  dnam = ((struct named_section *)sect)->name;
-  gcc_assert (strncmp (dnam, "__DWARF,", 8) == 0);
-  gcc_assert (strchr (dnam + 8, ','));
-
-  namelen = strchr (dnam + 8, ',') - (dnam + 8);
-  fprintf (file, "Lsection%.*s:\n", namelen, dnam + 8);
-}
-
 /* Output a difference of two labels that will be an assembly time
    constant if the two labels are local.  (.long lab1-lab2 will be
    very different if lab1 is at the boundary between two sections; it
@@ -1914,6 +1884,21 @@ 
       flag_reorder_blocks_and_partition = 0;
       flag_reorder_blocks = 1;
     }
+    
+  if (flag_asynchronous_unwind_tables)
+    {
+      if (flag_asynchronous_unwind_tables == 2)
+	flag_asynchronous_unwind_tables = 0;
+      else
+	/* Issue a warning */
+	warning (OPT_Wall,
+	  "this architecture does not fully support"
+	  " -fasynchronous-unwind-tables");
+    }
+
+  if ((flag_exceptions || flag_non_call_exceptions)
+       && strverscmp (darwin_macosx_version_min, "10.4") >= 0)
+    flag_unwind_tables = 1;
 
   if (flag_mkernel || flag_apple_kext)
     {
@@ -1927,6 +1912,9 @@ 
       flag_exceptions = 0;
       /* No -fnon-call-exceptions data in kexts.  */
       flag_non_call_exceptions = 0;
+      /* so no tables either.. */
+      flag_unwind_tables = 0;
+      flag_asynchronous_unwind_tables = 0;
       /* We still need to emit branch islands for kernel context.  */
       darwin_emit_branch_islands = true;
     }
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.h gcc-4.6-20100815/gcc/config/darwin.h
--- gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.h	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/config/darwin.h	2010-08-15 16:48:16.000000000 -0400
@@ -672,6 +672,7 @@ 
    Make Objective-C internal symbols local and in doing this, we need 
    to accommodate the name mangling done by c++ on file scope locals.  */
 
+
 int darwin_label_is_anonymous_local_objc_name (const char *name);
 
 #undef	ASM_OUTPUT_LABELREF
@@ -929,16 +930,6 @@ 
    ? (DW_EH_PE_pcrel | DW_EH_PE_indirect | DW_EH_PE_sdata4) : \
      ((CODE) == 1 || (GLOBAL) == 0) ? DW_EH_PE_pcrel : DW_EH_PE_absptr)
 
-/* Mark the start of each dwarf debug section to allow us to compute local
-   offsets within the sections.  We do this in darwin, rather than emitting
-   relocs.  */
-#define TARGET_ASM_OUTPUT_DWARF_SECTION_START_LABEL \
-	darwin_asm_output_dwarf_section_start_label
-
-/* For OSX compatibility we do not want to emit epilogues in _eh frames.  */
-#define TARGET_ASM_SUPPRESS_EH_EPILOGUE_P \
-	darwin_asm_suppress_eh_epilogue_p
-
 #define ASM_OUTPUT_DWARF_DELTA(FILE,SIZE,LABEL1,LABEL2)  \
   darwin_asm_output_dwarf_delta (FILE, SIZE, LABEL1, LABEL2)
 
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/dwarf2out.c gcc-4.6-20100815/gcc/dwarf2out.c
--- gcc-4.6-20100815.old_eh_patch/gcc/dwarf2out.c	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/dwarf2out.c	2010-08-15 16:51:53.000000000 -0400
@@ -268,12 +268,20 @@ 
 }
 dw_cfi_oprnd;
 
+/* Use the first out-of-band opcode number as a marker for the start of 
+   epilogues.  */
+#define DW_CFA_INTERNAL_start_epilogue 0x100
+#define CFI_T enum dwarf_call_frame_info
+
+static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc (unsigned int cfi);
+static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc (unsigned int cfi);
+
 typedef struct GTY(()) dw_cfi_struct {
   dw_cfi_ref dw_cfi_next;
-  enum dwarf_call_frame_info dw_cfi_opc;
-  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)")))
+  unsigned int dw_cfi_opc;
+  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc ((unsigned int)%1.dw_cfi_opc)")))
     dw_cfi_oprnd1;
-  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)")))
+  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc ((unsigned int)%1.dw_cfi_opc)")))
     dw_cfi_oprnd2;
 }
 dw_cfi_node;
@@ -720,9 +728,7 @@ 
       return "DW_CFA_GNU_args_size";
     case DW_CFA_GNU_negative_offset_extended:
       return "DW_CFA_GNU_negative_offset_extended";
-    case DW_CFA_GNU_start_epilogue:
-      return "DW_CFA_GNU_start_epilogue";
-      
+
     default:
       return "DW_CFA_<unknown>";
     }
@@ -803,9 +809,6 @@ 
 /* True if remember_state should be emitted before following CFI directive.  */
 static bool emit_cfa_remember;
 
-/* True if start_epilogue should be emitted before following CFI directive.  */
-static bool emit_cfa_start_epilogue;
-
 /* Add CFI to the current fde at the PC value indicated by LABEL if specified,
    or to the CIE if LABEL is NULL.  */
 
@@ -814,17 +817,6 @@ 
 {
   dw_cfi_ref *list_head;
 
-  if (emit_cfa_start_epilogue)
-    {
-      dw_cfi_ref cfi_epi_start;
-
-      /* Emit the state save.  */
-      emit_cfa_start_epilogue = false;
-      cfi_epi_start = new_cfi ();
-      cfi_epi_start->dw_cfi_opc = DW_CFA_GNU_start_epilogue;
-      add_fde_cfi (label, cfi_epi_start);   
-    }
-    
   if (emit_cfa_remember)
     {
       dw_cfi_ref cfi_remember;
@@ -837,8 +829,14 @@ 
     }
 
   list_head = &cie_cfi_head;
-
-  if (dwarf2out_do_cfi_asm ())
+  
+  if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
+    {
+      dw_fde_ref fde = current_fde ();
+      gcc_assert (fde != NULL);
+      list_head = &fde->dw_fde_cfi;
+    }
+  else if (dwarf2out_do_cfi_asm ())
     {
       if (label)
 	{
@@ -866,6 +864,7 @@ 
 		case DW_CFA_def_cfa_sf:
 		case DW_CFA_def_cfa_expression:
 		case DW_CFA_restore_state:
+		case DW_CFA_INTERNAL_start_epilogue:
 		  if (*label == 0 || strcmp (label, "<do not output>") == 0)
 		    label = dwarf2out_cfi_label (true);
 
@@ -2910,14 +2909,19 @@ 
     return;
 
   /* Otherwise, search forward to see if the return insn was the last
-     basic block of the function.  If so, we don't need save/restore.  */
+     basic block of the function.  If so, we don't need save/restore.  
+     However, we do mark the position so that we can skip the epilogue
+     in _eh frames where required.  */
   gcc_assert (i != NULL);
   i = next_real_insn (i);
   if (i == NULL)
     {
-      /* But we do mark the start of the epilogue to allow it to be skipped
-         in _eh frames.  */
-      emit_cfa_start_epilogue = true; 
+      dw_cfi_ref cfi_epi_start;
+
+      /* Emit a marker for the epilogue start. */
+      cfi_epi_start = new_cfi ();
+      cfi_epi_start->dw_cfi_opc = DW_CFA_INTERNAL_start_epilogue;
+      add_fde_cfi ("", cfi_epi_start);   
       return;
     }
 
@@ -2962,11 +2966,9 @@ 
 }
 
 /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used.  */
-static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc
- (enum dwarf_call_frame_info cfi);
 
 static enum dw_cfi_oprnd_type
-dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi)
+dw_cfi_oprnd1_desc (unsigned int cfi)
 {
   switch (cfi)
     {
@@ -2974,7 +2976,7 @@ 
     case DW_CFA_GNU_window_save:
     case DW_CFA_remember_state:
     case DW_CFA_restore_state:
-    case DW_CFA_GNU_start_epilogue:
+    case DW_CFA_INTERNAL_start_epilogue:
       return dw_cfi_oprnd_unused;
 
     case DW_CFA_set_loc:
@@ -3012,11 +3014,9 @@ 
 }
 
 /* Describe for the GTY machinery what parts of dw_cfi_oprnd2 are used.  */
-static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc
- (enum dwarf_call_frame_info cfi);
 
 static enum dw_cfi_oprnd_type
-dw_cfi_oprnd2_desc (enum dwarf_call_frame_info cfi)
+dw_cfi_oprnd2_desc (unsigned int cfi)
 {
   switch (cfi)
     {
@@ -3143,10 +3143,13 @@ 
       dw2_asm_output_data (1, (cfi->dw_cfi_opc | (r & 0x3f)),
 			   "DW_CFA_restore, column %#lx", r);
     }
-  else if (cfi->dw_cfi_opc == DW_CFA_GNU_start_epilogue)
-/* DEBUG */
-    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
-    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+  else if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
+    {
+      /* This is a nop unless we want a debug message.  */
+      if (flag_debug_asm)
+	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
+    }
   else
     {
       dw2_asm_output_data (1, cfi->dw_cfi_opc,
@@ -3329,10 +3332,11 @@ 
 	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
       break;
 
-    case DW_CFA_GNU_start_epilogue:
-/*DEBUG */
-    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
-    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+    case DW_CFA_INTERNAL_start_epilogue:
+      /* no-op apart from an informational message.  */
+      if (flag_debug_asm)
+	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
       break;
       
     case DW_CFA_remember_state:
@@ -3537,10 +3541,13 @@ 
 emit_cfi_or_skip_epilogue (dw_cfi_ref cfi, dw_fde_ref fde, bool for_eh)
 {
   if (for_eh 
-      && targetm.asm_out.suppress_eh_epilogue_p())
+      && !flag_asynchronous_unwind_tables)
     {
       if (cfi->dw_cfi_opc == DW_CFA_remember_state)
 	{
+	  if (flag_debug_asm)
+	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
+	      " DW_CFA_remember/restore_state pair skipped\n",asm_out_file);
 	  /* Skip to the restore, unless there's an error and we fall off
 	     the end.  */
 	  while (cfi->dw_cfi_next 
@@ -3548,11 +3555,11 @@ 
 	    cfi = cfi->dw_cfi_next;
 	  return cfi;
         }
-      if (cfi->dw_cfi_opc == DW_CFA_GNU_start_epilogue)
+      if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
         {
-/*DEBUG */
-    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
-    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+	  if (flag_debug_asm)
+	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
 	  while (cfi->dw_cfi_next) 
 	    /* Skip to the end.  */
 	    cfi = cfi->dw_cfi_next;
@@ -3560,7 +3567,9 @@ 
         }
     }
 
-  /* if it's not a special case, then just emit it.  */
+  /* If it's not a special case, then just carry on.  
+     This will also cause the no-op 'DW_CFA_INTERNAL_start_epilogue' to be
+     listed when flag_debug_asm is set.  */
   output_cfi (cfi, fde, for_eh);
   return cfi;
 }
@@ -3694,7 +3703,6 @@ 
   else
     {
       dw_cfi_ref cfi_next = fde->dw_fde_cfi;
-
       if (fde->dw_fde_switch_cfi)
 	{
 	  cfi_next = fde->dw_fde_switch_cfi->dw_cfi_next;
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/target.def gcc-4.6-20100815/gcc/target.def
--- gcc-4.6-20100815.old_eh_patch/gcc/target.def	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/target.def	2010-08-15 16:44:16.000000000 -0400
@@ -390,15 +390,6 @@ 
  void, (FILE *file, int size, rtx x),
  NULL)
 
-/* Targets might not need epilogue information in dwarf2 _eh frames.  This
-   hook should return true if the epilogue should be suppressed in such frames.
-   Epilogues will still be emitted in _debug_frames.  */
-DEFHOOK_UNDOC
-(suppress_eh_epilogue_p,
- "",
- bool, (void),
- hook_bool_void_false)
-
 /* Some target machines need to postscan each insn after it is output.  */
 DEFHOOK
 (final_postscan_insn,
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/toplev.c gcc-4.6-20100815/gcc/toplev.c
--- gcc-4.6-20100815.old_eh_patch/gcc/toplev.c	2010-08-16 00:00:52.000000000 -0400
+++ gcc-4.6-20100815/gcc/toplev.c	2010-08-15 16:51:53.000000000 -0400
@@ -1798,9 +1798,9 @@ 
   if (flag_rename_registers == AUTODETECT_VALUE)
     flag_rename_registers = flag_unroll_loops || flag_peel_loops;
 
-  if (flag_non_call_exceptions)
+  if (flag_non_call_exceptions && flag_asynchronous_unwind_tables == 2)
     flag_asynchronous_unwind_tables = 1;
-  if (flag_asynchronous_unwind_tables)
+  if (flag_asynchronous_unwind_tables || flag_non_call_exceptions)
     flag_unwind_tables = 1;
 
   if (flag_value_profile_transformations)
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/include/dwarf2.h gcc-4.6-20100815/include/dwarf2.h
--- gcc-4.6-20100815.old_eh_patch/include/dwarf2.h	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/include/dwarf2.h	2010-08-15 16:45:11.000000000 -0400
@@ -854,8 +854,7 @@ 
     /* GNU extensions.  */
     DW_CFA_GNU_window_save = 0x2d,
     DW_CFA_GNU_args_size = 0x2e,
-    DW_CFA_GNU_negative_offset_extended = 0x2f,
-    DW_CFA_GNU_start_epilogue = 0x30
+    DW_CFA_GNU_negative_offset_extended = 0x2f
   };
 
 #define DW_CIE_ID	  0xffffffff