Patch RFA: Emit .cfi_sections after some input code has been seen

Message ID CAOyqgcX1Zu94=-cW_qMN2roPxfm+7h_3-6Yvk=fP48HmHepQ+g@mail.gmail.com
State New
Headers show
Series
  • Patch RFA: Emit .cfi_sections after some input code has been seen
Related show

Commit Message

Ian Lance Taylor Sept. 18, 2019, 4:34 a.m.
This seemingly innocuous change

2019-09-11  Richard Biener  <rguenther@suse.de>

* lto-opts.c (lto_write_options): Stream -g when debug is enabled.
* lto-wrapper.c (merge_and_complain): Pick up -g.
(append_compiler_options): Likewise.
(run_gcc): Re-instantiate handling -g0 at link-time.
* doc/invoke.texi (flto): Document debug info generation.

caused PR 91763, a test failure building Go code with -flto.  The
problem only arose when using the GNU assembler on Solaris.

The bug is that when emitting debug info but not exception info, and
when using gas, the DWARF code will emit
    .cfi_sections .debug_frame
This will direct gas to emit unwind info into .debug_frame but not .eh_frame.

Go code requires unwind info, and the Go library expects it to be in
.eh_frame.  The Go frontend always turns on exceptions, so this
.cfi_sections directive is not used.

However, when using -flto, the lto1 program decides whether it is
using exceptions based on what it reads from the input files.  Before
lto1 sees any input files, flag_exceptions will have its default value
of 0.  And lto1 initializes the debug info before seeing any input
files, so the debug initialization thinks that exceptions are not in
use, and emits the .cfi_sections directive.

This problem was uncovered by the above patch because Go code also
turns on debugging by default, and so lto1 now sees a -g option that
it did not see before.

This patch fixes the problem by moving the emission of .cfi_sections
from debug init to the first time that the debug info needs to know
whether CFI is supported.  This is only done when actually emitting
debug info, and therefore after some input files have been read.

Bootstrapped and ran full testsuite on x86_64-pc-linux-gnu.  Tested
that formerly failing case now passes on sparc-sun-solaris2.11.

OK for trunk?

Ian


2019-09-17  Ian Lance Taylor  <iant@golang.org>

PR go/91763
* dwarf2out.c (dwarf2out_assembly_start): Move ".cfi_sections
.debug_frame" output from here...
* dwarf2cfi.c (dwarf2out_do_cfi_asm): ...to here.

Comments

Richard Biener Sept. 18, 2019, 7:40 a.m. | #1
On Tue, 17 Sep 2019, Ian Lance Taylor wrote:

> This seemingly innocuous change

> 

> 2019-09-11  Richard Biener  <rguenther@suse.de>

> 

> * lto-opts.c (lto_write_options): Stream -g when debug is enabled.

> * lto-wrapper.c (merge_and_complain): Pick up -g.

> (append_compiler_options): Likewise.

> (run_gcc): Re-instantiate handling -g0 at link-time.

> * doc/invoke.texi (flto): Document debug info generation.

> 

> caused PR 91763, a test failure building Go code with -flto.  The

> problem only arose when using the GNU assembler on Solaris.

> 

> The bug is that when emitting debug info but not exception info, and

> when using gas, the DWARF code will emit

>     .cfi_sections .debug_frame

> This will direct gas to emit unwind info into .debug_frame but not .eh_frame.

> 

> Go code requires unwind info, and the Go library expects it to be in

> .eh_frame.  The Go frontend always turns on exceptions, so this

> .cfi_sections directive is not used.

> 

> However, when using -flto, the lto1 program decides whether it is

> using exceptions based on what it reads from the input files.  Before

> lto1 sees any input files, flag_exceptions will have its default value

> of 0.  And lto1 initializes the debug info before seeing any input

> files, so the debug initialization thinks that exceptions are not in

> use, and emits the .cfi_sections directive.

> 

> This problem was uncovered by the above patch because Go code also

> turns on debugging by default, and so lto1 now sees a -g option that

> it did not see before.

> 

> This patch fixes the problem by moving the emission of .cfi_sections

> from debug init to the first time that the debug info needs to know

> whether CFI is supported.  This is only done when actually emitting

> debug info, and therefore after some input files have been read.

> 

> Bootstrapped and ran full testsuite on x86_64-pc-linux-gnu.  Tested

> that formerly failing case now passes on sparc-sun-solaris2.11.

> 

> OK for trunk?


