Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test

[Bruno Larsen <blarsen@redhat.com>]

On 4/29/22 12:48, Carl Love via Gdb-patches wrote:
> On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:
> 
> <snip>
> 
>>> Fix gdb.cp/no-dmgl-verbose.exp test
>>>
>>> The test tries to check that setting a break point on f
>>> (std::string) fails
>>> since the function is not defined.  The test is not syntactically
>>> correct.
>>> The test fails on both PowerPC and Intel.
>>>
>>> This patch fixes the test to set the breakpoint and verify that it
>>> fails
>>> since the function is not defined.
>>>
>>> The test now runs correctly on Power 10 and Intel x86_64.
>>> ---
>>>   gdb/testsuite/gdb.cp/no-dmgl-verbose.exp | 5 +----
>>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
>>> b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
>>> index 14f11ddcf04..d4ec88f3b6d 100644
>>> --- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
>>> +++ b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
>>> @@ -28,8 +28,5 @@ clean_restart ${testfile}.o
>>>   
>>>   gdb_test_no_output "set breakpoint pending off"
>>>   
>>> -gdb_breakpoint {'f(std::string)'}
>>> -
>>> -gdb_test {break 'f(std::basic_string<char, std::char_traits<char>,
>>> std::allocator<char> >)'} \
>>> -	 {Function ".*" not defined\.} \
>>> +gdb_test "break 'f(std::string)'" ".*Function.*not defined." \
>>                                                               ^
>>
>> I guess here the last . should be escaped (\.) otherwise has a
>> special
>> meaning for a regex.
> True, as written I believe the period will match any character.

Because of the wonders of DejaGNU, the period here has to be double escaped, otherwise it still works like a regex '.'

To confirm (because there are no rules for single, double or triple escaping) the easiest way to confirm your regex is doing what you want is surrounding the testcase with:

exp_internal 1
gdb_test ...
exp_internal 0

And try to parse what comes out. In this case, when the match happens, the important output is:

expect: does "break 'f(std::string)'\r\nFunction "f(std::string)" not defined.\r\n(gdb) " (spawn_id exp9) match regular expression ".*A problem internal to GDB has been detected"? Gate "*A problem internal to GDB has been detected"? gate=no
"\*\*\* DOSEXIT code.*"? Gate "\*\*\* DOSEXIT code*"? gate=no
"[\r\n]*(?:.*Function.*not defined.)[\r\n]+\(gdb\) $"? (No Gate, RE only) gate=yes re=yes

and in this last line you can see, for instance, the literal () characters being escaped, but the end of sentence period is not being escaped. Adding another backslash shows the escaped period.

After all this, I don't think it is really important that the period ends up correctly escaped, as the point of the test sems to be to confirm we can't set a breakpoint in the function, not necessarily checking the exact grammar of the output.

>>
>>>   	 "DMGL_VERBOSE-demangled f(std::string) is not defined"
>>
>> Also looks like that the test was testing that we can break on
>> "f(std::string)", not "f($what_string_really_is)".  This is not GDB's
>> behaviour, as far as I can tell.  To me GDB does the opposite.  One
>> can
>> break using the full name of std::string, not "std::string".
>>
>> Also, on my system, std::string is in reality
>> "std::__cxx11::basic_string<char, std::char_traits<char>,
>> std::allocator<char> >",
>> not what is used in the test.
> 
> Yes, on PowerPC I also see the same type
> 
> File /usr/include/c++/11/bits/stringfwd.h:
> 79:     typedef std::__cxx11::basic_string<char,
> std::char_traits<char>, std::allocator<char> > std::string;
>>
>> I can test such behaviour with something like:
>>
>>      gdb_test "break 'f(std::string)'" ".*Function.*not defined\." \
>>          "f(std::string) is not defined"
>>
>>      set realtype ""
>>      gdb_test_multiple "info types ^std::string$" "" {
>>          -re -wrap "typedef (\[^;\]*) std::string;" {
>>              set realtype $expect_out(1,string)
>>              pass $gdb_test_name
>>          }
>>      }
>>
>>      if { $realtype != "" } {
>>          gdb_breakpoint "f($realtype)"
>>      }
>>
>> That being said, it would be nice to be able to place a breakpoint
>> using
>> "f(std::string)"…
> 
> OK, I added the new test.  It works as expected on PowerPC and on my
> Intel system.
> 
> (gdb) PASS: gdb.cp/no-dmgl-verbose.exp: info types ^std::string$
> break f(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
> 
> Breakpoint 1 at 0x10: file /home/carll/GDB/build-current/gdb/testsuite/../../../binutils-gdb-current/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc, line 23.
> 
> The additional test is good to have.
> 
> The above suggested changes were tested on both PowerPC and Intel and
> pass as expected.
> 
> Please let me know if the updated patch looks ok.  Thanks.
> 
>                          Carl Love
> -----------------------------------------------------------
> Fix gdb.cp/no-dmgl-verbose.exp test
> 
> The test tries to check that setting a break point on f (std::string) fails
> since the function is not defined as given.  The test was chaged to verify
> f(std::string) is not defined.  An additional test was added to test setting
> a breakpoint using the actual string type.
> 
> The test now runs correctly on Power 10 and Intel x86_64.
> ---
>   gdb/testsuite/gdb.cp/no-dmgl-verbose.exp | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> index 14f11ddcf04..a18ab81318c 100644
> --- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> +++ b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> @@ -28,8 +28,17 @@ clean_restart ${testfile}.o
>   
>   gdb_test_no_output "set breakpoint pending off"
>   
> -gdb_breakpoint {'f(std::string)'}
> +gdb_test "break 'f(std::string)'" ".*Function.*not defined\." \
> +    "f(std::string) is not defined"
> +
> +set realtype ""
> +gdb_test_multiple "info types ^std::string$" "" {
> +    -re -wrap "typedef (\[^;\]*) std::string;" {
> +	set realtype $expect_out(1,string)
> +	pass $gdb_test_name
> +    }
> +}
>   
> -gdb_test {break 'f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)'} \
> -	 {Function ".*" not defined\.} \
> -	 "DMGL_VERBOSE-demangled f(std::string) is not defined"
> +if { $realtype != "" } {
> +    gdb_breakpoint "f($realtype)"
> +}

The updated patch does LGTM, but I can't approve patches.

Cheers!
Bruno Larsen