[review] gdb: recognize new DWARF attributes: defaulted, deleted, calling conv.

["Tankut Baris Aktemur (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135
......................................................................


Patch Set 1:

(5 comments)

> I assume this is used elsewhere in your series.  Now that we're on gerrit and can't
> do cover letters, I think it would be helpful if supporting patches like this could
> mention facts like that... something for us all to consider as we write commit
> messages.

Yes.  I update the commit message in v2.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/dwarf2read.c 
File gdb/dwarf2read.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/dwarf2read.c@15533 
PS1, Line 15533: 
15422 | dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
      | ...
15528 |   if (attr && DW_UNSND (attr) != 0)
15529 |     fnp->is_artificial = 1;
15530 | 
15531 |   /* Check for defaulted methods.  */
15532 |   attr = dwarf2_attr (die, DW_AT_defaulted, cu);
15533 >   if (attr)
15534 |     fnp->defaulted = DW_UNSND (attr);
15535 | 
15536 |   /* Check for deleted methods.  */
15537 |   attr = dwarf2_attr (die, DW_AT_deleted, cu);
15538 |   if (attr && DW_UNSND (attr) != 0)

> Use: […]

Done


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/dwarf2read.c@15534 
PS1, Line 15534: 
15422 | dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
      | ...
15529 |     fnp->is_artificial = 1;
15530 | 
15531 |   /* Check for defaulted methods.  */
15532 |   attr = dwarf2_attr (die, DW_AT_defaulted, cu);
15533 |   if (attr)
15534 >     fnp->defaulted = DW_UNSND (attr);
15535 | 
15536 |   /* Check for deleted methods.  */
15537 |   attr = dwarf2_attr (die, DW_AT_deleted, cu);
15538 |   if (attr && DW_UNSND (attr) != 0)
15539 |     fnp->is_deleted = 1;

> Shouldn't we do a bit of validation here, making sure `attr` is one of the valid values, issuing a c […]

Done


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/gdbtypes.h 
File gdb/gdbtypes.h:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/gdbtypes.h@929 
PS1, Line 929: 
899 | struct fn_field
    | ...
924 |   struct type *fcontext;
925 | 
926 |   /* * DW_AT_defaulted attribute for this function; the value is one
927 |      of the DW_DEFAULTED enum dwarf_defaulted_attribute constants.  */
928 | 
929 >   unsigned int defaulted;
930 | 
931 |   /* Attributes.  */
932 | 
933 |   unsigned int is_const:1;
934 |   unsigned int is_volatile:1;

> Should this be a bit-field or a narrower integer type? […]

In v2, I make this 2-bit-wide and move down for better packing.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/gdbtypes.h@929 
PS1, Line 929: 
899 | struct fn_field
    | ...
924 |   struct type *fcontext;
925 | 
926 |   /* * DW_AT_defaulted attribute for this function; the value is one
927 |      of the DW_DEFAULTED enum dwarf_defaulted_attribute constants.  */
928 | 
929 >   unsigned int defaulted;
930 | 
931 |   /* Attributes.  */
932 | 
933 |   unsigned int is_const:1;
934 |   unsigned int is_volatile:1;

> It would be good for this field to be an enum. […]

In v2, I make this an ENUM_BITFIELD.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/135/1/gdb/gdbtypes.h@1101 
PS1, Line 1101: 
 986 | struct cplus_struct_type
     | ...
1096 | 
1097 |     /* * The calling convention for this type, fetched from the
1098 |        DW_AT_calling_convention attribute.  The value is one of the
1099 |        DW_CC enum dwarf_calling_convention constants.  */
1100 | 
1101 >     unsigned calling_convention : 8;
1102 |   };
1103 | 
1104 | /* * Struct used to store conversion rankings.  */
1105 | 
1106 | struct rank

> Ah, thanks for pointing out this other usage in func_type. […]

Yes, agree. In v2, I make this an ENUM_BITFIELD and move near `is_dynamic`.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I54192f363115b78ec7435a8563b73fcace420765
Gerrit-Change-Number: 135
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Thu, 31 Oct 2019 17:22:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Comment-In-Reply-To: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment