public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC][debug] Add -greadable-dwarf
@ 2018-08-15 14:02 Tom de Vries
  2018-08-20 12:59 ` Richard Biener
  0 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2018-08-15 14:02 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Biener, Jakub Jelinek, Jason Merrill, Cary Coutant

Hi,

This patch adds option -greadable-dwarf.  In absence of an DW_AT_comment
attribute, it sets the DW_AT_name attribute of dies that otherwise do not get
that attribute, to make it easier to figure out what the die is describing.

The option exports the names of artificial variables:
...
 DIE    0: DW_TAG_variable (0x7fa934dd54b0)
+  DW_AT_name: "D.1922"
   DW_AT_type: die -> 0 (0x7fa934dd0d70)
   DW_AT_artificial: 1

...
which can be traced back to gimple dumps:
...
  char a[0:D.1922] [value-expr: *a.0];
...

Furthermore, it adds names to external references:
...
 DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
+DW_AT_name: "main"
 DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
...

This is an undocumented developer-only option, because using this option may
change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
...
(gdb) info locals
a = 0x7fffffffda90 "\005"
D.4278 = <optimized out>
...

Any comments?

Thanks,
- Tom

[debug] Add -greadable-dwarf

2018-08-15  Tom de Vries  <tdevries@suse.de>

	* common.opt (greadable-dwarf): Add option.
	* dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add name
	for artifical decls.
	(add_decl_name): New function.
	(dwarf2out_register_external_die): Add name to external reference die.

---
 gcc/common.opt  |  5 +++++
 gcc/dwarf2out.c | 24 +++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index b2f2215ecc6..6e5e0558e49 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2972,6 +2972,11 @@ gstrict-dwarf
 Common Driver Report Var(dwarf_strict) Init(0)
 Don't emit DWARF additions beyond selected version.
 
+greadable-dwarf
+Common Driver Undocumented Report Var(flag_readable_dwarf) Init(0)
+Make generated dwarf more readable, at the cost of space and exposing compiler
+internals.
+
 gtoggle
 Common Driver Report Var(flag_gtoggle)
 Toggle debug information generation.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 4b63cbd8a1e..8c6b4372874 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3824,7 +3824,9 @@ static void add_prototyped_attribute (dw_die_ref, tree);
 static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
 static void add_pure_or_virtual_attribute (dw_die_ref, tree);
 static void add_src_coords_attributes (dw_die_ref, tree);
-static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false);
+static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false,
+						bool = false);
+static void add_decl_name (dw_die_ref, tree);
 static void add_discr_value (dw_die_ref, dw_discr_value *);
 static void add_discr_list (dw_die_ref, dw_discr_list_ref);
 static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
@@ -6022,6 +6024,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
   else
     equate_decl_number_to_die (decl, die);
 
+  if (flag_readable_dwarf)
+    add_decl_name (die, decl);
   /* Add a reference to the DIE providing early debug at $sym + off.  */
   add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
 }
@@ -21269,7 +21273,8 @@ add_linkage_name (dw_die_ref die, tree decl)
 
 static void
 add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
-				    bool no_linkage_name)
+				    bool no_linkage_name,
+				    bool no_src_coords_attributes)
 {
   tree decl_name;
 
@@ -21279,12 +21284,19 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
       const char *name = dwarf2_name (decl, 0);
       if (name)
 	add_name_attribute (die, name);
-      if (! DECL_ARTIFICIAL (decl))
+      if (!no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
 	add_src_coords_attributes (die, decl);
 
       if (!no_linkage_name)
 	add_linkage_name (die, decl);
     }
+  else if (flag_readable_dwarf && decl_name == NULL)
+    {
+      char *buf = XNEWVEC (char, 32);
+      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
+      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
+      add_name_attribute (die, buf);
+    }
 
 #ifdef VMS_DEBUGGING_INFO
   /* Get the function's name, as described by its RTL.  This may be different
@@ -21298,6 +21310,12 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
 #endif /* VMS_DEBUGGING_INFO */
 }
 
+static void
+add_decl_name (dw_die_ref die, tree decl)
+{
+  add_name_and_src_coords_attributes (die, decl, true, true);
+}
+
 /* Add VALUE as a DW_AT_discr_value attribute to DIE.  */
 
 static void

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC][debug] Add -greadable-dwarf
  2018-08-15 14:02 [RFC][debug] Add -greadable-dwarf Tom de Vries
@ 2018-08-20 12:59 ` Richard Biener
  2018-08-21  7:01   ` Jason Merrill
  2018-08-21 12:22   ` Tom de Vries
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Biener @ 2018-08-20 12:59 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

On Wed, 15 Aug 2018, Tom de Vries wrote:

> Hi,
> 
> This patch adds option -greadable-dwarf.  In absence of an DW_AT_comment
> attribute,

What's a DW_AT_comment attribute?  I don't see this mentioned in the
patch.

> it sets the DW_AT_name attribute of dies that otherwise do not get
> that attribute, to make it easier to figure out what the die is describing.
> 
> The option exports the names of artificial variables:
> ...
>  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
> +  DW_AT_name: "D.1922"
>    DW_AT_type: die -> 0 (0x7fa934dd0d70)
>    DW_AT_artificial: 1
> 
> ...
> which can be traced back to gimple dumps:
> ...
>   char a[0:D.1922] [value-expr: *a.0];
> ...
> 
> Furthermore, it adds names to external references:
> ...
>  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
> +DW_AT_name: "main"
>  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
> ...
> 
> This is an undocumented developer-only option, because using this option may
> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
> ...
> (gdb) info locals
> a = 0x7fffffffda90 "\005"
> D.4278 = <optimized out>
> ...
> 
> Any comments?

The idea is OK I guess but I'd call it -gforce-named-dies instead
of -greadable-dwarf.  It also goes only half-way since it doesn't
add names to DECL_NAMELESS vars.

There doesn't seem to be a convenient place to 

> Thanks,
> - Tom
> 
> [debug] Add -greadable-dwarf
> 
> 2018-08-15  Tom de Vries  <tdevries@suse.de>
> 
> 	* common.opt (greadable-dwarf): Add option.
> 	* dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add name
> 	for artifical decls.
> 	(add_decl_name): New function.
> 	(dwarf2out_register_external_die): Add name to external reference die.
> 
> ---
>  gcc/common.opt  |  5 +++++
>  gcc/dwarf2out.c | 24 +++++++++++++++++++++---
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index b2f2215ecc6..6e5e0558e49 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2972,6 +2972,11 @@ gstrict-dwarf
>  Common Driver Report Var(dwarf_strict) Init(0)
>  Don't emit DWARF additions beyond selected version.
>  
> +greadable-dwarf
> +Common Driver Undocumented Report Var(flag_readable_dwarf) Init(0)
> +Make generated dwarf more readable, at the cost of space and exposing compiler
> +internals.
> +
>  gtoggle
>  Common Driver Report Var(flag_gtoggle)
>  Toggle debug information generation.
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 4b63cbd8a1e..8c6b4372874 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -3824,7 +3824,9 @@ static void add_prototyped_attribute (dw_die_ref, tree);
>  static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
>  static void add_pure_or_virtual_attribute (dw_die_ref, tree);
>  static void add_src_coords_attributes (dw_die_ref, tree);
> -static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false);
> +static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false,
> +						bool = false);
> +static void add_decl_name (dw_die_ref, tree);
>  static void add_discr_value (dw_die_ref, dw_discr_value *);
>  static void add_discr_list (dw_die_ref, dw_discr_list_ref);
>  static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
> @@ -6022,6 +6024,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
>    else
>      equate_decl_number_to_die (decl, die);
>  
> +  if (flag_readable_dwarf)
> +    add_decl_name (die, decl);

Please use add_name_and_src_coords_attributes directly.

>    /* Add a reference to the DIE providing early debug at $sym + off.  */
>    add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
>  }
> @@ -21269,7 +21273,8 @@ add_linkage_name (dw_die_ref die, tree decl)
>  
>  static void
>  add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
> -				    bool no_linkage_name)
> +				    bool no_linkage_name,
> +				    bool no_src_coords_attributes)
>  {
>    tree decl_name;
>  
> @@ -21279,12 +21284,19 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
>        const char *name = dwarf2_name (decl, 0);
>        if (name)
>  	add_name_attribute (die, name);
> -      if (! DECL_ARTIFICIAL (decl))
> +      if (!no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))

inconsistent spacing after !

>  	add_src_coords_attributes (die, decl);
>  
>        if (!no_linkage_name)
>  	add_linkage_name (die, decl);
>      }
> +  else if (flag_readable_dwarf && decl_name == NULL)
> +    {
> +      char *buf = XNEWVEC (char, 32);
> +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
> +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
> +      add_name_attribute (die, buf);

I think you leak 'buf'.

> +    }
>  
>  #ifdef VMS_DEBUGGING_INFO

how does it interact with this VMS_DEBUGGING_INFO path?