Hmm.  To me it looks like there's nothing guaranteeing that
flag_exceptions is initialized appropriately since it's
set on function body read-in which is now on-demand.  So
I'm not sure that we cannot have functions output into
assembly before flag_exceptions is initialized.

Also dwarf2out_do_cfi_asm is a predicate which makes it
an awkward point.  Maybe at the time we emit the first
.cfi assembly instruction would be the correct (and latest)
point in time to emit this directive?

Anyways, I am testing the patch below which initializes
flag_exceptions before dwarf2out_assembly_start by
moving the initialization to a central place.

The lto_input_ts_function_decl_tree_pointers made this work
for most languages but not for those not having a language-specific
personality routine, so I have to check flag_exceptions as well
(the decls struct function are not input yet, but we do save
the CUs -fexception setting accordingly).

Note this doesn't solve PR91794 where the same issue for
-funwind-tables setting applies, but the fix could look
similar.

LTO bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-09-18  Richard Biener  <rguenther@suse.de>

	PR lto/91763
	* lto-streamer-in.c (input_eh_regions): Move EH init to
	lto_materialize_function.
	* tree-streamer-in.c (lto_input_ts_function_decl_tree_pointers):
	Likewise.

	lto/
	* lto.c (lto_materialize_function): Initialize EH by looking
	at the function personality and flag_exceptions setting.

Index: gcc/lto-streamer-in.c
===================================================================
--- gcc/lto-streamer-in.c	(revision 275800)
+++ gcc/lto-streamer-in.c	(working copy)
@@ -615,11 +615,6 @@ input_eh_regions (struct lto_input_block
 
   lto_tag_check_range (tag, LTO_eh_table, LTO_eh_table);
 
-  /* If the file contains EH regions, then it was compiled with
-     -fexceptions.  In that case, initialize the backend EH
-     machinery.  */
-  lto_init_eh ();
-
   gcc_assert (fn->eh);
 
   root_region = streamer_read_hwi (ib);
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	(revision 275800)
+++ gcc/tree-streamer-in.c	(working copy)
@@ -800,12 +800,6 @@ lto_input_ts_function_decl_tree_pointers
       }
   }
 #endif
-
-  /* If the file contains a function with an EH personality set,
-     then it was compiled with -fexceptions.  In that case, initialize
-     the backend EH machinery.  */
-  if (DECL_FUNCTION_PERSONALITY (expr))
-    lto_init_eh ();
 }
 
 
Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 275800)
+++ gcc/lto/lto.c	(working copy)
@@ -218,6 +218,12 @@ lto_materialize_function (struct cgraph_
 	return;
       if (DECL_FUNCTION_PERSONALITY (decl) && !first_personality_decl)
 	first_personality_decl = DECL_FUNCTION_PERSONALITY (decl);
+      /* If the file contains a function with a language specific EH
+	 personality set or with EH enabled initialize the backend EH
+	 machinery.  */
+      if (DECL_FUNCTION_PERSONALITY (decl)
+	  || opt_for_fn (decl, flag_exceptions))
+	lto_init_eh ();
     }
 
   /* Let the middle end know about the function.  */

Patch

Index: gcc/dwarf2cfi.c
===================================================================
--- gcc/dwarf2cfi.c	(revision 275698)
+++ gcc/dwarf2cfi.c	(working copy)
@@ -3551,6 +3551,13 @@  dwarf2out_do_cfi_asm (void)
 
   /* Success!  */
   saved_do_cfi_asm = 1;
+
+  /* Emit .cfi_sections before any other .cfi directive.  We used to
+     do this in dwarf2out_assembly_start, but lto1 will call that
+     before setting flag_exceptions.  */
+  if (HAVE_GAS_CFI_SECTIONS_DIRECTIVE && !dwarf2out_do_eh_frame ())
+    fprintf (asm_out_file, "\t.cfi_sections\t.debug_frame\n");
+
   return true;
 }
 
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 275698)
+++ gcc/dwarf2out.c	(working copy)
@@ -28901,11 +28901,6 @@  dwarf2out_assembly_start (void)
 #ifdef DWARF2_LINENO_DEBUGGING_INFO
   cur_line_info_table = text_section_line_info;
 #endif
-
-  if (HAVE_GAS_CFI_SECTIONS_DIRECTIVE
-      && dwarf2out_do_cfi_asm ()
-      && !dwarf2out_do_eh_frame ())
-    fprintf (asm_out_file, "\t.cfi_sections\t.debug_frame\n");
 }
 
 /* A helper function for dwarf2out_finish called through