Re: [PATCH 07/11] fix gdb.base/call-ar-st to work with clang

[Bruno Larsen <blarsen@redhat.com>]

On 3/2/22 15:59, Pedro Alves wrote:
> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> When running this test with clang-compiled code, this test was failing
>> by producing an output like
>> array_i=<main.integer_array>, ...
>>
>> instead of
>>
>> array_i = <integer_array>, ...
>>
>> This commit changes the testcase to accept both outputs, as they are
>> functionally identical.
> 
> It would be useful to explain that the symbols in question are local static variables.
> "main" is the name of the function they are defined in, and Clang puts that in the elf/linkage name.
> GCC instead appends a sequence number to the linkage name:
> 
>   $ nm -A call-ar-st.gcc | grep integer_
>   call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968
> 
>   $ nm -A call-ar-st.clang | grep integer_
>   call-ar-st:00000000004061a0 b main.integer_array
> 
> Note that for GCC, even though the ELF symbol is called "integer_array.3968", GDB still
> prints "integer_array", without the suffix.  I wonder how that happens?  Anyone knows offhand?
> It kind of looks like the testcase was expecting that the ".3968" suffix could appear in
> the output, given that it expects "array_i=<integer_array.*>" instead of "array_i=<integer_array>" ?

I tried to dig around for a bit, but couldn't find any obvious location. I can look some more if we ultimately decide it is a good idea to remove it, but personally, I think that knowing where the variable was declared could be helpful for debugging. Meanwhile, your patch looks good, I can add it to my tree for pushing at the end.

> 
>> ---
>>   gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
>> index d1fb3ac9ec1..e447d3e231d 100644
>> --- a/gdb/testsuite/gdb.base/call-ar-st.exp
>> +++ b/gdb/testsuite/gdb.base/call-ar-st.exp
>> @@ -153,9 +153,16 @@ if {!$skip_float_test && \
>>   
>>   #step
>>   set stop_line [gdb_get_line_number "-step1-"]
>> -gdb_test "step" \
>> -    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
>> -    "step inside print_all_arrays"
>> +
>> +gdb_test_multiple "step" "" {
>> +
>> +    -re "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
>> +	pass "step inside print_all_arrays"
>> +    }
>> +    -re "print_all_arrays \\(array_i=<main.integer_array>, array_c=<main.char_array> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<main.float_array>, array_d=<main.double_array>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
>> +	pass "step inside print_all_arrays (clang)"
>> +    }
>> +}
>>   
> 
> If we end up taking the approach of accepting the current GDB output, then it would seem better
> to me to continue having a single expression.  Something like this:
> 
>  From d024950e743df1712f9adda759023e5ba07ee6e1 Mon Sep 17 00:00:00 2001
> From: Bruno Larsen <blarsen@redhat.com>
> Date: Wed, 2 Mar 2022 18:09:31 +0000
> Subject: [PATCH] Fix gdb.base/call-ar-st to work with Clang
> 
> When running gdb.base/call-ar-st.exp against Clang, we see one FAIL,
> like so:
> 
>   print_all_arrays (array_i=<main.integer_array>, array_c=<main.char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa
>   ZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<main.float_array>, array_d=<main.double_array>) at ../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
>   274       print_int_array(array_i);     /* -step1- */
>   (gdb) FAIL: gdb.base/call-ar-st.exp: step inside print_all_arrays
> 
> With GCC we instead see:
> 
>   print_all_arrays (array_i=<integer_array>, array_c=<char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<float_array>, array_d=<double_array>) at /home/pedro/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
>   274       print_int_array(array_i);     /* -step1- */
>   (gdb) PASS: gdb.base/call-ar-st.exp: step inside print_all_arrays
> 
> The difference is that with Clang we get:
> 
>   array_i=<main.integer_array>, ...
> 
> instead of
> 
>   array_i = <integer_array>, ...
> 
> These symbols are local static variables, and "main" is the name of
> the function they are defined in.  GCC instead appends a sequence
> number to the linkage name:
> 
>   $ nm -A call-ar-st.gcc | grep integer_
>   call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968
> 
>   $ nm -A call-ar-st.clang | grep integer_
>   call-ar-st:00000000004061a0 b main.integer_array
> 
> This commit changes the testcase to accept both outputs, as they are
> functionally identical.
> 
> Co-Authored-By: Pedro Alves <pedro@palves.net>
> Change-Id: Iaf2ccdb9d5996e0268ed12f595a6e04b368bfcb4
> ---
>   gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
> index d1fb3ac9ec1..43fd6ff9f40 100644
> --- a/gdb/testsuite/gdb.base/call-ar-st.exp
> +++ b/gdb/testsuite/gdb.base/call-ar-st.exp
> @@ -151,10 +151,21 @@ if {!$skip_float_test && \
>       gdb_test "continue" ".*" ""
>   }
>   
> +# Return a regexp that matches the linkage name of SYM, assuming SYM
> +# is a local static variable inside the main function.
> +proc main_static_local_re {sym} {
> +    # Clang prepends the function name + '.'.
> +    return "(main\\.)?${sym}"
> +}
> +
>   #step
>   set stop_line [gdb_get_line_number "-step1-"]
>   gdb_test "step" \
> -    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
> +    "print_all_arrays \\(array_i=<[main_static_local_re integer_array]>,\
> +			 array_c=<[main_static_local_re char_array]> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa.,\
> +			 array_f=<[main_static_local_re float_array]>,\
> +			 array_d=<[main_static_local_re double_array]>\\)\
> +			 at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
>       "step inside print_all_arrays"
>   
>   #step -over
> 


-- 
Cheers!
Bruno Larsen