diff mbox

[2/7] gdb: Add an is_declaration field to each symbol

Message ID 02a60599e2dff9efead7adbb070733c0f4c65f04.1564243858.git.andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess July 27, 2019, 4:22 p.m. UTC 
  This commit is in preparation for a later commit, there should be no
user visible change after this commit.

Track if a symbol is a declaration or not.  At one point we could
possibly figure this out based on the LOC_UNRESOLVED address class of
the symbol, but, for Fortran we mark some symbols as LOC_UNRESOLVED
even when the DWARF supplies an address, this is because the address
supplied by the DWARF is actually wrong.  For details look in
dwarf2read.c and look for references to gFortran bug #40040.  I have
confirmed that current versions of gFortran still have the issue
mentioned in that bug report.

I considered two possible solutions to this problem, one was to add a
new address class, something like:
    LOC_UNRESOLVED_BUT_IS_STILL_A_DEFINITION
that I could use specifically for marking these "broken" Fortran
symbols.  My concern here is that every place LOC_UNRESOLVED is
mentioned would need to be possibly edited to also handle the new
address class, and any future change that mentions LOC_UNRESOLVED
would be a possible source of Fortran bugs due to not handling the new
address class correctly.

So, the solution I took was to add a new single bit flag to the symbol
to track if the symbol is a declaration or not.  There are already
some 1 bit flags in the symbol object, and on x86-64 GNU/Linux (at
least for me) adding one additional flag didn't increase the size of
the symbol object at all.

gdb/ChangeLog:

	* dwarf2read.c (new_symbol): Mark symbol as declaration when
	appropriate.
	* symtab.h (struct symbol): Add 'is_declaration' flag.
	(SYMBOL_IS_DECLARATION): Define.
---
 gdb/ChangeLog    | 7 +++++++
 gdb/dwarf2read.c | 3 +++
 gdb/symtab.h     | 4 ++++
 3 files changed, 14 insertions(+)

Comments

Tom Tromey July 29, 2019, 8:21 p.m. UTC | #1 
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Track if a symbol is a declaration or not.  At one point we could
Andrew> possibly figure this out based on the LOC_UNRESOLVED address class of
Andrew> the symbol, but, for Fortran we mark some symbols as LOC_UNRESOLVED
Andrew> even when the DWARF supplies an address, this is because the address
Andrew> supplied by the DWARF is actually wrong.  For details look in
Andrew> dwarf2read.c and look for references to gFortran bug #40040.  I have
Andrew> confirmed that current versions of gFortran still have the issue
Andrew> mentioned in that bug report.

That bug looks like another instance of the copy relocation problem.
So, I wonder if these workarounds could be removed if/when this series
lands:

    https://sourceware.org/ml/gdb-patches/2019-06/msg00612.html

(I still haven't looked at Pedro's review in detail, but I will get to
it sooner or later.)

Tom
Andrew Burgess July 30, 2019, 9 p.m. UTC | #2 
* Tom Tromey <tom@tromey.com> [2019-07-29 14:21:45 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> Track if a symbol is a declaration or not.  At one point we could
> Andrew> possibly figure this out based on the LOC_UNRESOLVED address class of
> Andrew> the symbol, but, for Fortran we mark some symbols as LOC_UNRESOLVED
> Andrew> even when the DWARF supplies an address, this is because the address
> Andrew> supplied by the DWARF is actually wrong.  For details look in
> Andrew> dwarf2read.c and look for references to gFortran bug #40040.  I have
> Andrew> confirmed that current versions of gFortran still have the issue
> Andrew> mentioned in that bug report.
> 
> That bug looks like another instance of the copy relocation problem.
> So, I wonder if these workarounds could be removed if/when this series
> lands:
> 
>     https://sourceware.org/ml/gdb-patches/2019-06/msg00612.html
> 
> (I still haven't looked at Pedro's review in detail, but I will get to
> it sooner or later.)

I checked your copy relocation series does indeed make parts #2 and #3
of my series redundant.

I'll address the other feedback with this series and rebase once your
series lands.

Thanks,
Andrew
diff mbox

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3d90d632891..01cab752888 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21503,6 +21503,9 @@  new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 				   dwarf2_full_name (name, die, cu),
 	                           NULL);
 
+      if (die_is_declaration (die, cu))
+	SYMBOL_IS_DECLARATION (sym) = 1;
+
       /* Default assumptions.
          Use the passed type or decode it from the die.  */
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 4f653bcdc1b..c4fd520e735 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1110,6 +1110,9 @@  struct symbol
   /* Whether this is an inlined function (class LOC_BLOCK only).  */
   unsigned is_inlined : 1;
 
+  /* Is this symbol a declaration?  */
+  unsigned is_declaration : 1;
+
   /* The concrete type of this symbol.  */
 
   ENUM_BITFIELD (symbol_subclass_kind) subclass : 2;
@@ -1171,6 +1174,7 @@  extern const struct symbol_impl *symbol_impls;
 #define SYMBOL_INLINED(symbol)		(symbol)->is_inlined
 #define SYMBOL_IS_CPLUS_TEMPLATE_FUNCTION(symbol) \
   (((symbol)->subclass) == SYMBOL_TEMPLATE)
+#define SYMBOL_IS_DECLARATION(symbol)	(symbol)->is_declaration
 #define SYMBOL_TYPE(symbol)		(symbol)->type
 #define SYMBOL_LINE(symbol)		(symbol)->line
 #define SYMBOL_COMPUTED_OPS(symbol)	(SYMBOL_IMPL (symbol).ops_computed)