debug/96383 - emit debug info for used external functions

Message ID nycvar.YFH.7.76.2007301154380.9963@zhemvz.fhfr.qr
State New
Headers show
Series
  • debug/96383 - emit debug info for used external functions
Related show

Commit Message

Richard Biener July 30, 2020, 9:55 a.m.
This makes sure to emit full declaration DIEs including
formal parameters for used external functions.  This helps
debugging when debug information of the external entity is
not available and also helps external tools cross-checking
ABI compatibility which was the bug reporters use case.

For cc1 this affects debug information size as follows:

     VM SIZE                     FILE SIZE
 ++++++++++++++ GROWING       ++++++++++++++
  [ = ]       0 .debug_info   +1.63Mi  +1.3%
  [ = ]       0 .debug_str     +263Ki  +3.4%
  [ = ]       0 .debug_abbrev  +101Ki  +4.9%
  [ = ]       0 .debug_line   +5.71Ki  +0.0%
   +44%     +16 [Unmapped]        +48  +1.2%

 -------------- SHRINKING     --------------
  [ = ]       0 .debug_loc       -213  -0.0%
  -0.0%     -48 .text             -48  -0.0%
  [ = ]       0 .debug_ranges     -16  -0.0%

  -0.0%     -32 TOTAL         +1.99Mi  +0.6%

and DWARF compression via DWZ can only shave off minor bits
here.

Previously we emitted no DIEs for external functions at all
unless they were referenced via DW_TAG_GNU_call_site which
for some GCC revs caused a regular DIE to appear and since
GCC 4.9 only a stub without formal parameters.  This means
at -O0 we did not emit any DIE for external functions
but with optimization we emitted stubs.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

OK for trunk and backports?

Thanks,
Richard.

2020-07-30  Richard Biener  <rguenther@suse.de>

	PR debug/96383
	* cgraphunit.c (symbol_table::finalize_compilation_unit):
	Emit debug information for all used functions, not for
	definitions.

	* gcc.dg/debug/dwarf2/pr96383-1.c: New testcase.
	* gcc.dg/debug/dwarf2/pr96383-2.c: Likewise.
---
 gcc/cgraphunit.c                              |  2 +-
 gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c | 17 +++++++++++++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-2.c | 17 +++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-2.c

-- 
2.26.2

Comments

Armin Brauns via Gcc-patches July 30, 2020, 10:09 a.m. | #1
On Thu, Jul 30, 2020 at 11:55:19AM +0200, Richard Biener wrote:
> For cc1 this affects debug information size as follows:

> 

>      VM SIZE                     FILE SIZE

>  ++++++++++++++ GROWING       ++++++++++++++

>   [ = ]       0 .debug_info   +1.63Mi  +1.3%

>   [ = ]       0 .debug_str     +263Ki  +3.4%

>   [ = ]       0 .debug_abbrev  +101Ki  +4.9%

>   [ = ]       0 .debug_line   +5.71Ki  +0.0%

>    +44%     +16 [Unmapped]        +48  +1.2%

> 

>  -------------- SHRINKING     --------------

>   [ = ]       0 .debug_loc       -213  -0.0%

>   -0.0%     -48 .text             -48  -0.0%

>   [ = ]       0 .debug_ranges     -16  -0.0%

> 

>   -0.0%     -32 TOTAL         +1.99Mi  +0.6%

> 

> and DWARF compression via DWZ can only shave off minor bits

> here.


I'm surprised DWZ doesn't shave off more, the prototypes in
the different TUs should be the same and therefore mergeable.