>    /* Get the function's name, as described by its RTL.  This may be different
> @@ -21298,6 +21310,12 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
>  #endif /* VMS_DEBUGGING_INFO */
>  }
>  
> +static void
> +add_decl_name (dw_die_ref die, tree decl)
> +{
> +  add_name_and_src_coords_attributes (die, decl, true, true);
> +}
> +
>  /* Add VALUE as a DW_AT_discr_value attribute to DIE.  */
>  
>  static void
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC][debug] Add -greadable-dwarf
  2018-08-20 12:59 ` Richard Biener
@ 2018-08-21  7:01   ` Jason Merrill
  2018-08-21  7:16     ` Richard Biener
  2018-08-21 12:40     ` Tom de Vries
  2018-08-21 12:22   ` Tom de Vries
  1 sibling, 2 replies; 15+ messages in thread
From: Jason Merrill @ 2018-08-21  7:01 UTC (permalink / raw)
  To: Richard Biener
  Cc: Tom de Vries, gcc-patches List, Jakub Jelinek, Cary Coutant

How about adding this name to a -dA comment instead of the actual dwarf?

On Tue, Aug 21, 2018, 12:59 AM Richard Biener <rguenther@suse.de> wrote:

> On Wed, 15 Aug 2018, Tom de Vries wrote:
>
> > Hi,
> >
> > This patch adds option -greadable-dwarf.  In absence of an DW_AT_comment
> > attribute,
>
> What's a DW_AT_comment attribute?  I don't see this mentioned in the
> patch.
>
> > it sets the DW_AT_name attribute of dies that otherwise do not get
> > that attribute, to make it easier to figure out what the die is
> describing.
> >
> > The option exports the names of artificial variables:
> > ...
> >  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
> > +  DW_AT_name: "D.1922"
> >    DW_AT_type: die -> 0 (0x7fa934dd0d70)
> >    DW_AT_artificial: 1
> >
> > ...
> > which can be traced back to gimple dumps:
> > ...
> >   char a[0:D.1922] [value-expr: *a.0];
> > ...
> >
> > Furthermore, it adds names to external references:
> > ...
> >  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
> > +DW_AT_name: "main"
> >  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29
> (0x7fa88b965140)
> > ...
> >
> > This is an undocumented developer-only option, because using this option
> may
> > change behaviour of dwarf consumers, f.i., gdb shows the artificial
> variables:
> > ...
> > (gdb) info locals
> > a = 0x7fffffffda90 "\005"
> > D.4278 = <optimized out>
> > ...
> >
> > Any comments?
>
> The idea is OK I guess but I'd call it -gforce-named-dies instead
> of -greadable-dwarf.  It also goes only half-way since it doesn't
> add names to DECL_NAMELESS vars.
>
> There doesn't seem to be a convenient place to
>
> > Thanks,
> > - Tom
> >
> > [debug] Add -greadable-dwarf
> >
> > 2018-08-15  Tom de Vries  <tdevries@suse.de>
> >
> >       * common.opt (greadable-dwarf): Add option.
> >       * dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add
> name
> >       for artifical decls.
> >       (add_decl_name): New function.
> >       (dwarf2out_register_external_die): Add name to external reference
> die.
> >
> > ---
> >  gcc/common.opt  |  5 +++++
> >  gcc/dwarf2out.c | 24 +++++++++++++++++++++---
> >  2 files changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index b2f2215ecc6..6e5e0558e49 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -2972,6 +2972,11 @@ gstrict-dwarf
> >  Common Driver Report Var(dwarf_strict) Init(0)
> >  Don't emit DWARF additions beyond selected version.
> >
> > +greadable-dwarf
> > +Common Driver Undocumented Report Var(flag_readable_dwarf) Init(0)
> > +Make generated dwarf more readable, at the cost of space and exposing
> compiler
> > +internals.
> > +
> >  gtoggle
> >  Common Driver Report Var(flag_gtoggle)
> >  Toggle debug information generation.
> > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> > index 4b63cbd8a1e..8c6b4372874 100644
> > --- a/gcc/dwarf2out.c
> > +++ b/gcc/dwarf2out.c
> > @@ -3824,7 +3824,9 @@ static void add_prototyped_attribute (dw_die_ref,
> tree);
> >  static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
> >  static void add_pure_or_virtual_attribute (dw_die_ref, tree);
> >  static void add_src_coords_attributes (dw_die_ref, tree);
> > -static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool
> = false);
> > +static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool
> = false,
> > +                                             bool = false);
> > +static void add_decl_name (dw_die_ref, tree);
> >  static void add_discr_value (dw_die_ref, dw_discr_value *);
> >  static void add_discr_list (dw_die_ref, dw_discr_list_ref);
> >  static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
> > @@ -6022,6 +6024,8 @@ dwarf2out_register_external_die (tree decl, const
> char *sym,
> >    else
> >      equate_decl_number_to_die (decl, die);
> >
> > +  if (flag_readable_dwarf)
> > +    add_decl_name (die, decl);
>
> Please use add_name_and_src_coords_attributes directly.
>
> >    /* Add a reference to the DIE providing early debug at $sym + off.  */
> >    add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
> >  }
> > @@ -21269,7 +21273,8 @@ add_linkage_name (dw_die_ref die, tree decl)
> >
> >  static void
> >  add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
> > -                                 bool no_linkage_name)
> > +                                 bool no_linkage_name,
> > +                                 bool no_src_coords_attributes)
> >  {
> >    tree decl_name;
> >
> > @@ -21279,12 +21284,19 @@ add_name_and_src_coords_attributes (dw_die_ref
> die, tree decl,
> >        const char *name = dwarf2_name (decl, 0);
> >        if (name)
> >       add_name_attribute (die, name);
> > -      if (! DECL_ARTIFICIAL (decl))
> > +      if (!no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
>
> inconsistent spacing after !
>
> >       add_src_coords_attributes (die, decl);
> >
> >        if (!no_linkage_name)
> >       add_linkage_name (die, decl);
> >      }
> > +  else if (flag_readable_dwarf && decl_name == NULL)
> > +    {
> > +      char *buf = XNEWVEC (char, 32);
> > +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
> > +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
> > +      add_name_attribute (die, buf);
>
> I think you leak 'buf'.
>
> > +    }
> >
> >  #ifdef VMS_DEBUGGING_INFO
>
> how does it interact with this VMS_DEBUGGING_INFO path?
>
> >    /* Get the function's name, as described by its RTL.  This may be
> different
> > @@ -21298,6 +21310,12 @@ add_name_and_src_coords_attributes (dw_die_ref
> die, tree decl,
> >  #endif /* VMS_DEBUGGING_INFO */
> >  }
> >
> > +static void
> > +add_decl_name (dw_die_ref die, tree decl)
> > +{
> > +  add_name_and_src_coords_attributes (die, decl, true, true);
> > +}
> > +
> >  /* Add VALUE as a DW_AT_discr_value attribute to DIE.  */
> >
> >  static void
> >
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> 21284 (AG Nuernberg)
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC][debug] Add -greadable-dwarf
  2018-08-21  7:01   ` Jason Merrill
@ 2018-08-21  7:16     ` Richard Biener
  2018-08-21 12:40     ` Tom de Vries
  1 sibling, 0 replies; 15+ messages in thread
From: Richard Biener @ 2018-08-21  7:16 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Tom de Vries, gcc-patches List, Jakub Jelinek, Cary Coutant

On Tue, 21 Aug 2018, Jason Merrill wrote:

> How about adding this name to a -dA comment instead of the actual dwarf?

That would also work but it requires either sticking that name somewhere
in die_struct or keep a reference to the decl we created a DIE for
in the same struct.  Though we already have die_struct->decl_id which
might be enough info we could even dump unconditionally besides
or in place of the DW_AT_name attribute.

Richard.

> On Tue, Aug 21, 2018, 12:59 AM Richard Biener <rguenther@suse.de> wrote:
> 
> > On Wed, 15 Aug 2018, Tom de Vries wrote:
> >
> > > Hi,
> > >
> > > This patch adds option -greadable-dwarf.  In absence of an DW_AT_comment
> > > attribute,
> >
> > What's a DW_AT_comment attribute?  I don't see this mentioned in the
> > patch.
> >
> > > it sets the DW_AT_name attribute of dies that otherwise do not get
> > > that attribute, to make it easier to figure out what the die is
> > describing.
> > >
> > > The option exports the names of artificial variables:
> > > ...
> > >  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
> > > +  DW_AT_name: "D.1922"
> > >    DW_AT_type: die -> 0 (0x7fa934dd0d70)
> > >    DW_AT_artificial: 1
> > >
> > > ...
> > > which can be traced back to gimple dumps:
> > > ...
> > >   char a[0:D.1922] [value-expr: *a.0];
> > > ...
> > >
> > > Furthermore, it adds names to external references:
> > > ...
> > >  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
> > > +DW_AT_name: "main"
> > >  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29
> > (0x7fa88b965140)
> > > ...
> > >
> > > This is an undocumented developer-only option, because using this option
> > may
> > > change behaviour of dwarf consumers, f.i., gdb shows the artificial
> > variables:
> > > ...
> > > (gdb) info locals
> > > a = 0x7fffffffda90 "\005"
> > > D.4278 = <optimized out>
> > > ...
> > >
> > > Any comments?
> >
> > The idea is OK I guess but I'd call it -gforce-named-dies instead
> > of -greadable-dwarf.  It also goes only half-way since it doesn't
> > add names to DECL_NAMELESS vars.
> >
> > There doesn't seem to be a convenient place to
> >
> > > Thanks,
> > > - Tom
> > >
> > > [debug] Add -greadable-dwarf
> > >
> > > 2018-08-15  Tom de Vries  <tdevries@suse.de>
> > >
> > >       * common.opt (greadable-dwarf): Add option.
> > >       * dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add
> > name
> > >       for artifical decls.
> > >       (add_decl_name): New function.
> > >       (dwarf2out_register_external_die): Add name to external reference
> > die.
> > >
> > > ---
> > >  gcc/common.opt  |  5 +++++
> > >  gcc/dwarf2out.c | 24 +++++++++++++++++++++---
> > >  2 files changed, 26 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index b2f2215ecc6..6e5e0558e49 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -2972,6 +2972,11 @@ gstrict-dwarf
> > >  Common Driver Report Var(dwarf_strict) Init(0)
> > >  Don't emit DWARF additions beyond selected version.
> > >
> > > +greadable-dwarf
> > > +Common Driver Undocumented Report Var(flag_readable_dwarf) Init(0)
> > > +Make generated dwarf more readable, at the cost of space and exposing
> > compiler
> > > +internals.
> > > +
> > >  gtoggle
> > >  Common Driver Report Var(flag_gtoggle)
> > >  Toggle debug information generation.
> > > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> > > index 4b63cbd8a1e..8c6b4372874 100644
> > > --- a/gcc/dwarf2out.c
> > > +++ b/gcc/dwarf2out.c
> > > @@ -3824,7 +3824,9 @@ static void add_prototyped_attribute (dw_die_ref,
> > tree);
> > >  static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
> > >  static void add_pure_or_virtual_attribute (dw_die_ref, tree);
> > >  static void add_src_coords_attributes (dw_die_ref, tree);
> > > -static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool
> > = false);
> > > +static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool
> > = false,
> > > +                                             bool = false);
> > > +static void add_decl_name (dw_die_ref, tree);
> > >  static void add_discr_value (dw_die_ref, dw_discr_value *);
> > >  static void add_discr_list (dw_die_ref, dw_discr_list_ref);
> > >  static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
> > > @@ -6022,6 +6024,8 @@ dwarf2out_register_external_die (tree decl, const
> > char *sym,
> > >    else
> > >      equate_decl_number_to_die (decl, die);
> > >
> > > +  if (flag_readable_dwarf)
> > > +    add_decl_name (die, decl);
> >
> > Please use add_name_and_src_coords_attributes directly.
> >
> > >    /* Add a reference to the DIE providing early debug at $sym + off.  */
> > >    add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
> > >  }
> > > @@ -21269,7 +21273,8 @@ add_linkage_name (dw_die_ref die, tree decl)
> > >
> > >  static void
> > >  add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
> > > -                                 bool no_linkage_name)
> > > +                                 bool no_linkage_name,
> > > +                                 bool no_src_coords_attributes)
> > >  {
> > >    tree decl_name;
> > >
> > > @@ -21279,12 +21284,19 @@ add_name_and_src_coords_attributes (dw_die_ref
> > die, tree decl,
> > >        const char *name = dwarf2_name (decl, 0);
> > >        if (name)
> > >       add_name_attribute (die, name);
> > > -      if (! DECL_ARTIFICIAL (decl))
> > > +      if (!no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
> >
> > inconsistent spacing after !
> >
> > >       add_src_coords_attributes (die, decl);
> > >
> > >        if (!no_linkage_name)
> > >       add_linkage_name (die, decl);
> > >      }
> > > +  else if (flag_readable_dwarf && decl_name == NULL)
> > > +    {
> > > +      char *buf = XNEWVEC (char, 32);
> > > +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
> > > +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
> > > +      add_name_attribute (die, buf);
> >
> > I think you leak 'buf'.
> >
> > > +    }
> > >
> > >  #ifdef VMS_DEBUGGING_INFO
> >
> > how does it interact with this VMS_DEBUGGING_INFO path?
> >
> > >    /* Get the function's name, as described by its RTL.  This may be
> > different
> > > @@ -21298,6 +21310,12 @@ add_name_and_src_coords_attributes (dw_die_ref
> > die, tree decl,
> > >  #endif /* VMS_DEBUGGING_INFO */
> > >  }
> > >
> > > +static void
> > > +add_decl_name (dw_die_ref die, tree decl)
> > > +{
> > > +  add_name_and_src_coords_attributes (die, decl, true, true);
> > > +}
> > > +
> > >  /* Add VALUE as a DW_AT_discr_value attribute to DIE.  */
> > >
> > >  static void
> > >
> > >
> >
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> > 21284 (AG Nuernberg)
> >
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC][debug] Add -greadable-dwarf
  2018-08-20 12:59 ` Richard Biener
  2018-08-21  7:01   ` Jason Merrill
@ 2018-08-21 12:22   ` Tom de Vries
  2018-08-22  6:56     ` [PATCH][debug] Add -gforce-named-dies Tom de Vries
  1 sibling, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2018-08-21 12:22 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

