Fix handling of DW_AT_artificial on fields
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
I noticed a few odd things about the handling of DW_AT_artificial on
fields:
* First, the code is checking the result of dwarf2_attr, but this only
checks for the presence of the attribute, not the value. This patch
changes the code to call dwarf2_flag_true_p instead.
* Second, the code also uses this to set the accessibility of the
field. This seems plainly wrong.
* Third, this attribute is only examined in one branch of
dwarf2_add_field; but I think it should be done in all branches.
---
gdb/dwarf2/read.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
base-commit: 7e0417f49792ea5865faa47735be638989fe5229
Comments
Hi,
On 3/24/26 9:04 AM, Tom Tromey wrote:
> I noticed a few odd things about the handling of DW_AT_artificial on
> fields:
>
> * First, the code is checking the result of dwarf2_attr, but this only
> checks for the presence of the attribute, not the value. This patch
> changes the code to call dwarf2_flag_true_p instead.
>
> * Second, the code also uses this to set the accessibility of the
> field. This seems plainly wrong.
>
> * Third, this attribute is only examined in one branch of
> dwarf2_add_field; but I think it should be done in all branches.
I agree. I just have one very minor observation, but this change LGTM.
Reviewed-By: Keith Seitz <keiths@redhat.com>
Keith
> ---
> gdb/dwarf2/read.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 8b87d58dd9c..f8e7972f223 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -9519,6 +9519,9 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
>
> fp = &new_field->field;
>
> + if (dwarf2_flag_true_p (die, DW_AT_artificial, cu))
> + fp->set_is_artificial (true);
> +
> if ((die->tag == DW_TAG_member || die->tag == DW_TAG_namelist_item)
> && !die_is_declaration (die, cu))
> {
In the DW_TAG_namelist_item part of this branch, the code currently
calls follow_die_ref, recording DW_AT_artificial of the followed DIE.
With this change, we will use the setting of the original DIE.
I am not familiar enough with Fortran to know if this could make a
difference (I suspect it is entirely hypothetical), but it seems worth
mentioning.
@@ -9519,6 +9519,9 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
fp = &new_field->field;
+ if (dwarf2_flag_true_p (die, DW_AT_artificial, cu))
+ fp->set_is_artificial (true);
+
if ((die->tag == DW_TAG_member || die->tag == DW_TAG_namelist_item)
&& !die_is_declaration (die, cu))
{
@@ -9546,14 +9549,6 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
/* The name is already allocated along with this objfile, so we don't
need to duplicate it for the type. */
fp->set_name (fieldname);
-
- /* Change accessibility for artificial fields (e.g. virtual table
- pointer or virtual base class pointer) to private. */
- if (dwarf2_attr (die, DW_AT_artificial, cu))
- {
- fp->set_is_artificial (true);
- fp->set_accessibility (accessibility::PRIVATE);
- }
}
else if (die->tag == DW_TAG_member || die->tag == DW_TAG_variable)
{