diff mbox series

[RFC,debug] Add -greadable-dwarf

Message ID 20180815140258.GA1999@delia
State New
Headers show
Series [RFC,debug] Add -greadable-dwarf | expand

Commit Message

Tom de Vries Aug. 15, 2018, 2:03 p.m. UTC
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(-)

Comments

Richard Biener Aug. 20, 2018, 12:59 p.m. UTC | #1
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
> 
>
Jason Merrill Aug. 21, 2018, 4:53 a.m. UTC | #2
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)
>
Richard Biener Aug. 21, 2018, 7:16 a.m. UTC | #3
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)
> >
>
Tom de Vries Aug. 21, 2018, 12:22 p.m. UTC | #4
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
[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)
Tom de Vries Aug. 21, 2018, 12:40 p.m. UTC | #5
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)
>
diff mbox series

Patch

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