What could help a little if DWZ is able to merge the prototypes with
definition DIE (verify that the type is the same and there is no information
the definition doesn't have appart from DW_AT_external).

> Bootstrap and regtest running on x86_64-unknown-linux-gnu.

> 

> OK for trunk and backports?


I'd go with it for trunk and 10.2.1 now and consider further backports
later.  Maybe even defer the 10.2.1 backport for two weeks.

	Jakub
Andreas Schwab July 30, 2020, 10:22 a.m. | #2
On Jul 30 2020, Richard Biener wrote:

> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c

> new file mode 100644

> index 00000000000..ede30f9a95e

> --- /dev/null

> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c

> @@ -0,0 +1,17 @@

> +/* { dg-do compile } */

> +/* { dg-options "-g -gdwarf -dA" } */

> +

> +extern void foo (int);

> +extern void bar (int);

> +

> +int main()

> +{

> +  foo (1);

> +}

> +

> +/* We want subprogram DIEs for both foo and main and a DIE for

> +   the formal parameter of foo.  We do not want a DIE for

> +   unusedbar.  */

> +/* { dg-final { scan-assembler-times "DW_TAG_subprogram" 4 } } */

> +/* { dg-final { scan-assembler-times "DW_TAG_formal_parameter" 2 } } */

> +/* { dg-final { scan-assembler-not "unusedbar" } } */


The test doesn't reference unusedbar at all, is that intended?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Richard Biener July 30, 2020, 10:40 a.m. | #3
On Thu, 30 Jul 2020, Andreas Schwab wrote:

> On Jul 30 2020, Richard Biener wrote:

> 

> > diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c

> > new file mode 100644

> > index 00000000000..ede30f9a95e

> > --- /dev/null

> > +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c

> > @@ -0,0 +1,17 @@

> > +/* { dg-do compile } */

> > +/* { dg-options "-g -gdwarf -dA" } */

> > +

> > +extern void foo (int);

> > +extern void bar (int);

> > +

> > +int main()

> > +{

> > +  foo (1);

> > +}

> > +

> > +/* We want subprogram DIEs for both foo and main and a DIE for

> > +   the formal parameter of foo.  We do not want a DIE for

> > +   unusedbar.  */

> > +/* { dg-final { scan-assembler-times "DW_TAG_subprogram" 4 } } */

> > +/* { dg-final { scan-assembler-times "DW_TAG_formal_parameter" 2 } } */

> > +/* { dg-final { scan-assembler-not "unusedbar" } } */

> 

> The test doesn't reference unusedbar at all, is that intended?


Whoops, fixed.

Richard.
Richard Biener July 30, 2020, 11:12 a.m. | #4
On Thu, 30 Jul 2020, Jakub Jelinek wrote:

> On Thu, Jul 30, 2020 at 11:55:19AM +0200, Richard Biener wrote:

> > For cc1 this affects debug information size as follows:

> > 

> >      VM SIZE                     FILE SIZE

> >  ++++++++++++++ GROWING       ++++++++++++++

> >   [ = ]       0 .debug_info   +1.63Mi  +1.3%

> >   [ = ]       0 .debug_str     +263Ki  +3.4%

> >   [ = ]       0 .debug_abbrev  +101Ki  +4.9%

> >   [ = ]       0 .debug_line   +5.71Ki  +0.0%

> >    +44%     +16 [Unmapped]        +48  +1.2%

> > 

> >  -------------- SHRINKING     --------------

> >   [ = ]       0 .debug_loc       -213  -0.0%

> >   -0.0%     -48 .text             -48  -0.0%

> >   [ = ]       0 .debug_ranges     -16  -0.0%

> > 

> >   -0.0%     -32 TOTAL         +1.99Mi  +0.6%

> > 

> > and DWARF compression via DWZ can only shave off minor bits

> > here.

> 

> I'm surprised DWZ doesn't shave off more, the prototypes in

> the different TUs should be the same and therefore mergeable.


Yeah, DWZ shaves off 60% of the .debug_info overhead, but due
to the size of cc1 I have not tried figuring out what the difference
in the end will be.

> What could help a little if DWZ is able to merge the prototypes with

> definition DIE (verify that the type is the same and there is no information

> the definition doesn't have appart from DW_AT_external).


DWZ could split out a shareable specification DIE from definition
DIEs and use DW_AT_specification.  When we bring declarations into
scope elsewhere we have to avoid refering to definitions there.

> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.

> > 

> > OK for trunk and backports?

> 

> I'd go with it for trunk and 10.2.1 now and consider further backports

> later.  Maybe even defer the 10.2.1 backport for two weeks.


So testing revealed I have to be more careful what nodes I create
DIEs for.  For example for g++.dg/debug/dwarf2/cdtor-1.C we'd now
end up with DIEs for all of the DTOR aliases (and IIRC aliases are
not subject to unused removal, nor unused members of a comdat group).

In particular guarding the nodes with

        if (!cnode->alias && !cnode->thunk.thunk_p
            && !fndecl_built_in_p (cnode->decl))

seems to fix the few regressions observed.  I'm retesting fully with
that.

Richard.

Patch

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index ea9a34bda6f..3873cf1195d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2989,7 +2989,7 @@  symbol_table::finalize_compilation_unit (void)
       /* Emit early debug for reachable functions, and by consequence,
 	 locally scoped symbols.  */
       struct cgraph_node *cnode;
-      FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (cnode)
+      FOR_EACH_FUNCTION (cnode)
 	(*debug_hooks->early_global_decl) (cnode->decl);
 
       /* Clean up anything that needs cleaning up after initial debug
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c
new file mode 100644
index 00000000000..ede30f9a95e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-1.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-g -gdwarf -dA" } */
+
+extern void foo (int);
+extern void bar (int);
+
+int main()
+{
+  foo (1);
+}
+
+/* We want subprogram DIEs for both foo and main and a DIE for
+   the formal parameter of foo.  We do not want a DIE for
+   unusedbar.  */
+/* { dg-final { scan-assembler-times "DW_TAG_subprogram" 4 } } */
+/* { dg-final { scan-assembler-times "DW_TAG_formal_parameter" 2 } } */
+/* { dg-final { scan-assembler-not "unusedbar" } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-2.c
new file mode 100644
index 00000000000..c3a710e2f89
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr96383-2.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-g -O2 -gdwarf -dA" } */
+
+extern void foo (int);
+extern void unusedbar (int);
+
+int main()
+{
+  foo (1);
+}
+
+/* We want subprogram DIEs for both foo and main and a DIE for
+   the formal parameter of foo.  We do not want a DIE for
+   unusedbar.  */
+/* { dg-final { scan-assembler-times "DW_TAG_subprogram" 4 } } */
+/* { dg-final { scan-assembler-times "DW_TAG_formal_parameter" 2 } } */
+/* { dg-final { scan-assembler-not "unusedbar" } } */