Re: [PATCH 1/5] Remove call to dwarf2_per_objfile::adjust from ranges readers

[Andrew Burgess <aburgess@redhat.com>]

Andrew Burgess <aburgess@redhat.com> writes:

> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Hi Tom,
>>
>> So, I don't know the details of the DWARF reader too well, so my attempt
>> to review this patch might be just wasting your time.  But I didn't
>> really understand what was going on here, so...
>>
>> Tom Tromey <tom@tromey.com> writes:
>>
>>> dwarf2_per_objfile::adjust applies gdbarch_adjust_dwarf2_addr to an
>>> address, leaving the result unrelocated.  However, this adjustment is
>>> only needed for text-section symbols -- it isn't needed for any sort
>>
>> I didn't know if the use of the word 'symbols' here was significant.
>> gdbarch_adjust_dwarf2_addr operates on addresses, and I guess symbol
>> could be a synonym for address in some contexts.  But the addresses here
>> do seem to be .text addresses, so clearly there's some important
>> distinction that I'm not understanding.
>>
>>> of address mapping.  Therefore, these calls can be removed from
>>
>> gdbarch_adjust_dwarf2_addr seems to be relevant for .text addresses,
>> which is what you're handling here, but you are creating a map, but it's
>> not clear _why_ those addresses wouldn't need to be updated.
>
> A follow on question.  Looking through gdb/dwarf/ there seem to be
> several other places where the addrmap_mutable::set_empty is called, and
> in at least some of those places the addresses are still being
> adjusted.  E.g.:
>
>   dwarf2_ranges_read
>   cooked_indexer::check_bounds
>   cooked_indexer::scan_attributes
>
> Why do these not need changing?

And an additional question.  Are lookups in these maps not done via the
two ::find_per_cu functions?  Which are passed, and are documented to
expected an un-adjusted (i.e. have text_section_offset removed) address.

If we don't add text_section_offset in to begin with doesn't that cause
problems?   Or maybe the bit I'm missing is that the two paths you've
changed already are adjusted?

Thanks,
Andrew


>
> Thanks,
> Andrew
>
>
>
>>
>> Sorry to ask what are probably obvious questions...
>>
>> Thanks,
>> Andrew
>>
>>> read_addrmap_from_aranges and create_addrmap_from_gdb_index.
>>> ---
>>>  gdb/dwarf2/aranges.c        | 2 --
>>>  gdb/dwarf2/read-gdb-index.c | 2 --
>>>  2 files changed, 4 deletions(-)
>>>
>>> diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
>>> index d577db62726..0d1dc11e27a 100644
>>> --- a/gdb/dwarf2/aranges.c
>>> +++ b/gdb/dwarf2/aranges.c
>>> @@ -190,8 +190,6 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
>>>  	      continue;
>>>  	    }
>>>  	  ULONGEST end = start + length;
>>> -	  start = (ULONGEST) per_objfile->adjust ((unrelocated_addr) start);
>>> -	  end = (ULONGEST) per_objfile->adjust ((unrelocated_addr) end);
>>>  	  mutable_map->set_empty (start, end - 1, per_cu);
>>>  	}
>>>  
>>> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
>>> index 8c0895b9639..cc6361674e8 100644
>>> --- a/gdb/dwarf2/read-gdb-index.c
>>> +++ b/gdb/dwarf2/read-gdb-index.c
>>> @@ -566,8 +566,6 @@ create_addrmap_from_gdb_index (dwarf2_per_objfile *per_objfile,
>>>  	  continue;
>>>  	}
>>>  
>>> -      lo = (ULONGEST) per_objfile->adjust ((unrelocated_addr) lo);
>>> -      hi = (ULONGEST) per_objfile->adjust ((unrelocated_addr) hi);
>>>        mutable_map.set_empty (lo, hi - 1, per_bfd->get_cu (cu_index));
>>>      }
>>>  
>>>
>>> -- 
>>> 2.43.0