Patchwork [RFA,v3,3/6] Introduce partial_symbol::address

login
register
mail settings
Submitter Tom Tromey
Date June 7, 2018, 4:19 p.m.
Message ID <20180607161955.9800-4-tom@tromey.com>
Download mbox | patch
Permalink /patch/27692/
State New
Headers show

Comments

Tom Tromey - June 7, 2018, 4:19 p.m.
This introduces a partial_symbol::address method.  This method takes
an objfile argument.  This is necessary so that we can later relocate
a partial symbol at its point of use.  It also adds a "raw" form of
the accessor, to compute the unrelocated value; and a method to be
used for setting the field.

Note that the new method doesn't actually perform any relocation yet.
That will come in a subsequent patch.  However, the comments are
written to reflect the intended, rather than the temporary, semantics.

gdb/ChangeLog
2018-06-07  Tom Tromey  <tromey@redhat.com>

	* psympriv.h (struct partial_symbol) <raw_address, address,
	set_address>: New methods.
	* psymtab.c (find_pc_sect_psymtab_closer, find_pc_sect_psymbol)
	(fixup_psymbol_section, relocate_psymtabs): Update.
	(print_partial_symbols): Add 'objfile' parameter.  Update.
	(dump_psymtab, add_psymbol_to_bcache, psym_fill_psymbol_map):
	Update.
---
 gdb/ChangeLog  | 10 ++++++++++
 gdb/psympriv.h | 20 ++++++++++++++++++++
 gdb/psymtab.c  | 43 +++++++++++++++++++++++--------------------
 3 files changed, 53 insertions(+), 20 deletions(-)
Simon Marchi - July 17, 2018, 4:04 p.m.
Hi Tom,

I have some naming suggestions below, but the patch LGTM either way.

> diff --git a/gdb/psympriv.h b/gdb/psympriv.h
> index f3cb0a6ba15..ec2d9923c1b 100644
> --- a/gdb/psympriv.h
> +++ b/gdb/psympriv.h
> @@ -44,6 +44,26 @@ struct partial_symbol : public general_symbol_info
>      return nullptr;
>    }
>  
> +  /* Return the unrelocated address of this partial symbol.  */
> +  CORE_ADDR raw_address () const
> +  {
> +    return value.address;
> +  }

I think it would be clearer to name this "unrelocated_address".

> +  /* Return the address of this partial symbol, relocated according to
> +     the offsets provided in OBJFILE.  */
> +  CORE_ADDR address (struct objfile *objfile) const
> +  {
> +    return value.address;
> +  }
> +
> +  /* Set the address of this partial symbol.  The address must be
> +     unrelocated.  */
> +  void set_address (CORE_ADDR addr)
> +  {
> +    value.address = addr;
> +  }

Since this is the mirror of raw_address (or unrelocated_address), it might good to
name it set_raw_address (or set_unrelocated_address).  Bugs in the calling code
would be more apparent.  If you're playing with a relocated address and call
set_unrelocated_address, it's quite obvious something is wrong.

Except it's not actually true as of this patch (we still pass a relocated address),
so maybe it can be renamed later, when it is the case.

Simon
Tom Tromey - July 18, 2018, 5:47 p.m.
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> I think it would be clearer to name this "unrelocated_address".

I made this change.

>> +  /* Set the address of this partial symbol.  The address must be
>> +     unrelocated.  */
>> +  void set_address (CORE_ADDR addr)
>> +  {
>> +    value.address = addr;
>> +  }

Simon> Since this is the mirror of raw_address (or unrelocated_address), it might good to
Simon> name it set_raw_address (or set_unrelocated_address).  Bugs in the calling code
Simon> would be more apparent.  If you're playing with a relocated address and call
Simon> set_unrelocated_address, it's quite obvious something is wrong.

Simon> Except it's not actually true as of this patch (we still pass a relocated address),
Simon> so maybe it can be renamed later, when it is the case.

I've changed the final patch to rename it to set_unrelocated_address.

Tom

Patch

diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index f3cb0a6ba15..ec2d9923c1b 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -44,6 +44,26 @@  struct partial_symbol : public general_symbol_info
     return nullptr;
   }
 
+  /* Return the unrelocated address of this partial symbol.  */
+  CORE_ADDR raw_address () const
+  {
+    return value.address;
+  }
+
+  /* Return the address of this partial symbol, relocated according to
+     the offsets provided in OBJFILE.  */
+  CORE_ADDR address (struct objfile *objfile) const
+  {
+    return value.address;
+  }
+
+  /* Set the address of this partial symbol.  The address must be
+     unrelocated.  */
+  void set_address (CORE_ADDR addr)
+  {
+    value.address = addr;
+  }
+
   /* Name space code.  */
 
   ENUM_BITFIELD(domain_enum_tag) domain : SYMBOL_DOMAIN_BITS;
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 9a06d68f91e..58df3c69a78 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -267,7 +267,7 @@  find_pc_sect_psymtab_closer (struct objfile *objfile,
 	     object's symbol table.  */
 	  p = find_pc_sect_psymbol (objfile, tpst, pc, section);
 	  if (p != NULL
-	      && (p->value.address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
+	      && (p->address (objfile) == BMSYMBOL_VALUE_ADDRESS (msymbol)))
 	    return tpst;
 
 	  /* Also accept the textlow value of a psymtab as a
@@ -275,7 +275,7 @@  find_pc_sect_psymtab_closer (struct objfile *objfile,
 	     symbol tables with line information but no debug
 	     symbols (e.g. those produced by an assembler).  */
 	  if (p != NULL)
-	    this_addr = p->value.address;
+	    this_addr = p->address (objfile);
 	  else
 	    this_addr = tpst->textlow;
 
@@ -333,7 +333,8 @@  find_pc_sect_psymtab (struct objfile *objfile, CORE_ADDR pc,
 		 object's symbol table.  */
 	      p = find_pc_sect_psymbol (objfile, pst, pc, section);
 	      if (p == NULL
-		  || (p->value.address != BMSYMBOL_VALUE_ADDRESS (msymbol)))
+		  || (p->address (objfile)
+		      != BMSYMBOL_VALUE_ADDRESS (msymbol)))
 		goto next;
 	    }
 
@@ -425,10 +426,10 @@  find_pc_sect_psymbol (struct objfile *objfile,
 
       if (p->domain == VAR_DOMAIN
 	  && p->aclass == LOC_BLOCK
-	  && pc >= p->value.address
-	  && (p->value.address > best_pc
+	  && pc >= p->address (objfile)
+	  && (p->address (objfile) > best_pc
 	      || (psymtab->textlow == 0
-		  && best_pc == 0 && p->value.address == 0)))
+		  && best_pc == 0 && p->address (objfile) == 0)))
 	{
 	  if (section != NULL)  /* Match on a specific section.  */
 	    {
@@ -437,7 +438,7 @@  find_pc_sect_psymbol (struct objfile *objfile,
 					  section))
 		continue;
 	    }
