[v2] debug/93751 Option to generate DIEs for external variables

Hi all,

Attached is a patch adjusted per discussion in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93751

- The manual is corrected to reflect that DIEs for external variables 
are not generated when using DWARF
- A new option is introduced that implements the behavior that was 
described in the manual (and is what other debugging formats do).

Please review/apply.

Regards,
Alexey.

Patch ping.

On 2/19/20 3:30 PM, Alexey Neyman wrote:
> Hi all,
>
> Attached is a patch adjusted per discussion in 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93751
>
> - The manual is corrected to reflect that DIEs for external variables 
> are not generated when using DWARF
> - A new option is introduced that implements the behavior that was 
> described in the manual (and is what other debugging formats do).
>
> Please review/apply.
>
> Regards,
> Alexey.
>
>From 7a1b7d99a98416ed8b1c8bf2e4877655fd820fd0 Mon Sep 17 00:00:00 2001
From: Alexey Neyman <stilor@att.net>
Date: Thu, 13 Feb 2020 22:01:10 -0800
Subject: [PATCH] debug/93751 Option to create DIEs for ext. vars

-g1 is described in the manual to generate debug info for functions and
external variables. It does that for older debugging formats but not for
DWARF. This change fixes the manual to describe the current behavior and
introduces a new option, -gexternal-variables, that makes -g1 output the
DIEs for external variables, as it does for older debugging formats.

2020-02-14  Alexey Neyman  <stilor@att.net>

        PR debug/93751
        * gcc/common.opt: New option, -gexternal-variables.
        * gcc/doc/invoke.texi: Describe the new option.
        * dwarf2out.c (gen_decl_die): Proceed to generating the DIE if
        the debug level is terse, DIEs for external variables have been
        requested and the declaration is public.
        (dwarf2out_decl): Same.
        * gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-1: New test.
        * gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-2: New test.
        * gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-3: New test.

Signed-off-by: Alexey Neyman <stilor@att.net>
---
 gcc/common.opt                                |  4 ++++
 gcc/doc/invoke.texi                           | 13 +++++++++++--
 gcc/dwarf2out.c                               | 14 ++++++++++----
 gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-1.c |  7 +++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-2.c |  7 +++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-3.c |  7 +++++++
 6 files changed, 46 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-1.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-2.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-3.c

diff --git a/gcc/common.opt b/gcc/common.opt
index 5692cd04374..aec0aacb116 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3075,6 +3075,10 @@ gdwarf-
 Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
 Generate debug information in DWARF v2 (or later) format.
 
+gexternal-variables
+Common Driver RejectNegative Var(debug_external_variables)
+Generate debug information for external variables.
+
 ggdb
 Common Driver JoinedOrMissing
 Generate debug information in default extended format.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bd9ecebf103..d09cf298c36 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -443,7 +443,8 @@ Objective-C and Objective-C++ Dialects}.
 @item Debugging Options
 @xref{Debugging Options,,Options for Debugging Your Program}.
 @gccoptlist{-g  -g@var{level}  -gdwarf  -gdwarf-@var{version} @gol
--ggdb  -grecord-gcc-switches  -gno-record-gcc-switches @gol
+-gexternal-variables  -ggdb  @gol
+-grecord-gcc-switches  -gno-record-gcc-switches @gol
 -gstabs  -gstabs+  -gstrict-dwarf  -gno-strict-dwarf @gol
 -gas-loc-support  -gno-as-loc-support @gol
 -gas-locview-support  -gno-as-locview-support @gol
@@ -8649,7 +8650,10 @@ Level 0 produces no debug information at all.  Thus, @option{-g0} negates
 Level 1 produces minimal information, enough for making backtraces in
 parts of the program that you don't plan to debug.  This includes
 descriptions of functions and external variables, and line number
-tables, but no information about local variables.
+tables, but no information about local variables.  For historical
+reasons, the descriptions of external variables are not generated
+when producing the debugging information in DWARF format; these
+descriptions can be requested using @option{-gexternal-variables}.
 
 Level 3 includes extra information, such as all the macro definitions
 present in the program.  Some debuggers support macro expansion when
@@ -8663,6 +8667,11 @@ confusion with @option{-gdwarf-@var{level}}.
 Instead use an additional @option{-g@var{level}} option to change the
 debug level for DWARF.
 
+@item -gexternal-variables
+@opindex gexternal-variables
+When using level 1 of the debugging information in DWARF format,
+also produce the descriptions of the external variables.
+
 @item -fno-eliminate-unused-debug-symbols
 @opindex feliminate-unused-debug-symbols
 @opindex fno-eliminate-unused-debug-symbols
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fe46c7e1eee..13c62ad1eec 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -26354,8 +26354,11 @@ gen_decl_die (tree decl, tree origin, struct vlr_context *ctx,
     case VAR_DECL:
     case RESULT_DECL:
       /* If we are in terse mode, don't generate any DIEs to represent any
-	 variable declarations or definitions.  */
-      if (debug_info_level <= DINFO_LEVEL_TERSE)
+	 variable declarations or definitions unless it is an external variable
+	 and we've been explicitly told to generate them.  */
+      if (debug_info_level < DINFO_LEVEL_TERSE
+	  || (debug_info_level == DINFO_LEVEL_TERSE
+	      && !(debug_external_variables && TREE_PUBLIC(decl_or_origin))))
 	break;
 
       /* Avoid generating stray type DIEs during late dwarf dumping.
@@ -26831,8 +26834,11 @@ dwarf2out_decl (tree decl)
 	context_die = lookup_decl_die (DECL_CONTEXT (decl));
 
       /* If we are in terse mode, don't generate any DIEs to represent any
-	 variable declarations or definitions.  */
-      if (debug_info_level <= DINFO_LEVEL_TERSE)
+	 variable declarations or definitions unless it is an external variable
+	 and we've been explicitly told to generate them.  */
+      if (debug_info_level < DINFO_LEVEL_TERSE
+	  || (debug_info_level == DINFO_LEVEL_TERSE
+	      && !(debug_external_variables && TREE_PUBLIC(decl))))
 	return;
       break;
 
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-1.c
new file mode 100644
index 00000000000..d9b75cb7537
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-1.c
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options "-O -gdwarf-2 -g1 -dA" }
+int foo;
+static int bar;
+
+// Verify that with -g1 we do not generate the DIEs for variables at all
+// { dg-final { scan-assembler-not " DW_TAG_variable" } }
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-2.c
new file mode 100644
index 00000000000..177256af5c2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-2.c
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options "-O -gdwarf-2 -g1 -gexternal-variables -dA" }
+static int bar;
+
+// Verify that with -g1 -gexternal-variables we still do not generate
+// DIEs for static variables.
+// { dg-final { scan-assembler-not " DW_TAG_variable" } }
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-3.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-3.c
new file mode 100644
index 00000000000..8d750498a35
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr93751-3.c
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options "-O -gdwarf-2 -g1 -gexternal-variables -dA" }
+int foo;
+
+// Verify that with -g1 -gexternal-variables we generate
+// DIEs for external variables.
+// { dg-final { scan-assembler " DW_TAG_variable" } }

