[PING][PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

[Tom de Vries <tdevries@suse.de>]

On 19-08-19 12:48, Tom de Vries wrote:
> On 16-08-19 20:33, Pedro Alves wrote:
>> On 8/12/19 2:10 PM, Tom de Vries wrote:
>>>> Tom> +	CORE_ADDR baseaddr
>>>> Tom> +	  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>>>
>>>> I guess this assumes the text section - but then can the call to
>>>> find_pc_section give anything else?  Maybe it's just something to
>>>> comment and move on.
>>>>
>>> I suppose that find_pc_section also can return .init or .fini., but I
>>> imagine these wil have the same sections offsets as .text.
>>>
>> Hmm.  That'll usually be the case on GNU/Linux and other standard
>> operating systems, where you have a single text segment containing all sections.
>> But they might well not have the same offsets if you're debugging a relocatable
>> object, for example.  Some targets' shared libraries are relocatable objects
>> instead of fully linked binaries.  See "Library List Format" in the manual:
>>
>>  ~~~
>>  For the common case of libraries that are fully linked binaries, the
>>  library should have a list of segments.  If the target supports
>>  dynamic linking of a relocatable object file, its library XML element
>>  should instead include a list of allocated sections.  The segment or
>>  section bases are start addresses, not relocation offsets; they do not
>>  depend on the library's link-time base addresses.
>>  ~~~
>>
>> Linux kernel modules would be something like that too, I think.
>>
>> If easy, it seems better to look up the section.
> Done.
> 
> Updated patch OK for trunk?
> 

Ping.

Thanks,
- Tom

> 0001-gdb-Fix-gdb.dwarf2-amd64-entry-value-param.exp-with-fPIE-pie.patch
> 
> [gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie
> 
> When running gdb.dwarf2/amd64-entry-value-param.exp with target board
> unix/-fPIE/-pie, we get:
> ...
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
> ...
> 
> The problem is that read_call_site_scope attempts to put relocated addresses
> in cu->call_site_htab, for both the pc field:
> ...
>   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>   ...
>   pc = attr_value_as_address (attr) + baseaddr;
>   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
>   ...
>   call_site->pc = pc;
> ...
> and the target field:
> ...
>              lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>              SET_FIELD_PHYSADDR (call_site->target, lowpc);
> ...
> but fails to do so because it is called before objfile_relocate, making
> baseaddr 0.
> 
> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
> relocation offset at the use sites in call_site_for_pc and
> call_site_to_target_addr.
> 
> Tested on x86_64-linux, both with and without -fPIE/-pie.
> 
> gdb/ChangeLog:
> 
> 2019-08-09  Tom de Vries  <tdevries@suse.de>
> 
> 	PR gdb/24892
> 	* objfiles.c (find_objfile_section_for_unrelocated_addr)
> 	(relocate_pc_addr): New function.
> 	* objfiles.h (relocate_pc_addr): Declare.
> 	* block.c (call_site_for_pc): Substract relocation offset before
> 	finding pc in COMPUNIT_CALL_SITE_HTAB.
> 	* dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
> 	FIELD_STATIC_PHYSADDR (call_site->target).
> 	* dwarf2read.c (read_call_site_scope): Eliminate baseaddr.
> 
> ---
>  gdb/block.c      | 12 +++++++++++-
>  gdb/dwarf2loc.c  |  7 ++++++-
>  gdb/dwarf2read.c |  7 ++-----
>  gdb/objfiles.c   | 33 +++++++++++++++++++++++++++++++++
>  gdb/objfiles.h   |  5 +++++
>  5 files changed, 57 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/block.c b/gdb/block.c
> index 5c6faa8504..3113980259 100644
> --- a/gdb/block.c
> +++ b/gdb/block.c
> @@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
>    cust = find_pc_compunit_symtab (pc - 1);
>  
>    if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
> -    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
> +    {
> +      struct obj_section *sec;
> +      sec = find_pc_section (pc);
> +      if (sec != NULL)
> +	{
> +	  CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
> +	  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
> +				 &pc_unrelocated, NO_INSERT);
> +	}
> +    }
> +
>  
>    if (slot == NULL)
>      {
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 63643cb45d..c936630e0c 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
>        }
>  
>      case FIELD_LOC_KIND_PHYSADDR:
> -      return FIELD_STATIC_PHYSADDR (call_site->target);
> +      {
> +	CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
> +	struct objfile *objfile
> +	  = call_site->per_cu->dwarf2_per_objfile->objfile;
> +	return relocate_pc_addr (addr, objfile);
> +      }
>  
>      default:
>        internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index de9755f6ce..6a8218dc61 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>        return;
>      }
>    pc = attr_value_as_address (attr) + baseaddr;
> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
> +  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr;
>  
>    if (cu->call_site_htab == NULL)
>      cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
> @@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>  		         "low pc, for referencing DIE %s [in module %s]"),
>  		       sect_offset_str (die->sect_off), objfile_name (objfile));
>  	  else
> -	    {
> -	      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
> -	      SET_FIELD_PHYSADDR (call_site->target, lowpc);
> -	    }
> +	    SET_FIELD_PHYSADDR (call_site->target, lowpc);
>  	}
>      }
>    else
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index 7cbcbbd01b..5e1c49c658 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -1493,3 +1493,36 @@ objfile_flavour_name (struct objfile *objfile)
>      return bfd_flavour_name (bfd_get_flavour (objfile->obfd));
>    return NULL;
>  }
> +
> +/* Return section of OBJFILE that contains unrelocated address ADDR.  */
> +
> +static struct obj_section *
> +find_objfile_section_for_unrelocated_addr (struct objfile *objfile,
> +					   CORE_ADDR addr)
> +{
> +  struct obj_section *osect;
> +
> +  if (objfile == NULL)
> +    return NULL;
> +
> +  ALL_OBJFILE_OSECTIONS (objfile, osect)
> +    {
> +      if (obj_section_addr (osect) - obj_section_offset (osect) <= addr
> +	  && addr < obj_section_endaddr (osect) - obj_section_offset (osect))
> +	return osect;
> +    }
> +
> +  return NULL;
> +}
> +
> +/* See objfiles.h.  */
> +
> +CORE_ADDR
> +relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile)
> +{
> +  struct obj_section *sec
> +    = find_objfile_section_for_unrelocated_addr (objfile, addr);
> +  CORE_ADDR baseaddr = obj_section_offset (sec);
> +  struct gdbarch *gdbarch = get_objfile_arch (objfile);
> +  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
> +}
> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index 239aba2c2a..d4a9726b73 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -789,4 +789,9 @@ extern void objfile_register_static_link
>  extern const struct dynamic_prop *objfile_lookup_static_link
>    (struct objfile *objfile, const struct block *block);
>  
> +/* Given an unrelocated pc address ADDR belonging to a section of OBJFILE,
> +   return the relocated address.  */
> +
> +extern CORE_ADDR relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile);
> +
>  #endif /* !defined (OBJFILES_H) */
>