Re: [PATCH 2/2] gdb/testsuite: fix running gdb.linespec/cp-completion-aliases.exp with clang

[Bruno Larsen <blarsen@redhat.com>]

On 03/02/2023 14:44, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> Currently, all important tests in gdb.linespec/cp-completion-aliases.exp
>> are failing when using clang because the wrong type is being suggested
>> for the completion.  For example, running with gcc and clang we get the
>> following output respectively:
>>
>> (gdb) break get_value(object_p) <- gcc version
>> (gdb) break get_value(object*)  <- clang version
> You are correct that what we print for Clang is not wrong.  But I don't
> think it's as correct as what we print for GCC.  The user wrote
> object_p, and the DWARF does encode the object_p for both versions.
> It's just in the Clang case there's an extra DW_AT_linkage_name which
> seems to send GDB off doing the "wrong" thing.
>
> I wonder how hard it would actually be to "fix" this so that we print
> object_p for Clang?  I think it would be helpful to really understand at
> which point we diverge when comparing Clang and GCC binaries.
I took a look at the next patch you sent, but it introduced over 200 
regressions, I will take a look at solving this later.
>
>> Since both suggestions are acceptable on their own, and the original bug
>> was if GDB suggested both at the same time, this commit updates the test
>> to accept both, gcc's and clang's outputs.
> On the content of this patch though...
>
> I think we can achieve the same results without adding anything from
> patch #1 in this series.  My idea would be to just ask GDB what it's
> going to print using 'info functions', then we can expect exactly the
> right strings.  The patch below implements this idea.
>
> What do you think?

This idea sounds good. Would you like this changed even if the clang 
output is fixed? I can send a patch for it before looking further into 
how to change GDB's output.

Also, do you think the first patch should be abandoned? I feel like it 
would be a good addition even if we eventually drop my change, but I can 
drop it if you disagree.

>
> Thanks,
> Andrew
>
> ---
>
> diff --git a/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp b/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp
> index 33ad72e6f05..342359ea6e1 100644
> --- a/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp
> +++ b/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp
> @@ -24,18 +24,67 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}]} {
>       return -1
>   }
>   
> +# Clang adds a DW_AT_linkage_name to the DW_TAG_subprogram entry,
> +# which results in GDB showing the underlying type names for the
> +# arguments, rather than the typedef names.
> +#
> +# In contrast, GCC doesn't include the DW_AT_linkage_name, and so GDB
> +# shows the type name aliases instead.
> +#
> +# To figure out which we expect to see in the tab-completion output,
> +# use 'info functions' output.  Look for the functions we care about,
> +# and build a dictionary mapping line number to the argument type.
> +set types_dict [dict create]
> +foreach_with_prefix name {get_something grab_it get_value} {
> +    gdb_test_multiple "info function $name" "" {
> +	-re "^($::decimal):\\s+static int $name\\((\[^)\]+)\\);\r\n" {
> +	    set linum $expect_out(1,string)
> +	    set type $expect_out(2,string)
> +	    dict set types_dict $linum $type
> +	    exp_continue
> +	}
> +
> +	-re "^\[^0-9\]\[^\r\n\]+\r\n" {
> +	    exp_continue
> +	}
> +
> +	-re "^\\s*\r\n" {
> +	    exp_continue
> +	}
> +
> +	-re "^$::gdb_prompt $" {
> +	    pass $gdb_test_name
> +	}
> +    }
> +}
> +
> +# Now look in the dictionary we built above to figure out how GDB will
> +# display the types for different arguments.
> +set linum [gdb_get_line_number "get_value (object_p obj)"]
> +set object_pattern [dict get $types_dict $linum]
> +
> +set linum [gdb_get_line_number "grab_it (int_magic_t *var)"]
> +set magic_pattern [dict get $types_dict $linum]
> +
> +set linum [gdb_get_line_number "get_something (my_string_t msg)"]
> +set string_pattern [dict get $types_dict $linum]
> +
> +# Restart GDB, just in case the function lookup above in someway
> +# impacted what tab-completion results we might return.
> +clean_restart $binfile
> +
>   # Disable the completion limit for the whole testcase.
>   gdb_test_no_output "set max-completions unlimited"
>   
>   test_gdb_complete_unique \
>       "break get_v" \
> -    "break get_value(object_p)"
> +    "break get_value($object_pattern)"
>   
>   test_gdb_complete_unique \
>       "break gr" \
> -    "break grab_it(int_magic_t*)"
> +    "break grab_it($magic_pattern)"
>   
> -test_gdb_complete_multiple "break " "get_som" "ething(" {
> -    "get_something(my_string_t)"
> -    "get_something(object_p)"
> -}
> +test_gdb_complete_multiple "break " "get_som" "ething(" [list \
> +    "get_something($string_pattern)" \
> +    "get_something($object_pattern)" \
> +]
>

-- 
Cheers,
Bruno