DWARF: Match behaviour of .cfi_xxx when doing manual frame output.

Message ID 20211117171718.59536-1-iain@sandoe.co.uk
State New
Headers
Series DWARF: Match behaviour of .cfi_xxx when doing manual frame output. |

Commit Message

Iain Sandoe Nov. 17, 2021, 5:17 p.m. UTC
  At present, for several reasons, it is not possible to switch
Darwin to use .cfi instructions for frame output.

When GCC uses .cfi_ instructions, the behaviour w.r.t frame
sections (for a target with unwind frames by defaults):

(no options ) .eh_frame
(-g ) .eh_frame
(-g -fno-unwind-tables -fno-asynchronous-unwind-tables) .debug_frame
(   -fno-unwind-tables -fno-asynchronous-unwind-tables) ---

However, for a target which outputs the FDEs "manually" (using
output_call_frame_info()) we have:

(no options ) __eh_frame
(-g ) __eh_frame *and* __debug_frame
(-g -fno-unwind-tables -fno-asynchronous-unwind-tables) __debug_frame
(   -fno-unwind-tables -fno-asynchronous-unwind-tables) ---

The first two cases are, of course, the most common and the extra
frame table is (a) a waste of space and (b) actually triggers a bug
when used with the LLVM assembler [with assertions enabled] for
Mach-O when we have hot/cold partitioning on, since that emits
Letext{.cold}0 labels *after* the __DWARF,__debug_frame and the
assembler is set up reject switches to non-debug sections after the
first __DWARF debug one has been seen.

The following patch makes the manual output of frame data follow the
same pattern as the .cfi instructions.

(a) From testing on Darwin which uses the 'manual frame output' I see
around 200Mb saving on gcc/ for master (5%).
(b) Since Darwin defaults to unwind frames for all languages, we see
only eh_frame sections before the "real debug" is emitted, so that
the LLVM constraint is avoided.

On testing on x86_64 and powerpc64le Linux, I see only a single test
that would need amendment (it counts the number of references to the
start/end local labels).

Since the majority of targets are using .cfi instructions, it is hard
to get wider testing.

It would be possible, of course, to wrap the change in a target hook
but it's not clear that we need to.

Is there some case that I've missed?
or - OK for master (the testcase amendments are not attached here)
but are simple.

thanks,
Iain

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

gcc/ChangeLog:

	* dwarf2out.c (output_call_frame_info): Output the FDEs when
	either EH or debug support is needed.
	(dwarf2out_frame_finish): When either EH or debug support is
	needed, call output_call_frame_info().
---
 gcc/dwarf2out.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
  

Comments

Iain Sandoe Dec. 2, 2021, 9:54 a.m. UTC | #1
Hi,

I’d like to ping this patch at least for opinion on whether the approach is
reasonable (as noted below it could be put behind a target hook if necessary)

thanks
Iain

