[PATCH][gdb] Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie

Pedro Alves palves@redhat.com
Fri Aug 16 18:34:00 GMT 2019


On 8/10/19 6:44 AM, Tom de Vries wrote:
> On 09-08-19 19:38, Pedro Alves wrote:
>> On 8/9/19 4:03 PM, Tom de Vries wrote:
>>
>>>  
>>> +/* Given an unrelocated address ADDR belonging to the text section of OBJFILE,
>>> +   return the relocated address.  */
>>> +
>>> +static CORE_ADDR
>>> +relocate_text_addr (CORE_ADDR addr, struct objfile *objfile)
>>> +{
>>> +  CORE_ADDR baseaddr
>>> +    = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>> +  struct gdbarch *gdbarch = get_objfile_arch (objfile);
>>> +  addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>>> +  return addr;
>>
>> I'd write:
>>
>>  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>>
> 
> Updated accordingly.
> 
>>> +}
>>> +
>>>  /* Find PC to be unwound from THIS_FRAME.  THIS_FRAME must be a part of
>>>     CACHE.  */
>>>  
>>> @@ -240,14 +255,25 @@ pretend_pc (struct frame_info *this_frame, struct tailcall_cache *cache)
>>>    gdb_assert (next_levels >= 0);
>>>  
>>>    if (next_levels < chain->callees)
>>> -    return chain->call_site[chain->length - next_levels - 1]->pc;
>>> +    {
>>> +      struct call_site *call_site
>>> +	= chain->call_site[chain->length - next_levels - 1];
>>> +      struct objfile *objfile = call_site->per_cu->dwarf2_per_objfile->objfile;
>>> +      return relocate_text_addr (call_site->pc, objfile);
>>> +    }
>>>    next_levels -= chain->callees;
>>>  
>>>    /* Otherwise CHAIN->CALLEES are already covered by CHAIN->CALLERS.  */
>>>    if (chain->callees != chain->length)
>>>      {
>>>        if (next_levels < chain->callers)
>>> -	return chain->call_site[chain->callers - next_levels - 1]->pc;
>>> +	{
>>> +	  struct call_site *call_site
>>> +	    = chain->call_site[chain->callers - next_levels - 1];
>>> +	  struct objfile *objfile
>>> +	    = call_site->per_cu->dwarf2_per_objfile->objfile;
>>> +	  return relocate_text_addr (call_site->pc, objfile);
>>> +	}
>>
>> That seems fine, but it seems you could have factored out more, like:
>>
>> static CORE_ADDR
>> call_site_relocated_pc (struct call_site *call_site)
>> {
>>   struct objfile *objfile
>>     = call_site->per_cu->dwarf2_per_objfile->objfile;
>>   CORE_ADDR baseaddr
>>     = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>   struct gdbarch *gdbarch = get_objfile_arch (objfile);
>>   return gdbarch_adjust_dwarf2_addr (gdbarch, call_size->pc + baseaddr);
>> }
>>
>> Then the other hunks would look like:
>>
>>   struct call_site *call_site
>>     = chain->call_site[chain->length - next_levels - 1];
>>   return call_site_relocated_pc (call_site);
>>
>> ...
>>
>>   struct call_site *call_site
>>     = chain->call_site[chain->callers - next_levels - 1];
>>   return call_site_relocated_pc (call_site);
>>
>> call_site_relocated_pc could even be a method of struct call_site, I guess,
>> so you'd write:
>>
>>   return call_site->relocated_pc ();
>>
>> Any reason you didn't do something like that?
> 
> I went for a utility function that can be maximally reused, as opposed
> to a function that maximally factors out the code in this particular
> patch. But we can do both, so this patch adds:
> - relocate_text_addr (I've moved it to objfiles.h/objfiles.c)
> - call_site::relocated_pc
> 
> [ BTW, I should mention that this fix is dependent on the patch "[gdb]
> Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie" (
> https://sourceware.org/ml/gdb-patches/2019-08/msg00215.html ). ]
> 
> OK like this?

Thanks.  It looks much better to me this way, but I think we
should resolve the SECT_OFF_TEXT discussion in the other patch
before moving forward.

Thanks,
Pedro Alves



More information about the Gdb-patches mailing list