[-- Attachment #1: Type: text/plain, Size: 5868 bytes --]

On 08/20/2018 02:59 PM, Richard Biener wrote:
> On Wed, 15 Aug 2018, Tom de Vries wrote:
> 
>> Hi,
>>
>> This patch adds option -greadable-dwarf.  In absence of an DW_AT_comment
>> attribute,
> 
> What's a DW_AT_comment attribute?  I don't see this mentioned in the
> patch.
> 

It's a hypothetical DWARF attribute, that I would have preferred to use
instead of DW_AT_name.

>> it sets the DW_AT_name attribute of dies that otherwise do not get
>> that attribute, to make it easier to figure out what the die is describing.
>>
>> The option exports the names of artificial variables:
>> ...
>>  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
>> +  DW_AT_name: "D.1922"
>>    DW_AT_type: die -> 0 (0x7fa934dd0d70)
>>    DW_AT_artificial: 1
>>
>> ...
>> which can be traced back to gimple dumps:
>> ...
>>   char a[0:D.1922] [value-expr: *a.0];
>> ...
>>
>> Furthermore, it adds names to external references:
>> ...
>>  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
>> +DW_AT_name: "main"
>>  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
>> ...
>>
>> This is an undocumented developer-only option, because using this option may
>> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
>> ...
>> (gdb) info locals
>> a = 0x7fffffffda90 "\005"
>> D.4278 = <optimized out>
>> ...
>>
>> Any comments?
> 
> The idea is OK I guess but I'd call it -gforce-named-dies instead
> of -greadable-dwarf.

Done.

>  It also goes only half-way since it doesn't
> add names to DECL_NAMELESS vars.

I've tried to add that.

> There doesn't seem to be a convenient place to 
> 

?

>> Thanks,
>> - Tom
>>
>> [debug] Add -greadable-dwarf
>>
>> 2018-08-15  Tom de Vries  <tdevries@suse.de>
>>
>> 	* common.opt (greadable-dwarf): Add option.
>> 	* dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add name
>> 	for artifical decls.
>> 	(add_decl_name): New function.
>> 	(dwarf2out_register_external_die): Add name to external reference die.
>>
>> ---
>>  gcc/common.opt  |  5 +++++
>>  gcc/dwarf2out.c | 24 +++++++++++++++++++++---
>>  2 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index b2f2215ecc6..6e5e0558e49 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -2972,6 +2972,11 @@ gstrict-dwarf
>>  Common Driver Report Var(dwarf_strict) Init(0)
>>  Don't emit DWARF additions beyond selected version.
>>  
>> +greadable-dwarf
>> +Common Driver Undocumented Report Var(flag_readable_dwarf) Init(0)
>> +Make generated dwarf more readable, at the cost of space and exposing compiler
>> +internals.
>> +
>>  gtoggle
>>  Common Driver Report Var(flag_gtoggle)
>>  Toggle debug information generation.
>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>> index 4b63cbd8a1e..8c6b4372874 100644
>> --- a/gcc/dwarf2out.c
>> +++ b/gcc/dwarf2out.c
>> @@ -3824,7 +3824,9 @@ static void add_prototyped_attribute (dw_die_ref, tree);
>>  static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
>>  static void add_pure_or_virtual_attribute (dw_die_ref, tree);
>>  static void add_src_coords_attributes (dw_die_ref, tree);
>> -static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false);
>> +static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false,
>> +						bool = false);
>> +static void add_decl_name (dw_die_ref, tree);
>>  static void add_discr_value (dw_die_ref, dw_discr_value *);
>>  static void add_discr_list (dw_die_ref, dw_discr_list_ref);
>>  static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
>> @@ -6022,6 +6024,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
>>    else
>>      equate_decl_number_to_die (decl, die);
>>  
>> +  if (flag_readable_dwarf)
>> +    add_decl_name (die, decl);
> 
> Please use add_name_and_src_coords_attributes directly.
> 

Done.

>>    /* Add a reference to the DIE providing early debug at $sym + off.  */
>>    add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
>>  }
>> @@ -21269,7 +21273,8 @@ add_linkage_name (dw_die_ref die, tree decl)
>>  
>>  static void
>>  add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
>> -				    bool no_linkage_name)
>> +				    bool no_linkage_name,
>> +				    bool no_src_coords_attributes)
>>  {
>>    tree decl_name;
>>  
>> @@ -21279,12 +21284,19 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
>>        const char *name = dwarf2_name (decl, 0);
>>        if (name)
>>  	add_name_attribute (die, name);
>> -      if (! DECL_ARTIFICIAL (decl))
>> +      if (!no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
> 
> inconsistent spacing after !
> 

Done.

>>  	add_src_coords_attributes (die, decl);
>>  
>>        if (!no_linkage_name)
>>  	add_linkage_name (die, decl);
>>      }
>> +  else if (flag_readable_dwarf && decl_name == NULL)
>> +    {
>> +      char *buf = XNEWVEC (char, 32);
>> +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
>> +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
>> +      add_name_attribute (die, buf);
> 
> I think you leak 'buf'.
> 

Oops, fixed.

>> +    }
>>  
>>  #ifdef VMS_DEBUGGING_INFO
> 
> how does it interact with this VMS_DEBUGGING_INFO path?
> 

Um, AFAICT, not in any particular way,.

>>    /* Get the function's name, as described by its RTL.  This may be different
>> @@ -21298,6 +21310,12 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
>>  #endif /* VMS_DEBUGGING_INFO */
>>  }
>>  
>> +static void
>> +add_decl_name (dw_die_ref die, tree decl)
>> +{
>> +  add_name_and_src_coords_attributes (die, decl, true, true);
>> +}
>> +
>>  /* Add VALUE as a DW_AT_discr_value attribute to DIE.  */
>>  
>>  static void
>>
>>
> 

Currently doing a bootstrap and reg-test on x86_64 of attached patch
with -gforce-named-dies enabled by default.

Thanks,
- Tom


[-- Attachment #2: 0001-debug-Add-gforce-named-dies.patch --]
[-- Type: text/x-patch, Size: 5413 bytes --]

[debug] Add -gforce-named-dies

This patch adds option -gforce-named-dies.  It sets the DW_AT_name attribute
of dies that otherwise do not get that attribute, to make it easier to figure
out what the die is describing.

The option exports the names of artificial variables:
...
 DIE    0: DW_TAG_variable (0x7fa934dd54b0)
+  DW_AT_name: "D.1922"
   DW_AT_type: die -> 0 (0x7fa934dd0d70)
   DW_AT_artificial: 1

...
which can be traced back to gimple dumps:
...
  char a[0:D.1922] [value-expr: *a.0];
...

Furthermore, it adds names to external references:
...
 DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
+DW_AT_name: "main"
 DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
...

This is an undocumented developer-only option, because using this option may
change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
...
(gdb) info locals
a = 0x7fffffffda90 "\005"
D.4278 = <optimized out>
...

2018-08-15  Tom de Vries  <tdevries@suse.de>

	* common.opt (gforce-named-dies): Add option.
	* dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add name
	for artifical and nameless decls.
	(dwarf2out_register_external_die): Add name to external reference die.
	(gen_subprogram_die): Add name to DW_TAG_call_site_parameter.

---
 gcc/common.opt  |  5 +++++
 gcc/dwarf2out.c | 27 +++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index ebc3ef43ce2..76032f9bb1d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2976,6 +2976,11 @@ gstrict-dwarf
 Common Driver Report Var(dwarf_strict) Init(0)
 Don't emit DWARF additions beyond selected version.
 
+gforce-named-dies
+Common Driver Undocumented Report Var(flag_force_named_dies) Init(0)
+Force DWARF DIEs to have a name attribute.  Undocumented because it exposes
+compiler internals to the DWARF consumer.
+
 gtoggle
 Common Driver Report Var(flag_gtoggle)
 Toggle debug information generation.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fb71ff349fa..dd8f438dfd3 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3824,7 +3824,8 @@ static void add_prototyped_attribute (dw_die_ref, tree);
 static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
 static void add_pure_or_virtual_attribute (dw_die_ref, tree);
 static void add_src_coords_attributes (dw_die_ref, tree);
-static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false);
+static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false,
+						bool = false);
 static void add_discr_value (dw_die_ref, dw_discr_value *);
 static void add_discr_list (dw_die_ref, dw_discr_list_ref);
 static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
@@ -6022,6 +6023,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
   else
     equate_decl_number_to_die (decl, die);
 
+  if (flag_force_named_dies && DECL_P (decl))
+    add_name_and_src_coords_attributes (die, decl, true, true);
   /* Add a reference to the DIE providing early debug at $sym + off.  */
   add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
 }
@@ -21277,22 +21280,33 @@ add_linkage_name (dw_die_ref die, tree decl)
 
 static void
 add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