On 2/26/20 2:02 PM, Alexey Neyman wrote:
> Patch ping.
> 
> On 2/19/20 3:30 PM, Alexey Neyman wrote:
>> Hi all,
>>
>> Attached is a patch adjusted per discussion in 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93751
>>
>> - The manual is corrected to reflect that DIEs for external variables 
>> are not generated when using DWARF
>> - A new option is introduced that implements the behavior that was 
>> described in the manual (and is what other debugging formats do).

Don't we want to fix the DWARF behavior to match the documentation?

Jason

On Wed, 26 Feb 2020, Jason Merrill wrote:

> Don't we want to fix the DWARF behavior to match the documentation?

+1 - I think Alexey correctly pointed out in the Bugzilla that debuginfo
growth from this change would be minimal (usually the number of global
variables is very small compared to number of functions (and linemap info).

Alexander

On February 26, 2020 8:26:06 PM GMT+01:00, Alexander Monakov <amonakov@ispras.ru> wrote:
>On Wed, 26 Feb 2020, Jason Merrill wrote:
>
>> Don't we want to fix the DWARF behavior to match the documentation?
>
>+1 - I think Alexey correctly pointed out in the Bugzilla that
>debuginfo
>growth from this change would be minimal (usually the number of global
>variables is very small compared to number of functions (and linemap
>info).

+1 from me as well

Richard. 

>Alexander

> +1 - I think Alexey correctly pointed out in the Bugzilla that debuginfo
> growth from this change would be minimal (usually the number of global
> variables is very small compared to number of functions (and linemap info).

Well, this will drag the associated types too, so figures would be welcome...

On Fri, 28 Feb 2020, Eric Botcazou wrote:

> > +1 - I think Alexey correctly pointed out in the Bugzilla that debuginfo
> > growth from this change would be minimal (usually the number of global
> > variables is very small compared to number of functions (and linemap info).
> 
> Well, this will drag the associated types too, so figures would be welcome...

Hm. So apparently at -g1 we don't emit type information for functions, and
gdb sees each function as 'void foo()' regardless of actual prototype.
Otherwise I would expect most of the types to be already present in debug info.

I wonder if it would make sense to emit defined variables in a similar fashion,
i.e. only address and size?

(assuming that still fits Alexey's needs)

Thanks.
Alexander

On 2/28/20 2:11 PM, Alexander Monakov wrote:
> 
> 
> On Fri, 28 Feb 2020, Eric Botcazou wrote:
> 
>>> +1 - I think Alexey correctly pointed out in the Bugzilla that debuginfo
>>> growth from this change would be minimal (usually the number of global
>>> variables is very small compared to number of functions (and linemap info).
>>
>> Well, this will drag the associated types too, so figures would be welcome...
> 
> Hm. So apparently at -g1 we don't emit type information for functions, and
> gdb sees each function as 'void foo()' regardless of actual prototype.
> Otherwise I would expect most of the types to be already present in debug info.
> 
> I wonder if it would make sense to emit defined variables in a similar fashion,
> i.e. only address and size?

I think so.  And we probably don't want function-scope or class-scope 
variables that happen to be TREE_PUBLIC.

Jason

On 2/28/20 12:28 PM, Jason Merrill wrote:
> On 2/28/20 2:11 PM, Alexander Monakov wrote:
>>
>>
>> On Fri, 28 Feb 2020, Eric Botcazou wrote:
>>
>>>> +1 - I think Alexey correctly pointed out in the Bugzilla that 
>>>> debuginfo
>>>> growth from this change would be minimal (usually the number of global
>>>> variables is very small compared to number of functions (and 
>>>> linemap info).
>>>
>>> Well, this will drag the associated types too, so figures would be 
>>> welcome...
>>
Below are the numbers from compiling GLIBC 2.31 for 
x86_64-unknown-linux-gnu. The first column is with a pristine copy of 
GCC at -g1, the 2nd/3rd columns are -g1 with this patch, 4th/5th columns 
are -g2.

.debug_ranges�� 125872� 125872� 0%����� 452864����� 260%
.debug_str����� 88689�� 102005� 15%���� 183650����� 107%
.debug_abbrev�� 112470� 255769� 127%��� 735914����� 554%
.debug_line���� 647942� 705070� 9%����� 1544877���� 138%
.debug_loc����� 64150�� 64150�� 0%����� 2976025���� 4539%
.debug_aranges� 77712�� 80784�� 4%����� 80912������ 4%
.debug_info���� 240284� 907461� 278%��� 4707104���� 1859%
TOTAL���������� 1357119 2241111 65%���� 10681346��� 687%

So indeed, an increase due to type info is substantial - but still 
nothing compared to -g2 increase.
>> Hm. So apparently at -g1 we don't emit type information for 
>> functions, and
>> gdb sees each function as 'void foo()' regardless of actual prototype.
>> Otherwise I would expect most of the types to be already present in 
>> debug info.
>>
>> I wonder if it would make sense to emit defined variables in a 
>> similar fashion,
>> i.e. only address and size?
I'll see if I can modify the patch to do so.
>
> I think so.� And we probably don't want function-scope or class-scope 
> variables that happen to be TREE_PUBLIC.

Can you give an example of such function/class scope variables being 
TREE_PUBLIC? I tried a static variable described within a function 
scope, it was not TREE_PUBLIC.

Regards,
Alexey.

On 2/28/20 4:12 PM, Alexey Neyman wrote:
> 
> On 2/28/20 12:28 PM, Jason Merrill wrote:
>> On 2/28/20 2:11 PM, Alexander Monakov wrote:
>>>
>>>
>>> On Fri, 28 Feb 2020, Eric Botcazou wrote:
>>>
>>>>> +1 - I think Alexey correctly pointed out in the Bugzilla that 
>>>>> debuginfo
>>>>> growth from this change would be minimal (usually the number of global
>>>>> variables is very small compared to number of functions (and 
>>>>> linemap info).
>>>>
>>>> Well, this will drag the associated types too, so figures would be 
>>>> welcome...
>>>
> Below are the numbers from compiling GLIBC 2.31 for 
> x86_64-unknown-linux-gnu. The first column is with a pristine copy of 
> GCC at -g1, the 2nd/3rd columns are -g1 with this patch, 4th/5th columns 
> are -g2.
> 
> .debug_ranges�� 125872� 125872� 0%����� 452864����� 260%
> .debug_str����� 88689�� 102005� 15%���� 183650����� 107%
> .debug_abbrev�� 112470� 255769� 127%��� 735914����� 554%
> .debug_line���� 647942� 705070� 9%����� 1544877���� 138%
> .debug_loc����� 64150�� 64150�� 0%����� 2976025���� 4539%
> .debug_aranges� 77712�� 80784�� 4%����� 80912������ 4%
> .debug_info���� 240284� 907461� 278%��� 4707104���� 1859%
> TOTAL���������� 1357119 2241111 65%���� 10681346��� 687%
> 
> So indeed, an increase due to type info is substantial - but still 
> nothing compared to -g2 increase.
>>> Hm. So apparently at -g1 we don't emit type information for 
>>> functions, and
>>> gdb sees each function as 'void foo()' regardless of actual prototype.
>>> Otherwise I would expect most of the types to be already present in 
>>> debug info.
>>>
>>> I wonder if it would make sense to emit defined variables in a 
>>> similar fashion,
>>> i.e. only address and size?
> I'll see if I can modify the patch to do so.
>>
>> I think so.� And we probably don't want function-scope or class-scope 
>> variables that happen to be TREE_PUBLIC.
> 
> Can you give an example of such function/class scope variables being 
> TREE_PUBLIC? I tried a static variable described within a function 
> scope, it was not TREE_PUBLIC.

A class static data member, or a static variable in an inline function 
or template instantiation.

Jason