[review,v3,gdb/symtab] Prefer var def over decl
Commit Message
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/29
......................................................................
Patch Set 3:
> Patch Set 3:
>
> Thank you for doing this.
>
> The patch itself seems perfectly fine to me. I appreciated the comment
> and the test case -- those help a lot.
>
> However, I wonder why gdb is even making a symbol for a declaration in the
> first place. For some other kinds of declarations, gdb doesn't bother, so
> I was wondering if it's possible to simply ignore them in general.
In general, there's a use of adding variable declarations to the symbol table. Say we have a declaration of a variable in one file, and the definition in another. If the file with the definition is build without debug info, but the file with the declaration with debug info, we can combine the debuginfo of the declaration and the minimal symbol info of the definition to print the value of the variable using the proper type.
But in the case of this PR, the size of the declaration is zero, and that currently means that this information composition doesn't work. That could be fixed by using the size of the symbol from the minimal symbol info (that's PR24989).
As for the question whether we can ignore the declaration, I've tried this approach (let's call it ignore.patch).
Ignore.patch:
...
...
It passes regression testing, and fixes the original test-case for this PR (PR24971).
Ignore.patch does not fix the test-case added in the current patch set, but that one would have to be adapted for this fix (this fix works when type size is zero, and that's not the case for that test-case).
Also, ignore.patch fixes the test-case of the spinoff PR24985.
OTOH, if we go with ignore.patch, it may have to be reverted again in order to trigger the (currently non-existing) fix for PR24989.
So AFAIU:
- if we fix this PR using ignore.patch and then revert it in order to fix PR24972, the test-case for this PR is handled by looking up the symbol size in the minimal symbol info
- if we fix this PR using the currently proposed patch set, and then fix PR24972, the test-case for this PR is still handled by finding the definition instead of the declaration.
@@ -21830,6 +21830,10 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
else if (attr2 && (DW_UNSND (attr2) != 0)
&& dwarf2_attr (die, DW_AT_type, cu) != NULL)
{
+ if (die->tag == DW_TAG_variable && die_is_declaration (die, cu)
+ && TYPE_LENGTH (SYMBOL_TYPE (sym)) == 0)
+ suppress_add = 1;
+
/* A variable with DW_AT_external is never static, but it
may be block-scoped. */
list_to_add