This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Support constants for DW_AT_data_member_location
- From: Jim Blandy <jimb at codesourcery dot com>
- To: Vladimir Prus <vladimir at codesourcery dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Thu, 13 Dec 2007 11:23:11 -0800
- Subject: Re: Support constants for DW_AT_data_member_location
- References: <200711291254.14423.vladimir@codesourcery.com> <200711292317.53967.vladimir@codesourcery.com> <m3mysvtcbq.fsf@codesourcery.com> <200712132005.17970.vladimir@codesourcery.com>
Sorry --- I committed, and then saw your review. I'll revise this.
Vladimir Prus <vladimir at codesourcery.com> writes:
> On Saturday 01 December 2007 04:48:09 Jim Blandy wrote:
>
>> > So, DW_FORM_data4 used for DW_AT_data_member_location is never interepreted
>> > as constant. Maybe we should have "is_surely_constant" function?
>>
>> Oh --- great catch, thanks. Â(We don't support location lists for
>> DW_AT_data_member_location yet, but we certainly should warn, not
>> randomly misinterpret the value.)
>>
>> How's this, then?
>>
>> gdb/ChangeLog:
>> 2007-11-30 ÂJim Blandy Â<jimb@codesourcery.com>
>>
>> ÂÂÂÂÂÂÂÂ* dwarf2read.c (attr_form_is_constant): New function.
>> ÂÂÂÂÂÂÂÂ(dwarf2_add_field): Use it and attr_form_is_section_offset to
>> ÂÂÂÂÂÂÂÂrecognize DW_AT_data_member_location attributes. ÂUse
>> ÂÂÂÂÂÂÂÂdwarf2_get_attr_constant_value when the attribute is a constant.
>>
>> ÂÂÂÂÂÂÂÂ* dwarf2read.c (attr_form_is_section_offset): New function.
>> ÂÂÂÂÂÂÂÂ(dwarf_add_member_fn, read_common_block, read_partial_die)
>> ÂÂÂÂÂÂÂÂ(dwarf2_symbol_mark_computed): Use it, instead of writing it out.
>>
>> diff -r c4f654de59cf gdb/dwarf2read.c
>> --- a/gdb/dwarf2read.cÂÂThu Nov 29 11:28:59 2007 -0800
>> +++ b/gdb/dwarf2read.cÂÂFri Nov 30 17:32:46 2007 -0800
>> @@ -1039,6 +1039,10 @@ static void dwarf_decode_macros (struct
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char *, bfd *, struct dwarf2_cu *);
>> Â
>> Âstatic int attr_form_is_block (struct attribute *);
>> +
>> +static int attr_form_is_section_offset (struct attribute *);
>> +
>> +static int attr_form_is_constant (struct attribute *);
>> Â
>> Âstatic void dwarf2_symbol_mark_computed (struct attribute *attr,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct symbol *sym,
>> @@ -3380,8 +3384,16 @@ dwarf2_add_field (struct field_info *fip
>> Â Â Â Âattr = dwarf2_attr (die, DW_AT_data_member_location, cu);
>> Â Â Â Âif (attr)
>> ÂÂÂÂÂÂÂÂ{
>> -ÂÂÂÂÂÂÂ ÂFIELD_BITPOS (*fp) =
>> -ÂÂÂÂÂÂÂ Â Âdecode_locdesc (DW_BLOCK (attr), cu) * bits_per_byte;
>> + Â Â Â Â Âif (attr_form_is_section_offset (attr))
>> + Â Â Â Â Â Â{
>> + Â Â Â Â Â Â Âdwarf2_complex_location_expr_complaint ();
>> + Â Â Â Â Â Â ÂFIELD_BITPOS (*fp) = 0;
>> + Â Â Â Â Â Â}
>> + Â Â Â Â Âelse if (attr_form_is_constant (attr))
>> + Â Â Â Â Â ÂFIELD_BITPOS (*fp) = dwarf2_get_attr_constant_value (attr, 0);
>
> You need "* 8" above, since the value of data_member_location is offset
> in bytes.
Fixed --- thanks.
>> + Â Â Â Â Âelse
>> + Â Â Â Â Â ÂFIELD_BITPOS (*fp) =
>> + Â Â Â Â Â Â Âdecode_locdesc (DW_BLOCK (attr), cu) * bits_per_byte;
>
> Or, if we really want GDB to support processors with 9-bit bytes, you
> need "* bits_per_byte".
That's what I used. I'm not sure what read_memory would return on
such processors, but... :)
>> +/* Return non-zero if ATTR's value is a section offset (classes
>> + Â lineptr, loclistptr, macptr or rangelistptr).
>
> I'd also note here that in all cases, an attribute value
> may fall to only one class listed above.
I've added a note citing 7.5.4, which says this (after the list of
classes).
>> In this case,
>> + Â you may use DW_UNSND (attr) to retrieve the offset. Â*/
>> +static int
>> +attr_form_is_section_offset (struct attribute *attr)
>> +{
>> + Âreturn (attr->form == DW_FORM_data4
>> + Â Â Â Â Â|| attr->form == DW_FORM_data8);
>
> I'd probably add a comment here saying that in DWARF standand,
> those are only two forms that can encode those classes. Not being
> DWARF expert, it took me a bit of looking to verify that. If you
> explicitly mention this, then future readers can just trust the
> comment ;-)
Hmm. If I look through the class definitions in 7.5.4, each paragraph
ends with "It is either form DW_FORM_data4 or form DW_FORM_data8." I
feel odd putting in a comment saying "Those classes use only these
forms", because if that weren't true, then the comment saying what the
function does wouldn't be accurate; it'd be missing cases.
> With the "* bits_per_byte" change, this patch fixes the problem I saw.
> Assuming there are no regressions, can you check it in?
Thanks for checking this. I did run the tests before I committed, but
I didn't notice any failures. I must be using a compiler that never
generates anything but expressions.