On 09-08-19 20:16, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
> Tom> relocation offset at the use sites in call_site_for_pc and
> Tom> call_site_to_target_addr.
> 
> I like this approach, since it represents some small progress on the
> objfile splitting project.
> 
> Tom> +    {
> Tom> +      struct obj_section *sec;
> Tom> +      sec = find_pc_section (pc);
> Tom> +      if (sec != NULL)
> Tom> +	{
> Tom> +	  struct objfile *objfile = sec->objfile;
> Tom> +	  CORE_ADDR baseaddr
> Tom> +	    = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
> 
> Why SECT_OFF_TEXT rather than the section "sec"?
> 

Fixed by using obj_section_offset:
...
        CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
...

> Tom> +	  CORE_ADDR pc_unrelocated
> Tom> +	    = gdbarch_adjust_dwarf2_addr (gdbarch, pc - baseaddr);
> 
> I am not sure gdbarch_adjust_dwarf2_addr can be used "bidirectionally"
> like this.  It is probably fine in practice but I wonder about the
> documented contract.
> 

Thanks for pointing that out. Hmm, it seems there is no documented
contract (as you mentioned here:
https://sourceware.org/ml/gdb-patches/2018-05/msg00074.html ).

But, I guess we assume relocated addresses as arg to
gdbarch_adjust_dwarf2_addr because mips_adjust_dwarf2_addr uses
mips_pc_is_mips which uses lookup_minimal_symbol_by_pc which uses
lookup_minimal_symbol_by_pc_section which uses frob_address to
de-relocate the used address before comparing it to the unrelocated
address in MSYMBOL_VALUE_RAW_ADDRESS.

> 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.

> Tom> -  pc = attr_value_as_address (attr) + baseaddr;
> Tom> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
> Tom> +  pc = attr_value_as_address (attr);
> 
> The approach taken elsewhere in dwarf2read.c is to bias, adjust, then
> unbias:
> 
>       CORE_ADDR low
> 	= (gdbarch_adjust_dwarf2_addr (gdbarch, best_lowpc + baseaddr)
> 	   - baseaddr);
> 
> Maybe this would be better here as well, or at least consistent.

Done. Though as pointed out in the rationale, baseaddr can be 0 here, so
effectively we pass at least sometimes unrelocated addresses into
gdbarch_adjust_dwarf2_addr. I'm not sure if frob_address is guaranteed
to be the identity function for unrelocated addresses.

Also, I've copied relocate_text_addr from
https://sourceware.org/ml/gdb-patches/2019-08/msg00241.html to use it in
call_site_to_target_addr.

OK like this?

Thanks,
- Tom