-				    bool no_linkage_name)
+				    bool no_linkage_name,
+				    bool no_src_coords_attributes)
 {
   tree decl_name;
 
   decl_name = DECL_NAME (decl);
   if (decl_name != NULL && IDENTIFIER_POINTER (decl_name) != NULL)
     {
-      const char *name = dwarf2_name (decl, 0);
+      const char *name = (flag_force_named_dies && DECL_NAMELESS (decl)
+			  ? IDENTIFIER_POINTER (decl_name)
+			  : dwarf2_name (decl, 0));
       if (name)
 	add_name_attribute (die, name);
-      if (! DECL_ARTIFICIAL (decl))
+      if (! no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
 	add_src_coords_attributes (die, decl);
 
       if (!no_linkage_name)
 	add_linkage_name (die, decl);
     }
+  else if (flag_force_named_dies && decl_name == NULL
+	   && (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == CONST_DECL))
+    {
+      char buf[32];
+      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
+      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
+      add_name_attribute (die, buf);
+    }
 
 #ifdef VMS_DEBUGGING_INFO
   /* Get the function's name, as described by its RTL.  This may be different
@@ -23265,6 +23279,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 	      dw_die_ref die = NULL;
 	      rtx tloc = NULL_RTX, tlocc = NULL_RTX;
 	      rtx arg, next_arg;
+	      tree arg_decl = NULL_TREE;
 
 	      for (arg = (ca_loc->call_arg_loc_note != NULL_RTX
 			  ? XEXP (ca_loc->call_arg_loc_note, 0)
@@ -23329,6 +23344,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		      tdie = lookup_decl_die (tdecl);
 		      if (tdie == NULL)
 			continue;
+		      arg_decl = tdecl;
 		    }
 		  else
 		    continue;
@@ -23345,6 +23361,9 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		    die = gen_call_site_die (decl, subr_die, ca_loc);
 		  cdie = new_die (dwarf_TAG (DW_TAG_call_site_parameter), die,
 				  NULL_TREE);
+		  if (flag_force_named_dies && arg_decl)
+		    add_name_and_src_coords_attributes (cdie, arg_decl, true,
+							true);
 		  if (reg != NULL)
 		    add_AT_loc (cdie, DW_AT_location, reg);
 		  else if (tdie != NULL)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC][debug] Add -greadable-dwarf
  2018-08-21  7:01   ` Jason Merrill
  2018-08-21  7:16     ` Richard Biener
@ 2018-08-21 12:40     ` Tom de Vries
  1 sibling, 0 replies; 15+ messages in thread
From: Tom de Vries @ 2018-08-21 12:40 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Richard Biener, gcc-patches List, Jakub Jelinek, Cary Coutant

Hi,

That solution is limited: it does not show the extra information in the
dump from an executable (which is the only way to see how things look
like post-linking).

Is your concern leaking compiler internals into the dwarf info?

Thanks,
- Tom

On 08/21/2018 06:53 AM, Jason Merrill wrote:
> How about adding this name to a -dA comment instead of the actual dwarf?
> 
> On Tue, Aug 21, 2018, 12:59 AM Richard Biener <rguenther@suse.de
> <mailto:rguenther@suse.de>> wrote:
> 
>     On Wed, 15 Aug 2018, Tom de Vries wrote:
> 
>     > Hi,
>     >
>     > This patch adds option -greadable-dwarf.  In absence of an
>     DW_AT_comment
>     > attribute,
> 
>     What's a DW_AT_comment attribute?  I don't see this mentioned in the
>     patch.
> 
>     > it sets the DW_AT_name attribute of dies that otherwise do not get
>     > that attribute, to make it easier to figure out what the die is
>     describing.
>     >
>     > The option exports the names of artificial variables:
>     > ...
>     >  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
>     > +  DW_AT_name: "D.1922"
>     >    DW_AT_type: die -> 0 (0x7fa934dd0d70)
>     >    DW_AT_artificial: 1
>     >
>     > ...
>     > which can be traced back to gimple dumps:
>     > ...
>     >   char a[0:D.1922] [value-expr: *a.0];
>     > ...
>     >
>     > Furthermore, it adds names to external references:
>     > ...
>     >  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
>     > +DW_AT_name: "main"
>     >  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29
>     (0x7fa88b965140)
>     > ...
>     >
>     > This is an undocumented developer-only option, because using this
>     option may
>     > change behaviour of dwarf consumers, f.i., gdb shows the
>     artificial variables:
>     > ...
>     > (gdb) info locals
>     > a = 0x7fffffffda90 "\005"
>     > D.4278 = <optimized out>
>     > ...
>     >
>     > Any comments?
> 
>     The idea is OK I guess but I'd call it -gforce-named-dies instead
>     of -greadable-dwarf.  It also goes only half-way since it doesn't
>     add names to DECL_NAMELESS vars.
> 
>     There doesn't seem to be a convenient place to
> 
>     > Thanks,
>     > - Tom
>     >
>     > [debug] Add -greadable-dwarf
>     >
>     > 2018-08-15  Tom de Vries  <tdevries@suse.de <mailto:tdevries@suse.de>>
>     >
>     >       * common.opt (greadable-dwarf): Add option.
>     >       * dwarf2out.c (add_name_and_src_coords_attributes): Add
>     param. Add name
>     >       for artifical decls.
>     >       (add_decl_name): New function.
>     >       (dwarf2out_register_external_die): Add name to external
>     reference die.
>     >
>     > ---
>     >  gcc/common.opt  |  5 +++++
>     >  gcc/dwarf2out.c | 24 +++++++++++++++++++++---
>     >  2 files changed, 26 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/gcc/common.opt b/gcc/common.opt
>     > index b2f2215ecc6..6e5e0558e49 100644
>     > --- a/gcc/common.opt
>     > +++ b/gcc/common.opt
>     > @@ -2972,6 +2972,11 @@ gstrict-dwarf
>     >  Common Driver Report Var(dwarf_strict) Init(0)
>     >  Don't emit DWARF additions beyond selected version.
>     > 
>     > +greadable-dwarf
>     > +Common Driver Undocumented Report Var(flag_readable_dwarf) Init(0)
>     > +Make generated dwarf more readable, at the cost of space and
>     exposing compiler
>     > +internals.
>     > +
>     >  gtoggle
>     >  Common Driver Report Var(flag_gtoggle)
>     >  Toggle debug information generation.
>     > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>     > index 4b63cbd8a1e..8c6b4372874 100644
>     > --- a/gcc/dwarf2out.c
>     > +++ b/gcc/dwarf2out.c
>     > @@ -3824,7 +3824,9 @@ static void add_prototyped_attribute
>     (dw_die_ref, tree);
>     >  static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
>     >  static void add_pure_or_virtual_attribute (dw_die_ref, tree);
>     >  static void add_src_coords_attributes (dw_die_ref, tree);
>     > -static void add_name_and_src_coords_attributes (dw_die_ref, tree,
>     bool = false);
>     > +static void add_name_and_src_coords_attributes (dw_die_ref, tree,
>     bool = false,
>     > +                                             bool = false);
>     > +static void add_decl_name (dw_die_ref, tree);
>     >  static void add_discr_value (dw_die_ref, dw_discr_value *);
>     >  static void add_discr_list (dw_die_ref, dw_discr_list_ref);
>     >  static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
>     > @@ -6022,6 +6024,8 @@ dwarf2out_register_external_die (tree decl,
>     const char *sym,
>     >    else
>     >      equate_decl_number_to_die (decl, die);
>     > 
>     > +  if (flag_readable_dwarf)
>     > +    add_decl_name (die, decl);
> 
>     Please use add_name_and_src_coords_attributes directly.
> 
>     >    /* Add a reference to the DIE providing early debug at $sym +
>     off.  */
>     >    add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
>     >  }
>     > @@ -21269,7 +21273,8 @@ add_linkage_name (dw_die_ref die, tree decl)
>     > 
>     >  static void
>     >  add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
>     > -                                 bool no_linkage_name)
>     > +                                 bool no_linkage_name,
>     > +                                 bool no_src_coords_attributes)
>     >  {
>     >    tree decl_name;
>     > 
>     > @@ -21279,12 +21284,19 @@ add_name_and_src_coords_attributes
>     (dw_die_ref die, tree decl,
>     >        const char *name = dwarf2_name (decl, 0);
>     >        if (name)
>     >       add_name_attribute (die, name);
>     > -      if (! DECL_ARTIFICIAL (decl))
>     > +      if (!no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
> 
>     inconsistent spacing after !
> 
>     >       add_src_coords_attributes (die, decl);
>     > 
>     >        if (!no_linkage_name)
>     >       add_linkage_name (die, decl);
>     >      }
>     > +  else if (flag_readable_dwarf && decl_name == NULL)
>     > +    {
>     > +      char *buf = XNEWVEC (char, 32);
>     > +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
>     > +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
>     > +      add_name_attribute (die, buf);
> 
>     I think you leak 'buf'.
> 
>     > +    }
>     > 
>     >  #ifdef VMS_DEBUGGING_INFO
> 
>     how does it interact with this VMS_DEBUGGING_INFO path?
> 
>     >    /* Get the function's name, as described by its RTL.  This may
>     be different
>     > @@ -21298,6 +21310,12 @@ add_name_and_src_coords_attributes
>     (dw_die_ref die, tree decl,
>     >  #endif /* VMS_DEBUGGING_INFO */
>     >  }
>     > 
>     > +static void
>     > +add_decl_name (dw_die_ref die, tree decl)
>     > +{
>     > +  add_name_and_src_coords_attributes (die, decl, true, true);
>     > +}
>     > +
>     >  /* Add VALUE as a DW_AT_discr_value attribute to DIE.  */
>     > 
>     >  static void
>     >
>     >
> 
>     -- 
>     Richard Biener <rguenther@suse.de <mailto:rguenther@suse.de>>
>     SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham
>     Norton, HRB 21284 (AG Nuernberg)
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH][debug] Add -gforce-named-dies
  2018-08-21 12:22   ` Tom de Vries
@ 2018-08-22  6:56     ` Tom de Vries
  2018-08-22  9:46       ` Tom de Vries
  0 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2018-08-22  6:56 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

[ was: Re: [RFC][debug] Add -greadable-dwarf ]

On 08/21/2018 02:22 PM, Tom de Vries wrote:
> Currently doing a bootstrap and reg-test on x86_64 of attached patch
> with -gforce-named-dies enabled by default.

That went well. There are some understandable failures in
libstdc++-prettyprinters/whatis.cc, explained in the rationale below.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0001-debug-Add-gforce-named-dies.patch --]
[-- Type: text/x-patch, Size: 5944 bytes --]

[debug] Add -gforce-named-dies

This patch adds option -gforce-named-dies.  It sets the DW_AT_name attribute
of dies that otherwise do not get that attribute, to make it easier to figure
out what the die is describing.

The option exports the names of artificial variables:
...
 DIE    0: DW_TAG_variable (0x7fa934dd54b0)
+  DW_AT_name: "D.1922"
   DW_AT_type: die -> 0 (0x7fa934dd0d70)
   DW_AT_artificial: 1

...
which can be traced back to gimple dumps:
...
  char a[0:D.1922] [value-expr: *a.0];
...

Furthermore, it adds names to external references:
...
 DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
+DW_AT_name: "main"
 DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
...

This is an undocumented developer-only option, because using this option may
change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
...
(gdb) info locals
a = 0x7fffffffda90 "\005"
D.4278 = <optimized out>
...

Bootstrapped and reg-tested on x86_64, in combination with a patch that
switches the option on by default.  The only failures are in
libstdc++-prettyprinters/whatis.cc, where we get failures of the following
type: we expect to gdb to print
...
type = holder<std::ios>
...
but instead we get
...
  type = holder<std::basic_ios<char, std::char_traits<char> > >
...
where std::ios is defined as:
...
 typedef basic_ios<char>               ios;
 template<typename _CharT, typename _Traits = char_traits<_CharT> > class basic_ios;
...

2018-08-15  Tom de Vries  <tdevries@suse.de>

	* common.opt (gforce-named-dies): Add option.
	* dwarf2out.c (add_name_and_src_coords_attributes): Add param. Add name
	for artifical and nameless decls.
	(dwarf2out_register_external_die): Add name to external reference die.
	(gen_subprogram_die): Add name to DW_TAG_call_site_parameter.

---
 gcc/common.opt  |  5 +++++
 gcc/dwarf2out.c | 27 +++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index ebc3ef43ce2..76032f9bb1d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2976,6 +2976,11 @@ gstrict-dwarf
 Common Driver Report Var(dwarf_strict) Init(0)
 Don't emit DWARF additions beyond selected version.
 
+gforce-named-dies
+Common Driver Undocumented Report Var(flag_force_named_dies) Init(0)
+Force DWARF DIEs to have a name attribute.  Undocumented because it exposes
+compiler internals to the DWARF consumer.
+
 gtoggle
 Common Driver Report Var(flag_gtoggle)
 Toggle debug information generation.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fb71ff349fa..dd8f438dfd3 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3824,7 +3824,8 @@ static void add_prototyped_attribute (dw_die_ref, tree);
 static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
 static void add_pure_or_virtual_attribute (dw_die_ref, tree);
 static void add_src_coords_attributes (dw_die_ref, tree);
-static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false);
+static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false,
+						bool = false);
 static void add_discr_value (dw_die_ref, dw_discr_value *);
 static void add_discr_list (dw_die_ref, dw_discr_list_ref);
 static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
@@ -6022,6 +6023,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
   else
     equate_decl_number_to_die (decl, die);
 
+  if (flag_force_named_dies && DECL_P (decl))
+    add_name_and_src_coords_attributes (die, decl, true, true);
   /* Add a reference to the DIE providing early debug at $sym + off.  */
   add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
 }
@@ -21277,22 +21280,33 @@ add_linkage_name (dw_die_ref die, tree decl)
 
 static void
 add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
-				    bool no_linkage_name)
+				    bool no_linkage_name,
+				    bool no_src_coords_attributes)
 {
   tree decl_name;
 
   decl_name = DECL_NAME (decl);
   if (decl_name != NULL && IDENTIFIER_POINTER (decl_name) != NULL)
     {
-      const char *name = dwarf2_name (decl, 0);
+      const char *name = (flag_force_named_dies && DECL_NAMELESS (decl)
+			  ? IDENTIFIER_POINTER (decl_name)
+			  : dwarf2_name (decl, 0));
       if (name)
 	add_name_attribute (die, name);
-      if (! DECL_ARTIFICIAL (decl))
+      if (! no_src_coords_attributes && ! DECL_ARTIFICIAL (decl))
 	add_src_coords_attributes (die, decl);
 
       if (!no_linkage_name)
 	add_linkage_name (die, decl);
     }
+  else if (flag_force_named_dies && decl_name == NULL
+	   && (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == CONST_DECL))
+    {
+      char buf[32];
+      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
+      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
+      add_name_attribute (die, buf);
+    }
 
 #ifdef VMS_DEBUGGING_INFO
   /* Get the function's name, as described by its RTL.  This may be different
@@ -23265,6 +23279,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 	      dw_die_ref die = NULL;
 	      rtx tloc = NULL_RTX, tlocc = NULL_RTX;
 	      rtx arg, next_arg;
+	      tree arg_decl = NULL_TREE;
 
 	      for (arg = (ca_loc->call_arg_loc_note != NULL_RTX
 			  ? XEXP (ca_loc->call_arg_loc_note, 0)
@@ -23329,6 +23344,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		      tdie = lookup_decl_die (tdecl);
 		      if (tdie == NULL)
 			continue;
+		      arg_decl = tdecl;
 		    }
 		  else
 		    continue;
@@ -23345,6 +23361,9 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		    die = gen_call_site_die (decl, subr_die, ca_loc);
 		  cdie = new_die (dwarf_TAG (DW_TAG_call_site_parameter), die,
 				  NULL_TREE);
+		  if (flag_force_named_dies && arg_decl)
+		    add_name_and_src_coords_attributes (cdie, arg_decl, true,
+							true);
 		  if (reg != NULL)
 		    add_AT_loc (cdie, DW_AT_location, reg);
 		  else if (tdie != NULL)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH][debug] Add -gforce-named-dies
  2018-08-22  6:56     ` [PATCH][debug] Add -gforce-named-dies Tom de Vries
