This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[review] gdb: recognize new DWARF attributes: defaulted, deleted, calling conv.
- From: "Tankut Baris Aktemur (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>
- To: gdb-patches at sourceware dot org
- Cc: Simon Marchi <simon dot marchi at polymtl dot ca>, Tom Tromey <tromey at sourceware dot org>, Andrew Burgess <andrew dot burgess at embecosm dot com>
- Date: Thu, 31 Oct 2019 13:22:16 -0400
- Subject: [review] gdb: recognize new DWARF attributes: defaulted, deleted, calling conv.
- Auto-submitted: auto-generated
- References: <gerrit.1571406803000.I54192f363115b78ec7435a8563b73fcace420765@gnutoolchain-gerrit.osci.io>
- Reply-to: gnutoolchain-gerrit at osci dot 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