-	  best_pc = p->value.address;
+	  best_pc = p->address (objfile);
 	  best = p;
 	}
     }
@@ -448,10 +449,10 @@  find_pc_sect_psymbol (struct objfile *objfile,
 
       if (p->domain == VAR_DOMAIN
 	  && p->aclass == LOC_BLOCK
-	  && pc >= p->value.address
-	  && (p->value.address > best_pc
+	  && pc >= p->address (objfile)
+	  && (p->address (objfile) > best_pc
 	      || (psymtab->textlow == 0
-		  && best_pc == 0 && p->value.address == 0)))
+		  && best_pc == 0 && p->address (objfile) == 0)))
 	{
 	  if (section != NULL)  /* Match on a specific section.  */
 	    {
@@ -460,7 +461,7 @@  find_pc_sect_psymbol (struct objfile *objfile,
 					  section))
 		continue;
 	    }
-	  best_pc = p->value.address;
+	  best_pc = p->address (objfile);
 	  best = p;
 	}
     }
@@ -486,7 +487,7 @@  fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
     case LOC_STATIC:
     case LOC_LABEL:
     case LOC_BLOCK:
-      addr = psym->value.address;
+      addr = psym->address (objfile);
       break;
     default:
       /* Nothing else will be listed in the minsyms -- no use looking
@@ -813,13 +814,15 @@  psym_relocate (struct objfile *objfile,
     {
       fixup_psymbol_section (psym, objfile);
       if (psym->section >= 0)
-	psym->value.address += ANOFFSET (delta, psym->section);
+	psym->set_address (psym->raw_address ()
+			   + ANOFFSET (delta, psym->section));
     }
   for (partial_symbol *psym : objfile->static_psymbols)
     {
       fixup_psymbol_section (psym, objfile);
       if (psym->section >= 0)
-	psym->value.address += ANOFFSET (delta, psym->section);
+	psym->set_address (psym->raw_address ()
+			   + ANOFFSET (delta, psym->section));
     }
 
   objfile->psymbol_map.clear ();
@@ -883,7 +886,7 @@  psym_forget_cached_source_info (struct objfile *objfile)
 }
 
 static void
-print_partial_symbols (struct gdbarch *gdbarch,
+print_partial_symbols (struct gdbarch *gdbarch, struct objfile *objfile,
 		       struct partial_symbol **p, int count, const char *what,
 		       struct ui_file *outfile)
 {
@@ -967,7 +970,7 @@  print_partial_symbols (struct gdbarch *gdbarch,
 	  break;
 	}
       fputs_filtered (", ", outfile);
-      fputs_filtered (paddress (gdbarch, (*p)->value.address), outfile);
+      fputs_filtered (paddress (gdbarch, (*p)->raw_address ()), outfile);
       fprintf_filtered (outfile, "\n");
       p++;
     }
@@ -1032,13 +1035,13 @@  dump_psymtab (struct objfile *objfile, struct partial_symtab *psymtab,
     }
   if (psymtab->n_global_syms > 0)
     {
-      print_partial_symbols (gdbarch,
+      print_partial_symbols (gdbarch, objfile,
 			     &objfile->global_psymbols[psymtab->globals_offset],
 			     psymtab->n_global_syms, "Global", outfile);
     }
   if (psymtab->n_static_syms > 0)
     {
-      print_partial_symbols (gdbarch,
+      print_partial_symbols (gdbarch, objfile,
 			     &objfile->static_psymbols[psymtab->statics_offset],
 			     psymtab->n_static_syms, "Static", outfile);
     }
@@ -1475,7 +1478,7 @@  psym_fill_psymbol_map (struct objfile *objfile,
 
       if (psym->aclass == LOC_STATIC)
 	{
-	  CORE_ADDR addr = psym->value.address;
+	  CORE_ADDR addr = psym->raw_address ();
 	  if (seen_addrs->find (addr) == seen_addrs->end ())
 	    {
 	      seen_addrs->insert (addr);
@@ -1718,7 +1721,7 @@  add_psymbol_to_bcache (const char *name, int namelength, int copy_name,
 {
   struct partial_symbol psymbol;
 
-  psymbol.value.address = coreaddr;
+  psymbol.set_address (coreaddr);
   psymbol.section = -1;
   psymbol.domain = domain;
   psymbol.aclass = theclass;