@ 2018-08-22  9:46       ` Tom de Vries
  2018-08-22 13:23         ` [PATCH][debug] Add -gdescriptive-dies Tom de Vries
  0 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2018-08-22  9:46 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

On 08/22/2018 08:56 AM, Tom de Vries wrote:
> This is an undocumented developer-only option, because using this option may
> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
> ...
> (gdb) info locals
> a = 0x7fffffffda90 "\005"
> D.4278 = <optimized out>
> ...

I just found in the dwarf 5 spec the attribute DW_AT_description
(present since version 3):
...
2.20 Entity Descriptions
Some debugging information entries may describe entities in the program
that are artificial, or which otherwise have a “name” that is not a
valid identifier in the programming language.

This attribute provides a means for the producer to indicate the purpose
or usage of the containing debugging infor

Generally, any debugging information entry that has, or may have, a
DW_AT_name attribute, may also have a DW_AT_description attribute whose
value is a null-terminated string providing a description of the entity.

It is expected that a debugger will display these descriptions as part
of displaying other properties of an entity.
...

AFAICT, gdb currently does not explicitly handle this attribute, which I
think means it's ignored.

It seems applicable to use DW_AT_description at least for the artificial
decls.

Perhaps even for all cases that are added by the patch?

I'll rewrite the patch.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH][debug] Add -gdescriptive-dies
  2018-08-22  9:46       ` Tom de Vries
@ 2018-08-22 13:23         ` Tom de Vries
  2018-08-24 10:44           ` Richard Biener
  0 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2018-08-22 13:23 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

[ was: Re: [PATCH][debug] Add -gforce-named-dies ]

On 08/22/2018 11:46 AM, Tom de Vries wrote:
> On 08/22/2018 08:56 AM, Tom de Vries wrote:
>> This is an undocumented developer-only option, because using this option may
>> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
>> ...
>> (gdb) info locals
>> a = 0x7fffffffda90 "\005"
>> D.4278 = <optimized out>
>> ...
> I just found in the dwarf 5 spec the attribute DW_AT_description
> (present since version 3):
> ...
> 2.20 Entity Descriptions
> Some debugging information entries may describe entities in the program
> that are artificial, or which otherwise have a “name” that is not a
> valid identifier in the programming language.
> 
> This attribute provides a means for the producer to indicate the purpose
> or usage of the containing debugging infor
> 
> Generally, any debugging information entry that has, or may have, a
> DW_AT_name attribute, may also have a DW_AT_description attribute whose
> value is a null-terminated string providing a description of the entity.
> 
> It is expected that a debugger will display these descriptions as part
> of displaying other properties of an entity.
> ...
> 
> AFAICT, gdb currently does not explicitly handle this attribute, which I
> think means it's ignored.
> 
> It seems applicable to use DW_AT_description at least for the artificial
> decls.
> 
> Perhaps even for all cases that are added by the patch?
> 

I've chosen for this option. Using DW_AT_desciption instead of
DW_AT_name should minimize difference in gdb behaviour with and without
-gdescriptive-dies.

> I'll rewrite the patch.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0001-debug-Add-gdescriptive-dies.patch --]
[-- Type: text/x-patch, Size: 7174 bytes --]

[debug] Add -gdescriptive-dies

This patch adds option -gdescriptive-dies.  It sets the DW_AT_description
attribute of dies that do not get a DW_AT_name attribute, to make it easier to
figure out what the die is describing.

The option exports the names of artificial variables:
...
 DIE    0: DW_TAG_variable (0x7fa934dd54b0)
+  DW_AT_description: "D.1922"
   DW_AT_type: die -> 0 (0x7fa934dd0d70)
   DW_AT_artificial: 1

...
which can be traced back to gimple dumps:
...
  char a[0:D.1922] [value-expr: *a.0];
...

Furthermore, it adds names to external references:
...
 DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
+DW_AT_description: "main"
 DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
...

Bootstrapped and reg-tested on x86_64, in combination with a patch that
switches the option on by default.

2018-08-15  Tom de Vries  <tdevries@suse.de>

	* common.opt (gdescriptive-dies): Add option.
	* dwarf2out.c (add_name_and_src_coords_attributes): Add description
	attribute for artifical and nameless decls.
	(dwarf2out_register_external_die): Add description attribute to
	external reference die.
	(add_desc_attribute): New functions.
	(gen_subprogram_die): Add description attribute to
	DW_TAG_call_site_parameter.
	* doc/invoke.texi (@item Debugging Options): Add -gdescriptive-dies and
	-gno-descriptive-dies.
	(@item -gdescriptive-dies): Add.

---
 gcc/common.opt      |  4 ++++
 gcc/doc/invoke.texi |  6 +++++-
 gcc/dwarf2out.c     | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index ebc3ef43ce2..c2ba80c323f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2976,6 +2976,10 @@ gstrict-dwarf
 Common Driver Report Var(dwarf_strict) Init(0)
 Don't emit DWARF additions beyond selected version.
 
+gdescriptive-dies
+Common Driver Report Var(flag_descriptive_dies) Init(0)
+Add description attribute to DWARF DIEs that have no name attribute.
+
 gtoggle
 Common Driver Report Var(flag_gtoggle)
 Toggle debug information generation.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 99849ec6467..5a9c5b49481 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -372,7 +372,7 @@ Objective-C and Objective-C++ Dialects}.
 -ginternal-reset-location-views  -gno-internal-reset-location-views @gol
 -ginline-points  -gno-inline-points @gol
 -gvms  -gxcoff  -gxcoff+  -gz@r{[}=@var{type}@r{]} @gol
--gsplit-dwarf @gol
+-gsplit-dwarf -gdescriptive-dies -gno-descriptive-dies @gol
 -fdebug-prefix-map=@var{old}=@var{new}  -fdebug-types-section @gol
 -fno-eliminate-unused-debug-types @gol
 -femit-struct-debug-baseonly  -femit-struct-debug-reduced @gol
@@ -7395,6 +7395,10 @@ the build system to avoid linking files with debug information.  To
 be useful, this option requires a debugger capable of reading @file{.dwo}
 files.
 
+@item -gdescriptive-dies
+@opindex gdescriptive-dies
+Add description attribute to DWARF DIEs that have no name attribute.
+
 @item -gpubnames
 @opindex gpubnames
 Generate DWARF @code{.debug_pubnames} and @code{.debug_pubtypes} sections.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fb71ff349fa..e1d4ea50695 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3808,6 +3808,7 @@ static bool add_location_or_const_value_attribute (dw_die_ref, tree, bool);
 static bool tree_add_const_value_attribute (dw_die_ref, tree);
 static bool tree_add_const_value_attribute_for_decl (dw_die_ref, tree);
 static void add_name_attribute (dw_die_ref, const char *);
+static void add_desc_attribute (dw_die_ref, tree);
 static void add_gnat_descriptive_type_attribute (dw_die_ref, tree, dw_die_ref);
 static void add_comp_dir_attribute (dw_die_ref);
 static void add_scalar_info (dw_die_ref, enum dwarf_attribute, tree, int,
@@ -6022,6 +6023,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
   else
     equate_decl_number_to_die (decl, die);
 
+  add_desc_attribute (die, decl);
+
   /* Add a reference to the DIE providing early debug at $sym + off.  */
   add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
 }
@@ -20529,6 +20532,53 @@ add_name_attribute (dw_die_ref die, const char *name_string)
     }
 }
 
+/* Generate a DW_AT_description attribute given some string value to be included
+   as the value of the attribute.  */
+
+static void
+add_desc_attribute (dw_die_ref die, const char *name_string)
+{
+  if (!flag_descriptive_dies || dwarf_version < 3)
+    return;
+
+  if (name_string != NULL && *name_string != 0)
+    {
+      if (demangle_name_func)
+	name_string = (*demangle_name_func) (name_string);
+
+      add_AT_string (die, DW_AT_description, name_string);
+    }
+}
+
+/* Generate a DW_AT_description attribute given some decl to be included
+   as the value of the attribute.  */
+
+static void
+add_desc_attribute (dw_die_ref die, tree decl)
+{
+  tree decl_name;
+
+  if (!flag_descriptive_dies || dwarf_version < 3)
+    return;
+
+  if (decl == NULL_TREE || !DECL_P (decl))
+    return;
+  decl_name = DECL_NAME (decl);
+
+  if (decl_name != NULL && IDENTIFIER_POINTER (decl_name) != NULL)
+    {
+      const char *name = dwarf2_name (decl, 0);
+      add_desc_attribute (die, name ? name : IDENTIFIER_POINTER (decl_name));
+    }
+  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == CONST_DECL)
+    {
+      char buf[32];
+      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
+      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
+      add_desc_attribute (die, buf);
+    }
+}
+
 /* Retrieve the descriptive type of TYPE, if any, make sure it has a
    DIE and attach a DW_AT_GNAT_descriptive_type attribute to the DIE
    of TYPE accordingly.
@@ -21287,12 +21337,17 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
       const char *name = dwarf2_name (decl, 0);
       if (name)
 	add_name_attribute (die, name);
+      else
+	add_desc_attribute (die, decl);
+
       if (! DECL_ARTIFICIAL (decl))
 	add_src_coords_attributes (die, decl);
 
       if (!no_linkage_name)
 	add_linkage_name (die, decl);
     }
+  else
+    add_desc_attribute (die, decl);
 
 #ifdef VMS_DEBUGGING_INFO
   /* Get the function's name, as described by its RTL.  This may be different
@@ -23265,6 +23320,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 	      dw_die_ref die = NULL;
 	      rtx tloc = NULL_RTX, tlocc = NULL_RTX;
 	      rtx arg, next_arg;
+	      tree arg_decl = NULL_TREE;
 
 	      for (arg = (ca_loc->call_arg_loc_note != NULL_RTX
 			  ? XEXP (ca_loc->call_arg_loc_note, 0)
@@ -23329,6 +23385,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		      tdie = lookup_decl_die (tdecl);
 		      if (tdie == NULL)
 			continue;
+		      arg_decl = tdecl;
 		    }
 		  else
 		    continue;
@@ -23345,6 +23402,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		    die = gen_call_site_die (decl, subr_die, ca_loc);
 		  cdie = new_die (dwarf_TAG (DW_TAG_call_site_parameter), die,
 				  NULL_TREE);
+		  add_desc_attribute (cdie, arg_decl);
 		  if (reg != NULL)
 		    add_AT_loc (cdie, DW_AT_location, reg);
 		  else if (tdie != NULL)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH][debug] Add -gdescriptive-dies
  2018-08-22 13:23         ` [PATCH][debug] Add -gdescriptive-dies Tom de Vries
@ 2018-08-24 10:44           ` Richard Biener
  2018-08-24 15:38             ` [PATCH][debug] Add -gdescribe-dies Tom de Vries
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Biener @ 2018-08-24 10:44 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

On Wed, 22 Aug 2018, Tom de Vries wrote:

> [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
> 
> On 08/22/2018 11:46 AM, Tom de Vries wrote:
> > On 08/22/2018 08:56 AM, Tom de Vries wrote:
> >> This is an undocumented developer-only option, because using this option may
> >> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
> >> ...
> >> (gdb) info locals
> >> a = 0x7fffffffda90 "\005"
> >> D.4278 = <optimized out>
> >> ...
> > I just found in the dwarf 5 spec the attribute DW_AT_description
> > (present since version 3):
> > ...
> > 2.20 Entity Descriptions
> > Some debugging information entries may describe entities in the program
> > that are artificial, or which otherwise have a “name” that is not a
> > valid identifier in the programming language.
> > 
> > This attribute provides a means for the producer to indicate the purpose
> > or usage of the containing debugging infor
> > 
> > Generally, any debugging information entry that has, or may have, a
> > DW_AT_name attribute, may also have a DW_AT_description attribute whose
> > value is a null-terminated string providing a description of the entity.
> > 
> > It is expected that a debugger will display these descriptions as part
> > of displaying other properties of an entity.
> > ...
> > 
> > AFAICT, gdb currently does not explicitly handle this attribute, which I
> > think means it's ignored.
> > 
> > It seems applicable to use DW_AT_description at least for the artificial
> > decls.
> > 
> > Perhaps even for all cases that are added by the patch?
> > 
> 
> I've chosen for this option. Using DW_AT_desciption instead of
> DW_AT_name should minimize difference in gdb behaviour with and without
> -gdescriptive-dies.

-gdescribe-dies?

> > I'll rewrite the patch.
> 
> OK for trunk?

Few comments:

+static void
+add_desc_attribute (dw_die_ref die, tree decl)
+{
+  tree decl_name;
+
+  if (!flag_descriptive_dies || dwarf_version < 3)
+    return;
+

you said this is a DWARF5 "feature", I'd suggest changing the
check to

  if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))

