Re: [PATCH 08/10] Use the linkage name if it exists

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

On 24-04-2020 18:06, Tom de Vries wrote:
> On 25-03-2020 21:07, Tom Tromey wrote:
>> The DWARF reader has had some odd code since the "physname" patches landed.
>>
>> In particular, these patches caused PR symtab/12707; namely, they made
>> it so "set print demangle off" no longer works.
>>
>> This patch attempts to fix the problem.  It arranges to store the
>> linkage name on the symbol if it exists, and it changes the DWARF
>> reader so that the demangled name is no longer (usually) stored in the
>> symbol's "linkage name" field.
>>
>> c-linkage-name.exp needed a tweak, because it started working
>> correctly.  This conforms to what I think ought to happen, so this
>> seems like an improvement here.
>>
>> compile-object-load.c needed a small change to use
>> symbol_matches_search_name rather than directly examining the linkage
>> name.  Looking directly at the name does the wrong thing for C++.
>>
>> There is still some name-related confusion in the DWARF reader:
>>
>> * "physname" often refers to the logical name and not what I would
>>   consider to be the "physical" name;
>>
>> * dwarf2_full_name, dwarf2_name, and dwarf2_physname all exist and
>>   return different strings -- but this seems like at least one name
>>   too many.  For example, Fortran requires dwarf2_full_name, but other
>>   languages do not.
>>
>> * To my surprise, dwarf2_physname prefers the form emitted by the
>>   demangler over the one that it computes.  This seems backward to me,
>>   given that the partial symbol reader prefers the opposite, and it
>>   seems to me that this choice may perform better as well.
>>
>> I didn't attempt to clean up these things.  It would be good to do,
>> but whenever I contemplate it I get caught up in dreams of truly
>> rewriting the DWARF reader instead.
>>
> 
> Hi,
> 
> As you mentioned on IRC, there's a regression with
> gdb.dwarf2/dw2-bad-mips-linkage-name.exp and target board
> cc-with-debug-names. I tracked that down to this patch in the series.
> 
> Using readelf -w, I can read the .debug_names section, and see without
> this patch:
> ...
> [  6] #0002b60b f: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> [  8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> ...
> and with this patch:
> ...
> [  6] #0ef9dc4b _Z1fv: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> [  8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> ...
> 
> So we end up with the mangled name for f in the index.  That looks
> significant, but I'm not sure yet if that is the reason why we're not
> able to print the type of f.
> 

Hmm, one thing that is odd about this test-case is that it constructs a
C language CU, which contains a subprogram with DW_AT_MIPS_linkage_name
attribute with a C++ mangled name.

If I update the testcase to use a c++ language CU, the test passes:
...
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-lin
kage-name.exp
index d00308a5702..5f01c41aaa3 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -38,7 +38,7 @@ Dwarf::assemble $asm_file {

     cu {} {
        DW_TAG_compile_unit {
-                {DW_AT_language @DW_LANG_C}
+                {DW_AT_language @DW_LANG_C_plus_plus}
                 {DW_AT_name     dw2-bad-mips-linkage-name.c}
                 {DW_AT_comp_dir /tmp}

@@ -78,5 +78,5 @@ if { [prepare_for_testing "failed to prepare"
${testfile} \
 # much matter what we test here, so long as we do something to make
 # sure that the DWARF is read.

-gdb_test "ptype f" " = bool \\(\\)"
-gdb_test "ptype g" " = bool \\(\\)"
+gdb_test "ptype f" " = bool \\(void\\)"
+gdb_test "ptype g" " = bool \\(void\\)"
...

Thanks,
- Tom