On Fri, Aug 24, 2018 at 11:38 AM, Tom de Vries <tdevr...@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