given -gdescribe-dies is enough of a user request to enable the
feature.  Not sure if we should warn about -gstrict-dwarf
combination with it and -gdwarf-N < 5.

+  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == 
CONST_DECL)
+    {
+      char buf[32];
+      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
+      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
+      add_desc_attribute (die, buf);
+    }

I wondered before if we can use pretty-printing of decl here.  At
least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
certainly appear here I think.

+@item -gdescriptive-dies
+@opindex gdescriptive-dies
+Add description attribute to DWARF DIEs that have no name attribute.
+

Either "description attributes" or "a description attribute", not
100% sure being a non-native speaker.

Otherwise the patch looks OK to me but please leave Jason time
to comment.

Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH][debug] Add -gdescribe-dies
  2018-08-24 10:44           ` Richard Biener
@ 2018-08-24 15:38             ` Tom de Vries
  2018-08-31 21:19               ` Jason Merrill
  0 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2018-08-24 15:38 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jakub Jelinek, Jason Merrill, Cary Coutant

[ was: Re: [PATCH][debug] Add -gdescriptive-dies ]
On Fri, Aug 24, 2018 at 12:44:38PM +0200, Richard Biener wrote:
> On Wed, 22 Aug 2018, Tom de Vries wrote:
> 
> > [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
> > 
> > On 08/22/2018 11:46 AM, Tom de Vries wrote:
> > > On 08/22/2018 08:56 AM, Tom de Vries wrote:
> > >> This is an undocumented developer-only option, because using this option may
> > >> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
> > >> ...
> > >> (gdb) info locals
> > >> a = 0x7fffffffda90 "\005"
> > >> D.4278 = <optimized out>
> > >> ...
> > > I just found in the dwarf 5 spec the attribute DW_AT_description
> > > (present since version 3):
> > > ...
> > > 2.20 Entity Descriptions
> > > Some debugging information entries may describe entities in the program
> > > that are artificial, or which otherwise have a “name” that is not a
> > > valid identifier in the programming language.
> > > 
> > > This attribute provides a means for the producer to indicate the purpose
> > > or usage of the containing debugging infor
> > > 
> > > Generally, any debugging information entry that has, or may have, a
> > > DW_AT_name attribute, may also have a DW_AT_description attribute whose
> > > value is a null-terminated string providing a description of the entity.
> > > 
> > > It is expected that a debugger will display these descriptions as part
> > > of displaying other properties of an entity.
> > > ...
> > > 
> > > AFAICT, gdb currently does not explicitly handle this attribute, which I
> > > think means it's ignored.
> > > 
> > > It seems applicable to use DW_AT_description at least for the artificial
> > > decls.
> > > 
> > > Perhaps even for all cases that are added by the patch?
> > > 
> > 
> > I've chosen for this option. Using DW_AT_desciption instead of
> > DW_AT_name should minimize difference in gdb behaviour with and without
> > -gdescriptive-dies.
> 
> -gdescribe-dies?
>

Done.

> > > I'll rewrite the patch.
> > 
> > OK for trunk?
> 
> Few comments:
> 
> +static void
> +add_desc_attribute (dw_die_ref die, tree decl)
> +{
> +  tree decl_name;
> +
> +  if (!flag_descriptive_dies || dwarf_version < 3)
> +    return;
> +
> 
> you said this is a DWARF5 "feature",

No, it's a DWARF3 "feature". I copied the text from the DWARF5 spec.

> I'd suggest changing the
> check to
> 
>   if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))
> 
> given -gdescribe-dies is enough of a user request to enable the
> feature.

Done.

> Not sure if we should warn about -gstrict-dwarf
> combination with it and -gdwarf-N < 5.
>

Not sure either, left it out.

> +  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == 
> CONST_DECL)
> +    {
> +      char buf[32];
> +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
> +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
> +      add_desc_attribute (die, buf);
> +    }
> 
> I wondered before if we can use pretty-printing of decl here.  At
> least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
> certainly appear here I think.
>

Done.

> +@item -gdescriptive-dies
> +@opindex gdescriptive-dies
> +Add description attribute to DWARF DIEs that have no name attribute.
> +
> 
> Either "description attributes" or "a description attribute", not
> 100% sure being a non-native speaker.
>

Went for "description attributes".

> Otherwise the patch looks OK to me but please leave Jason time
> to comment.
>

Will do.

Untested patch attached.

Thanks,
- Tom

[debug] Add -gdescribe-dies

This patch adds option -gdescribe-dies.  It sets the DW_AT_description
attribute of dies that do not get a DW_AT_name attribute, to make it easier to
figure out what the die is describing.

The option exports the names of artificial variables:
...
 DIE    0: DW_TAG_variable (0x7fa934dd54b0)
+  DW_AT_description: "D.1922"
   DW_AT_type: die -> 0 (0x7fa934dd0d70)
   DW_AT_artificial: 1

...
which can be traced back to gimple dumps:
...
  char a[0:D.1922] [value-expr: *a.0];
...

Furthermore, it adds names to external references:
...
 DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
+DW_AT_description: "main"
 DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
...

2018-08-15  Tom de Vries  <tdevries@suse.de>

	* common.opt (gdescribe-dies): Add option.
	* dwarf2out.c (add_name_and_src_coords_attributes): Add description
	attribute for artifical and nameless decls.
	(dwarf2out_register_external_die): Add description attribute to
	external reference die.
	(add_desc_attribute): New functions.
	(gen_subprogram_die): Add description attribute to
	DW_TAG_call_site_parameter.
	* tree-pretty-print.c (print_generic_expr_to_str): New function.
	* tree-pretty-print.h (print_generic_expr_to_str): Declare.
	* doc/invoke.texi (@item Debugging Options): Add -gdescribe-dies and
	-gno-describe-dies.
	(@item -gdescribe-dies): Add.

---
 gcc/common.opt          |  4 ++++
 gcc/doc/invoke.texi     |  6 +++++-
 gcc/dwarf2out.c         | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/tree-pretty-print.c | 10 +++++++++
 gcc/tree-pretty-print.h |  1 +
 5 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index ebc3ef43ce2..f41f8f74b30 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2976,6 +2976,10 @@ gstrict-dwarf
 Common Driver Report Var(dwarf_strict) Init(0)
 Don't emit DWARF additions beyond selected version.
 
+gdescribe-dies
+Common Driver Report Var(flag_describe_dies) Init(0)
+Add description attributes to DWARF DIEs that have no name attribute.
+
 gtoggle
 Common Driver Report Var(flag_gtoggle)
 Toggle debug information generation.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 99849ec6467..5b51eeca594 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -372,7 +372,7 @@ Objective-C and Objective-C++ Dialects}.
 -ginternal-reset-location-views  -gno-internal-reset-location-views @gol
 -ginline-points  -gno-inline-points @gol
 -gvms  -gxcoff  -gxcoff+  -gz@r{[}=@var{type}@r{]} @gol
--gsplit-dwarf @gol
+-gsplit-dwarf -gdescribe-dies -gno-describe-dies @gol
 -fdebug-prefix-map=@var{old}=@var{new}  -fdebug-types-section @gol
 -fno-eliminate-unused-debug-types @gol
 -femit-struct-debug-baseonly  -femit-struct-debug-reduced @gol
@@ -7395,6 +7395,10 @@ the build system to avoid linking files with debug information.  To
 be useful, this option requires a debugger capable of reading @file{.dwo}
 files.
 
+@item -gdescribe-dies
+@opindex gdescribe-dies
+Add description attributes to DWARF DIEs that have no name attribute.
+
 @item -gpubnames
 @opindex gpubnames
 Generate DWARF @code{.debug_pubnames} and @code{.debug_pubtypes} sections.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fb71ff349fa..1fc1dbb3392 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3808,6 +3808,7 @@ static bool add_location_or_const_value_attribute (dw_die_ref, tree, bool);
 static bool tree_add_const_value_attribute (dw_die_ref, tree);
 static bool tree_add_const_value_attribute_for_decl (dw_die_ref, tree);
 static void add_name_attribute (dw_die_ref, const char *);
+static void add_desc_attribute (dw_die_ref, tree);
 static void add_gnat_descriptive_type_attribute (dw_die_ref, tree, dw_die_ref);
 static void add_comp_dir_attribute (dw_die_ref);
 static void add_scalar_info (dw_die_ref, enum dwarf_attribute, tree, int,
@@ -6022,6 +6023,8 @@ dwarf2out_register_external_die (tree decl, const char *sym,
   else
     equate_decl_number_to_die (decl, die);
 
+  add_desc_attribute (die, decl);
+
   /* Add a reference to the DIE providing early debug at $sym + off.  */
   add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
 }
@@ -20529,6 +20532,52 @@ add_name_attribute (dw_die_ref die, const char *name_string)
     }
 }
 
+/* Generate a DW_AT_description attribute given some string value to be included
+   as the value of the attribute.  */
+
+static void
+add_desc_attribute (dw_die_ref die, const char *name_string)
+{
+  if (!flag_describe_dies || (dwarf_version < 3 && dwarf_strict))
+    return;
+
+  if (name_string == NULL || *name_string == 0)
+    return;
+
+  if (demangle_name_func)
+    name_string = (*demangle_name_func) (name_string);
+
+  add_AT_string (die, DW_AT_description, name_string);
+}
+
+/* Generate a DW_AT_description attribute given some decl to be included
+   as the value of the attribute.  */
+
+static void
+add_desc_attribute (dw_die_ref die, tree decl)
+{
+  tree decl_name;
+
+  if (!flag_describe_dies || (dwarf_version < 3 && dwarf_strict))
+    return;
+
+  if (decl == NULL_TREE || !DECL_P (decl))
+    return;
+  decl_name = DECL_NAME (decl);
+
+  if (decl_name != NULL && IDENTIFIER_POINTER (decl_name) != NULL)
+    {
+      const char *name = dwarf2_name (decl, 0);
+      add_desc_attribute (die, name ? name : IDENTIFIER_POINTER (decl_name));
+    }
+  else
+    {
+      char *desc = print_generic_expr_to_str (decl);
+      add_desc_attribute (die, desc);
+      free (desc);
+    }
+}
+
 /* Retrieve the descriptive type of TYPE, if any, make sure it has a
    DIE and attach a DW_AT_GNAT_descriptive_type attribute to the DIE
    of TYPE accordingly.
@@ -21287,12 +21336,17 @@ add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
       const char *name = dwarf2_name (decl, 0);
       if (name)
 	add_name_attribute (die, name);
+      else
+	add_desc_attribute (die, decl);
+
       if (! DECL_ARTIFICIAL (decl))
 	add_src_coords_attributes (die, decl);
 
       if (!no_linkage_name)
 	add_linkage_name (die, decl);
     }
+  else
+    add_desc_attribute (die, decl);
 
 #ifdef VMS_DEBUGGING_INFO
   /* Get the function's name, as described by its RTL.  This may be different
@@ -23265,6 +23319,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 	      dw_die_ref die = NULL;
 	      rtx tloc = NULL_RTX, tlocc = NULL_RTX;
 	      rtx arg, next_arg;
+	      tree arg_decl = NULL_TREE;
 
 	      for (arg = (ca_loc->call_arg_loc_note != NULL_RTX
 			  ? XEXP (ca_loc->call_arg_loc_note, 0)
@@ -23329,6 +23384,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		      tdie = lookup_decl_die (tdecl);
 		      if (tdie == NULL)
 			continue;
+		      arg_decl = tdecl;
 		    }
 		  else
 		    continue;
@@ -23345,6 +23401,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 		    die = gen_call_site_die (decl, subr_die, ca_loc);
 		  cdie = new_die (dwarf_TAG (DW_TAG_call_site_parameter), die,
 				  NULL_TREE);
+		  add_desc_attribute (cdie, arg_decl);
 		  if (reg != NULL)
 		    add_AT_loc (cdie, DW_AT_location, reg);
 		  else if (tdie != NULL)
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 627d8d7e2d7..a4489d7f5ab 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -162,6 +162,16 @@ print_generic_expr (FILE *file, tree t, dump_flags_t flags)
   pp_flush (tree_pp);
 }
 
+/* Print a single expression T to string, and return it.  */
+
+char *
+print_generic_expr_to_str (tree t)
+{
+  pretty_printer pp;
+  dump_generic_node (&pp, t, 0, TDF_VOPS|TDF_MEMSYMS, false);
+  return xstrdup (pp_formatted_text (&pp));
+}
+
 /* Dump NAME, an IDENTIFIER_POINTER, sanitized so that D<num> sequences
    in it are replaced with Dxxxx, as long as they are at the start or
    preceded by $ and at the end or followed by $.  See make_fancy_name
diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
index adfc77b29cd..4de0e090cba 100644
--- a/gcc/tree-pretty-print.h
+++ b/gcc/tree-pretty-print.h
@@ -38,6 +38,7 @@ extern void print_generic_decl (FILE *, tree, dump_flags_t);
 extern void print_generic_stmt (FILE *, tree, dump_flags_t = TDF_NONE);
 extern void print_generic_stmt_indented (FILE *, tree, dump_flags_t, int);
 extern void print_generic_expr (FILE *, tree, dump_flags_t = TDF_NONE);
+extern char *print_generic_expr_to_str (tree);
 extern void dump_omp_clauses (pretty_printer *, tree, int, dump_flags_t);
 extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t, bool);
 extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH][debug] Add -gdescribe-dies
  2018-08-24 15:38             ` [PATCH][debug] Add -gdescribe-dies Tom de Vries
