Re: [PATCH][gdb/symtab] Store external var decls in psymtab

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

On 08-04-2020 17:07, Tom de Vries wrote:
> Hi,
> 
> Consider a test-case consisting of source file test.c:
> ...
> extern int aaa;
> int
> main (void)
> {
>   return 0;
> }
> ...
> and test-2.c:
> ...
> int aaa = 33;
> ...
> compiled with debug info only for test.c:
> ...
> $ gcc -c test.c -g; gcc -c test2.c; gcc test.o test2.o -g
> ...
> 
> When trying to print aaa, we get:
> ...
> $ gdb -batch a.out -ex "print aaa"
> 'aaa' has unknown type; cast it to its declared type
> ...
> but with -readnow we have:
> ...
> $ gdb -readnow -batch a.out -ex "print aaa"
> $1 = 33
> ...
> 
> In the -readnow case, the symbol for aaa in the full symtab has
> LOC_UNRESOLVED, and the symbol type is combined with the minimal symbol
> address, to read the value and print it without cast.
> 
> Without the -readnow, we create partial symbols, but the aaa decl is missing
> from the partial symtabs, so we find it only in the minimal symbols, resulting
> in the cast request.  If the aaa decl would have been in the partial symtabs,
> it would have been found, and the full symtab would have been expanded, after
> which things would be as with -readnow.
> 
> The function add_partial_symbol has a comment on the LOC_UNRESOLVED +
> minimal symbol addres construct at DW_TAG_variable handling:
> ...
>       else if (pdi->is_external)
> 	{
> 	  /* Global Variable.
> 	     Don't enter into the minimal symbol tables as there is
> 	     a minimal symbol table entry from the ELF symbols already.
> 	     Enter into partial symbol table if it has a location
> 	     descriptor or a type.
> 	     If the location descriptor is missing, new_symbol will create
> 	     a LOC_UNRESOLVED symbol, the address of the variable will then
> 	     be determined from the minimal symbol table whenever the variable
> 	     is referenced.
> ...
> but it's not triggered due to this test in scan_partial_symbols:
> ...
>             case DW_TAG_variable:
> 	    ...
>               if (!pdi->is_declaration)
>                 {
>                   add_partial_symbol (pdi, cu);
>                 }
> ...
> 
> Fix this in scan_partial_symbols by allowing external variable decls to be
> added to the partial symtabs.
> 
> Build and reg-tested on x86_64-linux.
> 
> The patch caused this regression:
> ...
> (gdb) print a_thread_local^M
> Cannot find thread-local storage for process 0, executable file tls/tls:^M
> Cannot find thread-local variables on this target^M
> (gdb) FAIL: gdb.threads/tls.exp: print a_thread_local
> ...
> while without the patch we have:
> ...
> (gdb) print a_thread_local^M
> Cannot read `a_thread_local' without registers^M
> (gdb) PASS: gdb.threads/tls.exp: print a_thread_local
> ...
> 
> However, without the patch but with -readnow we have the same FAIL as with the
> patch.  In other words, the patch has the effect that we get the same result
> with and without -readnow.
> 
> This can be explained as follows.  Without the patch, and without -readnow, we
> have two a_thread_locals, the def and the decl:
> ...
> $ gdb -batch outputs/gdb.threads/tls/tls \
>     -ex "maint expand-symtabs" \
>     -ex "print a_thread_local" \
>     -ex "maint print symbols" \
>     | grep "a_thread_local;"
> Cannot read `a_thread_local' without registers
>  int a_thread_local; computed at runtime
>  int a_thread_local; unresolved
> ...
> while without the patch and with -readnow, we have the opposite order:
> ...
> $ gdb -readnow -batch outputs/gdb.threads/tls/tls  \
>     -ex "maint expand-symtabs" \
>     -ex "print a_thread_local" \
>     -ex "maint print symbols" \
>     | grep "a_thread_local;"
> Cannot find thread-local storage for process 0, executable file tls/tls:
> Cannot find thread-local variables on this target
>  int a_thread_local; unresolved
>  int a_thread_local; computed at runtime
> ...
> 
> With the patch we have the same order with and without -readnow, but just a
> different one than before without -readnow.
> 
> AFAIU, the fact that we pick the unresolved symbol over the computed at
> runtime symbol is some variant of PR24985 - "Cannot print value of global
> variable because decl in one CU shadows def in other CU".
> 
> Mark the "Cannot find thread-local variables on this target" variant a PR24985
> kfail.
> 
> OK for trunk?

Committed (
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=317d2668d01c7ddc9545029bf56d2b8c4d2bbfeb
).

Thanks,
- Tom