> On 17 Nov 2021, at 17:17, Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> At present, for several reasons, it is not possible to switch
> Darwin to use .cfi instructions for frame output.
> 
> When GCC uses .cfi_ instructions, the behaviour w.r.t frame
> sections (for a target with unwind frames by defaults):
> 
> (no options ) .eh_frame
> (-g ) .eh_frame
> (-g -fno-unwind-tables -fno-asynchronous-unwind-tables) .debug_frame
> (   -fno-unwind-tables -fno-asynchronous-unwind-tables) ---
> 
> However, for a target which outputs the FDEs "manually" (using
> output_call_frame_info()) we have:
> 
> (no options ) __eh_frame
> (-g ) __eh_frame *and* __debug_frame
> (-g -fno-unwind-tables -fno-asynchronous-unwind-tables) __debug_frame
> (   -fno-unwind-tables -fno-asynchronous-unwind-tables) ---
> 
> The first two cases are, of course, the most common and the extra
> frame table is (a) a waste of space and (b) actually triggers a bug
> when used with the LLVM assembler [with assertions enabled] for
> Mach-O when we have hot/cold partitioning on, since that emits
> Letext{.cold}0 labels *after* the __DWARF,__debug_frame and the
> assembler is set up reject switches to non-debug sections after the
> first __DWARF debug one has been seen.
> 
> The following patch makes the manual output of frame data follow the
> same pattern as the .cfi instructions.
> 
> (a) From testing on Darwin which uses the 'manual frame output' I see
> around 200Mb saving on gcc/ for master (5%).
> (b) Since Darwin defaults to unwind frames for all languages, we see
> only eh_frame sections before the "real debug" is emitted, so that
> the LLVM constraint is avoided.
> 
> On testing on x86_64 and powerpc64le Linux, I see only a single test
> that would need amendment (it counts the number of references to the
> start/end local labels).
> 
> Since the majority of targets are using .cfi instructions, it is hard
> to get wider testing.
> 
> It would be possible, of course, to wrap the change in a target hook
> but it's not clear that we need to.
> 
> Is there some case that I've missed?
> or - OK for master (the testcase amendments are not attached here)
> but are simple.
> 
> thanks,
> Iain
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> 
> gcc/ChangeLog:
> 
> 	* dwarf2out.c (output_call_frame_info): Output the FDEs when
> 	either EH or debug support is needed.
> 	(dwarf2out_frame_finish): When either EH or debug support is
> 	needed, call output_call_frame_info().
> ---
> gcc/dwarf2out.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index e1d6a79ecd7..96307d6747a 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -283,7 +283,7 @@ static GTY(()) dw_die_ref decltype_auto_die;
> 
> /* Forward declarations for functions defined in this file.  */
> 
> -static void output_call_frame_info (int);
> +static void output_call_frame_info (bool, bool);
> 
> /* Personality decl of current unit.  Used only when assembler does not support
>    personality CFI.  */
> @@ -750,7 +750,7 @@ fde_needed_for_eh_p (dw_fde_ref fde)
>    location of saved registers.  */
> 
> static void
> -output_call_frame_info (int for_eh)
> +output_call_frame_info (bool for_eh, bool for_debug)
> {
>   unsigned int i;
>   dw_fde_ref fde;
> @@ -795,7 +795,7 @@ output_call_frame_info (int for_eh)
> 	    targetm.asm_out.emit_unwind_label (asm_out_file, fde->decl, 1, 1);
> 	}
> 
> -      if (!any_eh_needed)
> +      if (!any_eh_needed && !for_debug)
> 	return;
>     }
> 
> @@ -1271,12 +1271,9 @@ void
> dwarf2out_frame_finish (void)
> {
>   /* Output call frame information.  */
> -  if (targetm.debug_unwind_info () == UI_DWARF2)
> -    output_call_frame_info (0);
> -
> -  /* Output another copy for the unwinder.  */
> -  if (do_eh_frame)
> -    output_call_frame_info (1);
> +  if (targetm.debug_unwind_info () == UI_DWARF2 || do_eh_frame)
> +    output_call_frame_info (do_eh_frame,
> +			    targetm.debug_unwind_info () == UI_DWARF2);
> }
> 
> static void var_location_switch_text_section (void);
> -- 
> 2.24.3 (Apple Git-128)
>
  

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index e1d6a79ecd7..96307d6747a 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -283,7 +283,7 @@  static GTY(()) dw_die_ref decltype_auto_die;
 
 /* Forward declarations for functions defined in this file.  */
 
-static void output_call_frame_info (int);
+static void output_call_frame_info (bool, bool);
 
 /* Personality decl of current unit.  Used only when assembler does not support
    personality CFI.  */
@@ -750,7 +750,7 @@  fde_needed_for_eh_p (dw_fde_ref fde)
    location of saved registers.  */
 
 static void
-output_call_frame_info (int for_eh)
+output_call_frame_info (bool for_eh, bool for_debug)
 {
   unsigned int i;
   dw_fde_ref fde;
@@ -795,7 +795,7 @@  output_call_frame_info (int for_eh)
 	    targetm.asm_out.emit_unwind_label (asm_out_file, fde->decl, 1, 1);
 	}
 
-      if (!any_eh_needed)
+      if (!any_eh_needed && !for_debug)
 	return;
     }
 
@@ -1271,12 +1271,9 @@  void
 dwarf2out_frame_finish (void)
 {
   /* Output call frame information.  */
-  if (targetm.debug_unwind_info () == UI_DWARF2)
-    output_call_frame_info (0);
-
-  /* Output another copy for the unwinder.  */
-  if (do_eh_frame)
-    output_call_frame_info (1);
+  if (targetm.debug_unwind_info () == UI_DWARF2 || do_eh_frame)
+    output_call_frame_info (do_eh_frame,
+			    targetm.debug_unwind_info () == UI_DWARF2);
 }
 
 static void var_location_switch_text_section (void);