@ 2018-08-31 21:19               ` Jason Merrill
  2018-09-01 18:10                 ` Tom de Vries
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2018-08-31 21:19 UTC (permalink / raw)
  To: Tom de Vries
  Cc: Richard Biener, gcc-patches List, Jakub Jelinek, Cary Coutant

On Fri, Aug 24, 2018 at 11:38 AM, Tom de Vries <tdevries@suse.de> wrote:
> [ was: Re: [PATCH][debug] Add -gdescriptive-dies ]
> On Fri, Aug 24, 2018 at 12:44:38PM +0200, Richard Biener wrote:
>> On Wed, 22 Aug 2018, Tom de Vries wrote:
>>
>> > [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
>> >
>> > On 08/22/2018 11:46 AM, Tom de Vries wrote:
>> > > On 08/22/2018 08:56 AM, Tom de Vries wrote:
>> > >> This is an undocumented developer-only option, because using this option may
>> > >> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
>> > >> ...
>> > >> (gdb) info locals
>> > >> a = 0x7fffffffda90 "\005"
>> > >> D.4278 = <optimized out>
>> > >> ...
>> > > I just found in the dwarf 5 spec the attribute DW_AT_description
>> > > (present since version 3):
>> > > ...
>> > > 2.20 Entity Descriptions
>> > > Some debugging information entries may describe entities in the program
>> > > that are artificial, or which otherwise have a “name” that is not a
>> > > valid identifier in the programming language.
>> > >
>> > > This attribute provides a means for the producer to indicate the purpose
>> > > or usage of the containing debugging infor
>> > >
>> > > Generally, any debugging information entry that has, or may have, a
>> > > DW_AT_name attribute, may also have a DW_AT_description attribute whose
>> > > value is a null-terminated string providing a description of the entity.
>> > >
>> > > It is expected that a debugger will display these descriptions as part
>> > > of displaying other properties of an entity.
>> > > ...
>> > >
>> > > AFAICT, gdb currently does not explicitly handle this attribute, which I
>> > > think means it's ignored.
>> > >
>> > > It seems applicable to use DW_AT_description at least for the artificial
>> > > decls.
>> > >
>> > > Perhaps even for all cases that are added by the patch?
>> > >
>> >
>> > I've chosen for this option. Using DW_AT_desciption instead of
>> > DW_AT_name should minimize difference in gdb behaviour with and without
>> > -gdescriptive-dies.
>>
>> -gdescribe-dies?
>>
>
> Done.
>
>> > > I'll rewrite the patch.
>> >
>> > OK for trunk?
>>
>> Few comments:
>>
>> +static void
>> +add_desc_attribute (dw_die_ref die, tree decl)
>> +{
>> +  tree decl_name;
>> +
>> +  if (!flag_descriptive_dies || dwarf_version < 3)
>> +    return;
>> +
>>
>> you said this is a DWARF5 "feature",
>
> No, it's a DWARF3 "feature". I copied the text from the DWARF5 spec.
>
>> I'd suggest changing the
>> check to
>>
>>   if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))
>>
>> given -gdescribe-dies is enough of a user request to enable the
>> feature.
>
> Done.
>
>> Not sure if we should warn about -gstrict-dwarf
>> combination with it and -gdwarf-N < 5.
>>
>
> Not sure either, left it out.
>
>> +  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) ==
>> CONST_DECL)
>> +    {
>> +      char buf[32];
>> +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
>> +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
>> +      add_desc_attribute (die, buf);
>> +    }
>>
>> I wondered before if we can use pretty-printing of decl here.  At
>> least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
>> certainly appear here I think.
>>
>
> Done.
>
>> +@item -gdescriptive-dies
>> +@opindex gdescriptive-dies
>> +Add description attribute to DWARF DIEs that have no name attribute.
>> +
>>
>> Either "description attributes" or "a description attribute", not
>> 100% sure being a non-native speaker.
>>
>
> Went for "description attributes".
>
>> Otherwise the patch looks OK to me but please leave Jason time
>> to comment.
>>
>
> Will do.
>
> Untested patch attached.
>
> Thanks,
> - Tom
>
> [debug] Add -gdescribe-dies
>
> This patch adds option -gdescribe-dies.  It sets the DW_AT_description
> attribute of dies that do not get a DW_AT_name attribute, to make it easier to
> figure out what the die is describing.
>
> The option exports the names of artificial variables:
> ...
>  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
> +  DW_AT_description: "D.1922"
>    DW_AT_type: die -> 0 (0x7fa934dd0d70)
>    DW_AT_artificial: 1
>
> ...
> which can be traced back to gimple dumps:
> ...
>   char a[0:D.1922] [value-expr: *a.0];
> ...
>
> Furthermore, it adds names to external references:
> ...
>  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
> +DW_AT_description: "main"
>  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)

Please add more of this description to the one-line documentation
patch you have now; there are many DIEs that have no name because they
don't need one, and this patch doesn't add names to all of them.

These two cases seem like they have very different uses, but I suppose
we don't really need two new options.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH][debug] Add -gdescribe-dies
  2018-08-31 21:19               ` Jason Merrill
@ 2018-09-01 18:10                 ` Tom de Vries
  2018-09-11 15:32                   ` [PING][PATCH][debug] " Tom de Vries
  2018-09-11 16:16                   ` [PATCH][debug] " Jason Merrill
  0 siblings, 2 replies; 15+ messages in thread
From: Tom de Vries @ 2018-09-01 18:10 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Richard Biener, gcc-patches List, Jakub Jelinek, Cary Coutant

[-- Attachment #1: Type: text/plain, Size: 5333 bytes --]

On 08/31/2018 11:19 PM, Jason Merrill wrote:
> On Fri, Aug 24, 2018 at 11:38 AM, Tom de Vries <tdevries@suse.de> wrote:
>> [ was: Re: [PATCH][debug] Add -gdescriptive-dies ]
>> On Fri, Aug 24, 2018 at 12:44:38PM +0200, Richard Biener wrote:
>>> On Wed, 22 Aug 2018, Tom de Vries wrote:
>>>
>>>> [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
>>>>
>>>> On 08/22/2018 11:46 AM, Tom de Vries wrote:
>>>>> On 08/22/2018 08:56 AM, Tom de Vries wrote:
>>>>>> This is an undocumented developer-only option, because using this option may
>>>>>> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
>>>>>> ...
>>>>>> (gdb) info locals
>>>>>> a = 0x7fffffffda90 "\005"
>>>>>> D.4278 = <optimized out>
>>>>>> ...
>>>>> I just found in the dwarf 5 spec the attribute DW_AT_description
>>>>> (present since version 3):
>>>>> ...
>>>>> 2.20 Entity Descriptions
>>>>> Some debugging information entries may describe entities in the program
>>>>> that are artificial, or which otherwise have a “name” that is not a
>>>>> valid identifier in the programming language.
>>>>>
>>>>> This attribute provides a means for the producer to indicate the purpose
>>>>> or usage of the containing debugging infor
>>>>>
>>>>> Generally, any debugging information entry that has, or may have, a
>>>>> DW_AT_name attribute, may also have a DW_AT_description attribute whose
>>>>> value is a null-terminated string providing a description of the entity.
>>>>>
>>>>> It is expected that a debugger will display these descriptions as part
>>>>> of displaying other properties of an entity.
>>>>> ...
>>>>>
>>>>> AFAICT, gdb currently does not explicitly handle this attribute, which I
>>>>> think means it's ignored.
>>>>>
>>>>> It seems applicable to use DW_AT_description at least for the artificial
>>>>> decls.
>>>>>
>>>>> Perhaps even for all cases that are added by the patch?
>>>>>
>>>>
>>>> I've chosen for this option. Using DW_AT_desciption instead of
>>>> DW_AT_name should minimize difference in gdb behaviour with and without
>>>> -gdescriptive-dies.
>>>
>>> -gdescribe-dies?
>>>
>>
>> Done.
>>
>>>>> I'll rewrite the patch.
>>>>
>>>> OK for trunk?
>>>
>>> Few comments:
>>>
>>> +static void
>>> +add_desc_attribute (dw_die_ref die, tree decl)
>>> +{
>>> +  tree decl_name;
>>> +
>>> +  if (!flag_descriptive_dies || dwarf_version < 3)
>>> +    return;
>>> +
>>>
>>> you said this is a DWARF5 "feature",
>>
>> No, it's a DWARF3 "feature". I copied the text from the DWARF5 spec.
>>
>>> I'd suggest changing the
>>> check to
>>>
>>>   if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))
>>>
>>> given -gdescribe-dies is enough of a user request to enable the
>>> feature.
>>
>> Done.
>>
>>> Not sure if we should warn about -gstrict-dwarf
>>> combination with it and -gdwarf-N < 5.
>>>
>>
>> Not sure either, left it out.
>>
>>> +  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) ==
>>> CONST_DECL)
>>> +    {
>>> +      char buf[32];
>>> +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
>>> +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
>>> +      add_desc_attribute (die, buf);
>>> +    }
>>>
>>> I wondered before if we can use pretty-printing of decl here.  At
>>> least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
>>> certainly appear here I think.
>>>
>>
>> Done.
>>
>>> +@item -gdescriptive-dies
>>> +@opindex gdescriptive-dies
>>> +Add description attribute to DWARF DIEs that have no name attribute.
>>> +
>>>
>>> Either "description attributes" or "a description attribute", not
>>> 100% sure being a non-native speaker.
>>>
>>
>> Went for "description attributes".
>>
>>> Otherwise the patch looks OK to me but please leave Jason time
>>> to comment.
>>>
>>
>> Will do.
>>
>> Untested patch attached.
>>

Now bootstrapped and reg-tested on x86_64.

