Re: [PATCH 01/12] Handle another edge case for TLS variable lookups.

[John Baldwin <jhb@FreeBSD.org>]

On 3/23/22 4:55 PM, John Baldwin wrote:
> On 3/23/22 2:00 PM, John Baldwin wrote:
>> This is similar to the change in
>> df22c1e5d53c38f38bce6072bb46de240f9e0e2b but applies to the main
>> object file.  The original change I encountered when testing TLS on
>> RISC-V for which I was unable to test TLS variables in the main
>> executable due to issues with compiler-generated debug info, so I only
>> checked for a backlink before walking the list of shared library
>> object files.
>>
>> However, I ran into this issue again (of a separate debug object file
>> being passed to svr4_fetch_objfile_link_map) when testing TLS
>> variables on 32-bit arm.  To fix, move the check for a backlink
>> earlier before the check for the main object file.
> 
> I now see this when using a separate debug file on FreeBSD/amd64 as
> well FWIW.  This definitely used to work but now needs this fix for
> me.  Curiously, 'info address' shows the variable belonging to the
> separate debug file, e.g.:
> 
> % gdb tls tls.core
> ...
> (gdb) info address id
> Symbol "id" is a thread-local variable at offset 0x0 in the thread-local storage for `/usr/home/john/work/johnsvn/test/tls/tls.debug'.
> 
> Arguably this is a recent bug somewhere in the DWARF parsing that
> is longer following the backlink when choosing the object file
> for a TLS variable?  I think fixing svr4_fetch_objfile_link_map to
> be resilient against this is still ok, but it might be worth
> tracking down the other bug as well.  I'll see if I can find it.

So the change in behavior was introduced by this commit:

commit 9f47c7071654d8cee82ff91ec1e65d57bd78e77f
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Wed May 27 11:14:04 2020 -0400

     Remove dwarf2_per_cu_data::objfile ()
     
     Since dwarf2_per_cu_data objects are going to become
     objfile-independent, the backlink from dwarf2_per_cu_data to one
     particular objfile must be removed.  Instead, users of
     dwarf2_per_cu_data that need an objfile must know from somewhere else in
     the context of which objfile they are using this CU.
     
     This also helps remove a dwarf2_per_cu_data::dwarf2_per_objfile
     reference (from where the objfile was obtained).
     
     Note that the dwarf2_per_cu_data::objfile method has a special case to
     make sure to return the main objfile, if the objfile associated to the
     dwarf2_per_cu_data is a separate debug objfile.  I don't really know if
     this is necessary: I ignored that, and didn't see any regression when
     testing with the various Dejagnu boards with separate debug info, so I
     presume it wasn't needed.  If it turns out this was needed, then we can
     have a helper method on the objfile type for that.
     
     gdb/ChangeLog:
     
             * dwarf2/read.h (struct dwarf2_per_cu_data) <objfile>: Remove.
             * dwarf2/read.c (dwarf2_compute_name): Pass per_objfile down.
             (read_call_site_scope): Assign per_objfile.
             (dwarf2_per_cu_data::objfile): Remove.
             * gdbtypes.h (struct call_site) <per_objfile>: New member.
             * dwarf2/loc.h (dwarf2_evaluate_loc_desc): Add
             dwarf2_per_objfile parameter.
             * dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Add
             dwarf2_per_objfile parameter.
             (dwarf_expr_reg_to_entry_parameter): Add output
             dwarf2_per_objfile parameter.
             (locexpr_get_frame_base): Update.
             (class dwarf_evaluate_loc_desc) <get_tls_address>: Update.
             <push_dwarf_reg_entry_value>: Update.
             <call_site_to_target_addr>: Update.
             (dwarf_entry_parameter_to_value): Add dwarf2_per_objfile
             parameter.
             (value_of_dwarf_reg_entry): Update.
             (rw_pieced_value): Update.
             (indirect_synthetic_pointer): Update.
             (dwarf2_evaluate_property): Update.
             (dwarf2_loc_desc_get_symbol_read_needs): Add dwarf2_per_objfile
             parameter.
             (locexpr_read_variable): Update.
             (locexpr_get_symbol_read_needs): Update.
             (loclist_read_variable): Update.
     
In particular, this appears to be fallout from removing the special case
to return the main objfile described in the last paragraph (and this means
it is also likely a regression in GDB 12?).  I did find that 'info address'
also prints the name of the separate debug file in GDB 11, so that behavior
is not new.

-- 
John Baldwin