>> [debug] Add -gdescribe-dies
>>
>> This patch adds option -gdescribe-dies.  It sets the DW_AT_description
>> attribute of dies that do not get a DW_AT_name attribute, to make it easier to
>> figure out what the die is describing.
>>
>> The option exports the names of artificial variables:
>> ...
>>  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
>> +  DW_AT_description: "D.1922"
>>    DW_AT_type: die -> 0 (0x7fa934dd0d70)
>>    DW_AT_artificial: 1
>>
>> ...
>> which can be traced back to gimple dumps:
>> ...
>>   char a[0:D.1922] [value-expr: *a.0];
>> ...
>>
>> Furthermore, it adds names to external references:
>> ...
>>  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
>> +DW_AT_description: "main"
>>  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
> 
> Please add more of this description to the one-line documentation
> patch you have now;

Done.

> there are many DIEs that have no name because they
> don't need one, and this patch doesn't add names to all of them.
> 

I've added the qualification "some" (to both documentation and command
line option description).

> These two cases seem like they have very different uses, but I suppose
> we don't really need two new options.

In both cases we add a description of the DIE. Could you explain what
the difference in usage you refer to is?

Attached patch (which I will merge with the posted patch pre-commit)
fixes up documentation and command line option description.

OK?

Thanks,
- Tom

[-- Attachment #2: 0002-fixup.patch --]
[-- Type: text/x-patch, Size: 1082 bytes --]

fixup

---
 gcc/common.opt      | 2 +-
 gcc/doc/invoke.texi | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index f41f8f74b30..ef6a63087af 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2978,7 +2978,7 @@ Don't emit DWARF additions beyond selected version.
 
 gdescribe-dies
 Common Driver Report Var(flag_describe_dies) Init(0)
-Add description attributes to DWARF DIEs that have no name attribute.
+Add description attributes to some DWARF DIEs that have no name attribute.
 
 gtoggle
 Common Driver Report Var(flag_gtoggle)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b06189f73ce..d7cf30ad2d1 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -7468,7 +7468,9 @@ files.
 
 @item -gdescribe-dies
 @opindex gdescribe-dies
-Add description attributes to DWARF DIEs that have no name attribute.
+Add description attributes to some DWARF DIEs that have no name attribute,
+such as artificial variables, external references and call site
+parameter DIEs.
 
 @item -gpubnames
 @opindex gpubnames

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PING][PATCH][debug] Add -gdescribe-dies
  2018-09-01 18:10                 ` Tom de Vries
@ 2018-09-11 15:32                   ` Tom de Vries
  2018-09-11 16:16                   ` [PATCH][debug] " Jason Merrill
  1 sibling, 0 replies; 15+ messages in thread
From: Tom de Vries @ 2018-09-11 15:32 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Richard Biener, gcc-patches List, Jakub Jelinek, Cary Coutant

On 09/01/2018 08:10 PM, Tom de Vries wrote:
>> Please add more of this description to the one-line documentation
>> patch you have now;
> Done.
> 
>> there are many DIEs that have no name because they
>> don't need one, and this patch doesn't add names to all of them.
>>
> I've added the qualification "some" (to both documentation and command
> line option description).
> 
>> These two cases seem like they have very different uses, but I suppose
>> we don't really need two new options.
> In both cases we add a description of the DIE. Could you explain what
> the difference in usage you refer to is?
> 
> Attached patch (which I will merge with the posted patch pre-commit)
> fixes up documentation and command line option description.
> 
> OK?
> 

Ping.

Thanks,
- Tom
> 
> 
> 0002-fixup.patch
> 
> 
> fixup
> 
> ---
>  gcc/common.opt      | 2 +-
>  gcc/doc/invoke.texi | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index f41f8f74b30..ef6a63087af 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2978,7 +2978,7 @@ Don't emit DWARF additions beyond selected version.
>  
>  gdescribe-dies
>  Common Driver Report Var(flag_describe_dies) Init(0)
> -Add description attributes to DWARF DIEs that have no name attribute.
> +Add description attributes to some DWARF DIEs that have no name attribute.
>  
>  gtoggle
>  Common Driver Report Var(flag_gtoggle)
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index b06189f73ce..d7cf30ad2d1 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -7468,7 +7468,9 @@ files.
>  
>  @item -gdescribe-dies
>  @opindex gdescribe-dies
> -Add description attributes to DWARF DIEs that have no name attribute.
> +Add description attributes to some DWARF DIEs that have no name attribute,
> +such as artificial variables, external references and call site
> +parameter DIEs.
>  
>  @item -gpubnames
>  @opindex gpubnames

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH][debug] Add -gdescribe-dies
  2018-09-01 18:10                 ` Tom de Vries
  2018-09-11 15:32                   ` [PING][PATCH][debug] " Tom de Vries
@ 2018-09-11 16:16                   ` Jason Merrill
  1 sibling, 0 replies; 15+ messages in thread
From: Jason Merrill @ 2018-09-11 16:16 UTC (permalink / raw)
  To: Tom de Vries
  Cc: Richard Biener, gcc-patches List, Jakub Jelinek, Cary Coutant

On Sat, Sep 1, 2018 at 7:10 PM, Tom de Vries <tdevries@suse.de> wrote:
> On 08/31/2018 11:19 PM, Jason Merrill wrote:
>> On Fri, Aug 24, 2018 at 11:38 AM, Tom de Vries <tdevries@suse.de> wrote:
>>> [ was: Re: [PATCH][debug] Add -gdescriptive-dies ]
>>> On Fri, Aug 24, 2018 at 12:44:38PM +0200, Richard Biener wrote:
>>>> On Wed, 22 Aug 2018, Tom de Vries wrote:
>>>>
>>>>> [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
>>>>>
>>>>> On 08/22/2018 11:46 AM, Tom de Vries wrote:
>>>>>> On 08/22/2018 08:56 AM, Tom de Vries wrote:
>>>>>>> This is an undocumented developer-only option, because using this option may
>>>>>>> change behaviour of dwarf consumers, f.i., gdb shows the artificial variables:
>>>>>>> ...
>>>>>>> (gdb) info locals
>>>>>>> a = 0x7fffffffda90 "\005"
>>>>>>> D.4278 = <optimized out>
>>>>>>> ...
>>>>>> I just found in the dwarf 5 spec the attribute DW_AT_description
>>>>>> (present since version 3):
>>>>>> ...
>>>>>> 2.20 Entity Descriptions
>>>>>> Some debugging information entries may describe entities in the program
>>>>>> that are artificial, or which otherwise have a “name” that is not a
>>>>>> valid identifier in the programming language.
>>>>>>
>>>>>> This attribute provides a means for the producer to indicate the purpose
>>>>>> or usage of the containing debugging infor
>>>>>>
>>>>>> Generally, any debugging information entry that has, or may have, a
>>>>>> DW_AT_name attribute, may also have a DW_AT_description attribute whose
>>>>>> value is a null-terminated string providing a description of the entity.
>>>>>>
>>>>>> It is expected that a debugger will display these descriptions as part
>>>>>> of displaying other properties of an entity.
>>>>>> ...
>>>>>>
>>>>>> AFAICT, gdb currently does not explicitly handle this attribute, which I
>>>>>> think means it's ignored.
>>>>>>
>>>>>> It seems applicable to use DW_AT_description at least for the artificial
>>>>>> decls.
>>>>>>
>>>>>> Perhaps even for all cases that are added by the patch?
>>>>>>
>>>>>
>>>>> I've chosen for this option. Using DW_AT_desciption instead of
>>>>> DW_AT_name should minimize difference in gdb behaviour with and without
>>>>> -gdescriptive-dies.
>>>>
>>>> -gdescribe-dies?
>>>>
>>>
>>> Done.
>>>
>>>>>> I'll rewrite the patch.
>>>>>
>>>>> OK for trunk?
>>>>
>>>> Few comments:
>>>>
>>>> +static void
>>>> +add_desc_attribute (dw_die_ref die, tree decl)
>>>> +{
>>>> +  tree decl_name;
>>>> +
>>>> +  if (!flag_descriptive_dies || dwarf_version < 3)
>>>> +    return;
>>>> +
>>>>
>>>> you said this is a DWARF5 "feature",
>>>
>>> No, it's a DWARF3 "feature". I copied the text from the DWARF5 spec.
>>>
>>>> I'd suggest changing the
>>>> check to
>>>>
>>>>   if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))
>>>>
>>>> given -gdescribe-dies is enough of a user request to enable the
>>>> feature.
>>>
>>> Done.
>>>
>>>> Not sure if we should warn about -gstrict-dwarf
>>>> combination with it and -gdwarf-N < 5.
>>>>
>>>
>>> Not sure either, left it out.
>>>
>>>> +  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) ==
>>>> CONST_DECL)
>>>> +    {
>>>> +      char buf[32];
>>>> +      char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
>>>> +      sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
>>>> +      add_desc_attribute (die, buf);
>>>> +    }
>>>>
>>>> I wondered before if we can use pretty-printing of decl here.  At
>>>> least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
>>>> certainly appear here I think.
>>>>
>>>
>>> Done.
>>>
>>>> +@item -gdescriptive-dies
>>>> +@opindex gdescriptive-dies
>>>> +Add description attribute to DWARF DIEs that have no name attribute.
>>>> +
>>>>
>>>> Either "description attributes" or "a description attribute", not
>>>> 100% sure being a non-native speaker.
>>>>
>>>
>>> Went for "description attributes".
>>>
>>>> Otherwise the patch looks OK to me but please leave Jason time
>>>> to comment.
>>>>
>>>
>>> Will do.
>>>
>>> Untested patch attached.
>>>
>
> Now bootstrapped and reg-tested on x86_64.
>
>>> [debug] Add -gdescribe-dies
>>>
>>> This patch adds option -gdescribe-dies.  It sets the DW_AT_description
>>> attribute of dies that do not get a DW_AT_name attribute, to make it easier to
>>> figure out what the die is describing.
>>>
>>> The option exports the names of artificial variables:
>>> ...
>>>  DIE    0: DW_TAG_variable (0x7fa934dd54b0)
>>> +  DW_AT_description: "D.1922"
>>>    DW_AT_type: die -> 0 (0x7fa934dd0d70)
>>>    DW_AT_artificial: 1
>>>
>>> ...
>>> which can be traced back to gimple dumps:
>>> ...
>>>   char a[0:D.1922] [value-expr: *a.0];
>>> ...
>>>
>>> Furthermore, it adds names to external references:
>>> ...
>>>  DIE    0: DW_TAG_subprogram (0x7fa88b9650f0)
>>> +DW_AT_description: "main"
>>>  DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
>>
>> Please add more of this description to the one-line documentation
>> patch you have now;
>
> Done.
>
>> there are many DIEs that have no name because they
>> don't need one, and this patch doesn't add names to all of them.
>>
>
> I've added the qualification "some" (to both documentation and command
> line option description).
>
>> These two cases seem like they have very different uses, but I suppose
>> we don't really need two new options.
>
> In both cases we add a description of the DIE. Could you explain what
> the difference in usage you refer to is?

I meant when you'd want them.  The artificial decl names are useful
for comparing debug output to optimizer dumps, while the external
reference names can be useful long after the compiler dumps are gone.

> Attached patch (which I will merge with the posted patch pre-commit)
> fixes up documentation and command line option description.
>
> OK?

OK.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-09-11 16:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-15 14:02 [RFC][debug] Add -greadable-dwarf Tom de Vries
2018-08-20 12:59 ` Richard Biener
2018-08-21  7:01   ` Jason Merrill
2018-08-21  7:16     ` Richard Biener
2018-08-21 12:40     ` Tom de Vries
2018-08-21 12:22   ` Tom de Vries
2018-08-22  6:56     ` [PATCH][debug] Add -gforce-named-dies Tom de Vries
2018-08-22  9:46       ` Tom de Vries
2018-08-22 13:23         ` [PATCH][debug] Add -gdescriptive-dies Tom de Vries
2018-08-24 10:44           ` Richard Biener
2018-08-24 15:38             ` [PATCH][debug] Add -gdescribe-dies Tom de Vries
2018-08-31 21:19               ` Jason Merrill
2018-09-01 18:10                 ` Tom de Vries
2018-09-11 15:32                   ` [PING][PATCH][debug] " Tom de Vries
2018-09-11 16:16                   ` [PATCH][debug] " Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).