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

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

[Carl Love]

GDB maintainers:

The gdb.cp/no-dmgl-verbose.exp test does a gdb_test which is not
written correctly.  The test fails on both PowerPC and Intel.  The
following patch update the gdb_test statement to the correct format. 
The test now works correctly on PowerPC and Intel.

Please let me know if the patch is acceptable for mainline.  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.  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." \
 	 "DMGL_VERBOSE-demangled f(std::string) is not defined"
-- 
2.31.1

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

[Lancelot SIX]

Hi,

Thanks for looking into it.

On Thu, Apr 28, 2022 at 06:28:31PM -0700, Carl Love via Gdb-patches wrote:
> 
> GDB maintainers:
> 
> The gdb.cp/no-dmgl-verbose.exp test does a gdb_test which is not
> written correctly.  The test fails on both PowerPC and Intel.  The
> following patch update the gdb_test statement to the correct format. 
> The test now works correctly on PowerPC and Intel.
> 
> Please let me know if the patch is acceptable for mainline.  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.  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.

>  	 "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.

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)"…

Best,
Lancelot.

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

[Carl Love]

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.
> 
> >  	 "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)"
+}
-- 
2.31.1

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

[Bruno Larsen]

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

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

[Pedro Alves]

On 2022-04-29 16:48, Carl Love via Gdb-patches wrote:
> On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:

> 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.

So the file is called gdb.cp/no-dmgl-verbose.exp.  "no-dmgl" most certainly means "no demangle".

However, the intro comment says:

# Test loading symbols from unrelocated C++ object files.

and indeed, the testcase only compiles an object:

 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug}] != "" } {
      untested "failed to compile"
      return -1
 }

... and loads that into gdb:

 clean_restart ${testfile}.o

How is that related to "no demangle verbose" ?  A mystery.

OK.  Now, if we're not going to be testing that setting a breakpoint via the
typedef, like "b f(std::string)", WORKS (I thought GDB had code to make that work,
why doesn't it?), then what's the point of the test?  Testing that the breakpoint
set at the non-typedef expansion of std::string is really no different from setting
a breakpoint at any other C++ function, so we could eliminate the std::string dependency.

It seems like something is missing here.

What did the original submission of the patch that added the test say?

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

[Keith Seitz]

On 4/29/22 09:57, Pedro Alves wrote:
> On 2022-04-29 16:48, Carl Love via Gdb-patches wrote:
>> On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:
> 
> So the file is called gdb.cp/no-dmgl-verbose.exp.  "no-dmgl" most certainly means "no demangle".
> 
> How is that related to "no demangle verbose" ?  A mystery.

I believe I remember some history of this...

When I did the physname work years ago, a maintainer objected that the
recorded physname for a function which takes a std::string was
"reduced" to "std::string<blah blah blah>" instead of recording (and
thus subsequently printing) "std::string" like other tools do. [He
speciifcally mentioned "nm".]

The "no-dmgl-verbose" refers to the demangler option, DMGL_VERBOSE,
which I originally used when computing physnames. I believe this test's
intention was to make sure that DMGL_VERBOSE didn't creep back into the
code.

[Background: At the time, the compiler did not output sufficient debuginfo
for a bunch of symbols, such as ctors. Thus the physname computation
was a way to "fill-in" this missing/necessary information.]

There's a number of other workarounds for this "std::string"
vs "std::string<blah blah blah>" (and others) in cp-support.c.
See "ignore_typedefs". [Pardon if my explanation is imprecise.
This was a looong time ago.]

Keith

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

[Pedro Alves]

On 2022-04-29 18:09, Keith Seitz wrote:
> On 4/29/22 09:57, Pedro Alves wrote:
>> On 2022-04-29 16:48, Carl Love via Gdb-patches wrote:
>>> On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:
>>
>> So the file is called gdb.cp/no-dmgl-verbose.exp.  "no-dmgl" most certainly means "no demangle".
>>
>> How is that related to "no demangle verbose" ?  A mystery.
> 
> I believe I remember some history of this...
> 
> When I did the physname work years ago, a maintainer objected that the
> recorded physname for a function which takes a std::string was
> "reduced" to "std::string<blah blah blah>" instead of recording (and
> thus subsequently printing) "std::string" like other tools do. [He
> speciifcally mentioned "nm".]
> 
> The "no-dmgl-verbose" refers to the demangler option, DMGL_VERBOSE,
> which I originally used when computing physnames. I believe this test's
> intention was to make sure that DMGL_VERBOSE didn't creep back into the
> code.
> 
> [Background: At the time, the compiler did not output sufficient debuginfo
> for a bunch of symbols, such as ctors. Thus the physname computation
> was a way to "fill-in" this missing/necessary information.]
> 
> There's a number of other workarounds for this "std::string"
> vs "std::string<blah blah blah>" (and others) in cp-support.c.
> See "ignore_typedefs". [Pardon if my explanation is imprecise.
> This was a looong time ago.]

Thanks Keith!

That leads to this:

 https://sourceware.org/pipermail/gdb-patches/2011-July/thread.html

The "Test loading symbols from unrelocated C++ object files." intro is found in

 gdb.cp/cp-relocate.exp:# Test loading symbols from unrelocated C++ object files.

so I guess that Jan copied that testcase, and didn't update the intro.  The "unrelocated"
aspect seems like not important for the test.

Note how Jan says:

"After Keith's fix of GDB PR 12266 GDB should resolve mostly any form
(typedefed/untypedefed etc.) of the user entered function parameters."

I believe that should be true today, and GDB should be able to set a breakpoint
on the typedef'ed f(std::string), no?  I find it very hard to believe that Jan
didn't notice that the

 gdb_breakpoint {'f(std::string)'}

call was failing back then.  From the message, it very much seems like it was
passing back then.  And then, the other test:

 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"

means that "std::basic_string<char, std::char_traits<char>, std::allocator<char> >" is what
you get when you demangle the function's linkage name with DMGL_VERBOSE, which is different
from what you get if you demangle without DMGL_VERBOSE, and this test is thus making sure that
such a demangled name is not defined.

The proposed change completely turns the testcase on its head.

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

[Lancelot SIX]

On Fri, Apr 29, 2022 at 10:09:49AM -0700, Keith Seitz wrote:
> On 4/29/22 09:57, Pedro Alves wrote:
> > On 2022-04-29 16:48, Carl Love via Gdb-patches wrote:
> > > On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:
> > 
> > So the file is called gdb.cp/no-dmgl-verbose.exp.  "no-dmgl" most certainly means "no demangle".
> > 
> > How is that related to "no demangle verbose" ?  A mystery.
> 
> I believe I remember some history of this...
> 
> When I did the physname work years ago, a maintainer objected that the
> recorded physname for a function which takes a std::string was
> "reduced" to "std::string<blah blah blah>" instead of recording (and
> thus subsequently printing) "std::string" like other tools do. [He
> speciifcally mentioned "nm".]
> 
> The "no-dmgl-verbose" refers to the demangler option, DMGL_VERBOSE,
> which I originally used when computing physnames. I believe this test's
> intention was to make sure that DMGL_VERBOSE didn't creep back into the
> code.
> 
> [Background: At the time, the compiler did not output sufficient debuginfo
> for a bunch of symbols, such as ctors. Thus the physname computation
> was a way to "fill-in" this missing/necessary information.]
> 
> There's a number of other workarounds for this "std::string"
> vs "std::string<blah blah blah>" (and others) in cp-support.c.
> See "ignore_typedefs". [Pardon if my explanation is imprecise.
> This was a looong time ago.]
> 
> Keith
> 

Hi,

Thanks for the history perspective.

I was digging in the commits logs, and this commit was introduced by
3f542ed1a314b4831f71ade5006d1926d58f8303.  This test removed the
DMGL_VERBOSE option.

I mainly did that because I was wondering if the test name was still
appropriate (I kind of think it is not), but I do not have a "good" name
to propose just yet.

Best,
Lancelot.

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

[Pedro Alves]

On 2022-04-29 18:20, Pedro Alves wrote:
> On 2022-04-29 18:09, Keith Seitz wrote:
>> On 4/29/22 09:57, Pedro Alves wrote:
>>> On 2022-04-29 16:48, Carl Love via Gdb-patches wrote:
>>>> On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:
>>>
>>> So the file is called gdb.cp/no-dmgl-verbose.exp.  "no-dmgl" most certainly means "no demangle".
>>>
>>> How is that related to "no demangle verbose" ?  A mystery.
>>
>> I believe I remember some history of this...
>>
>> When I did the physname work years ago, a maintainer objected that the
>> recorded physname for a function which takes a std::string was
>> "reduced" to "std::string<blah blah blah>" instead of recording (and
>> thus subsequently printing) "std::string" like other tools do. [He
>> speciifcally mentioned "nm".]
>>
>> The "no-dmgl-verbose" refers to the demangler option, DMGL_VERBOSE,
>> which I originally used when computing physnames. I believe this test's
>> intention was to make sure that DMGL_VERBOSE didn't creep back into the
>> code.
>>
>> [Background: At the time, the compiler did not output sufficient debuginfo
>> for a bunch of symbols, such as ctors. Thus the physname computation
>> was a way to "fill-in" this missing/necessary information.]
>>
>> There's a number of other workarounds for this "std::string"
>> vs "std::string<blah blah blah>" (and others) in cp-support.c.
>> See "ignore_typedefs". [Pardon if my explanation is imprecise.
>> This was a looong time ago.]
> 
> Thanks Keith!
> 
> That leads to this:
> 
>  https://sourceware.org/pipermail/gdb-patches/2011-July/thread.html

Sigh, wrong url, obviously.  I meant, this:

 https://sourceware.org/pipermail/gdb-patches/2011-June/083081.html

Pedro Alves

> 
> The "Test loading symbols from unrelocated C++ object files." intro is found in
> 
>  gdb.cp/cp-relocate.exp:# Test loading symbols from unrelocated C++ object files.
> 
> so I guess that Jan copied that testcase, and didn't update the intro.  The "unrelocated"
> aspect seems like not important for the test.
> 
> Note how Jan says:
> 
> "After Keith's fix of GDB PR 12266 GDB should resolve mostly any form
> (typedefed/untypedefed etc.) of the user entered function parameters."
> 
> I believe that should be true today, and GDB should be able to set a breakpoint
> on the typedef'ed f(std::string), no?  I find it very hard to believe that Jan
> didn't notice that the
> 
>  gdb_breakpoint {'f(std::string)'}
> 
> call was failing back then.  From the message, it very much seems like it was
> passing back then.  And then, the other test:
> 
>  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"
> 
> means that "std::basic_string<char, std::char_traits<char>, std::allocator<char> >" is what
> you get when you demangle the function's linkage name with DMGL_VERBOSE, which is different
> from what you get if you demangle without DMGL_VERBOSE, and this test is thus making sure that
> such a demangled name is not defined.
> 
> The proposed change completely turns the testcase on its head.

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

[Pedro Alves]

On 2022-04-29 18:20, Pedro Alves wrote:
> On 2022-04-29 18:09, Keith Seitz wrote:
>> On 4/29/22 09:57, Pedro Alves wrote:
>>> On 2022-04-29 16:48, Carl Love via Gdb-patches wrote:
>>>> On Fri, 2022-04-29 at 09:14 +0000, Lancelot SIX wrote:
>>>
>>> So the file is called gdb.cp/no-dmgl-verbose.exp.  "no-dmgl" most certainly means "no demangle".
>>>
>>> How is that related to "no demangle verbose" ?  A mystery.
>>
>> I believe I remember some history of this...
>>
>> When I did the physname work years ago, a maintainer objected that the
>> recorded physname for a function which takes a std::string was
>> "reduced" to "std::string<blah blah blah>" instead of recording (and
>> thus subsequently printing) "std::string" like other tools do. [He
>> speciifcally mentioned "nm".]
>>
>> The "no-dmgl-verbose" refers to the demangler option, DMGL_VERBOSE,
>> which I originally used when computing physnames. I believe this test's
>> intention was to make sure that DMGL_VERBOSE didn't creep back into the
>> code.
>>
>> [Background: At the time, the compiler did not output sufficient debuginfo
>> for a bunch of symbols, such as ctors. Thus the physname computation
>> was a way to "fill-in" this missing/necessary information.]
>>
>> There's a number of other workarounds for this "std::string"
>> vs "std::string<blah blah blah>" (and others) in cp-support.c.
>> See "ignore_typedefs". [Pardon if my explanation is imprecise.
>> This was a looong time ago.]
> 
> Thanks Keith!
> 
> That leads to this:
> 
>  https://sourceware.org/pipermail/gdb-patches/2011-July/thread.html
> 
> The "Test loading symbols from unrelocated C++ object files." intro is found in
> 
>  gdb.cp/cp-relocate.exp:# Test loading symbols from unrelocated C++ object files.
> 
> so I guess that Jan copied that testcase, and didn't update the intro.  The "unrelocated"
> aspect seems like not important for the test.
> 
> Note how Jan says:
> 
> "After Keith's fix of GDB PR 12266 GDB should resolve mostly any form
> (typedefed/untypedefed etc.) of the user entered function parameters."
> 
> I believe that should be true today, and GDB should be able to set a breakpoint
> on the typedef'ed f(std::string), no?  I find it very hard to believe that Jan
> didn't notice that the
> 
>  gdb_breakpoint {'f(std::string)'}
> 
> call was failing back then.  From the message, it very much seems like it was
> passing back then.  

Thanks for pointing at "ignore_typedefs" in cp-support.c, Keith.  So if I hack out
that special casing for std::string, like in the patch below, then GDB replaces the
typedef, and setting the breakpoint works.  However, that only works if we debug
a fully linked binary for some reason...  If we debug just the .o file, then the symbol
still isn't found...  That seems yet another bug somewhere.

To be clear, with this hack patch below, the testcase passes for me.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
From 6154b784240703f2b7be1aa05c2b813aa2511435 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 29 Apr 2022 18:41:08 +0100
Subject: [PATCH] don't mask out std::string

Change-Id: Ief1da58246b859228fbe318d514cb9ce34629990
---
 gdb/cp-support.c                         | 3 ++-
 gdb/testsuite/gdb.cp/no-dmgl-verbose.cc  | 5 +++++
 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp | 4 ++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index f52055893d2..78c302abaef 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -146,13 +146,14 @@ inspect_type (struct demangle_parse_info *info,
   memcpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len);
   name[ret_comp->u.s_name.len] = '\0';
 
+#if 0
   /* Ignore any typedefs that should not be substituted.  */
   for (const char *ignorable : ignore_typedefs)
     {
       if (strcmp (name, ignorable) == 0)
 	return 0;
     }
-
+#endif
   sym = NULL;
 
   try
diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc b/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
index 454ab4c42ea..d9a666f2c48 100644
--- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
+++ b/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
@@ -21,3 +21,8 @@ void
 f (std::string s)
 {
 }
+
+int
+main ()
+{
+}
diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
index 14f11ddcf04..c54daf33c8b 100644
--- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
+++ b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
@@ -19,12 +19,12 @@ standard_testfile .cc
 
 if { [skip_cplus_tests] } { continue }
 
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug}] != "" } {
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {c++ debug}] != "" } {
      untested "failed to compile"
      return -1
 }
 
-clean_restart ${testfile}.o
+clean_restart ${testfile}
 
 gdb_test_no_output "set breakpoint pending off"
 

base-commit: 225170409b47bc96b62b2ecfcc0d9d5ae1ef8949
prerequisite-patch-id: be6afcdb35fd6ed141be7d568afbcd1ae4f082ee
prerequisite-patch-id: be2d0109aea479ee1973cf5b6d6454b5811cfce4
-- 
2.36.0
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So that seems like something that should be fixed in GDB, somehow.



Now, I'm wondering what is it that DMGL_VERBOSE outputs differently than not
having it.  The full name of std::string in libstd++ is nowadays:

  "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >",

and the only difference compared to what we have in the test is the __cxx11 namespace,
which has meanwhile been added for the new C++11 ABI.

Here's today, vs testcase's, with extra space added for alignment:

 today:  "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >"
 test:   "std::         basic_string<char, std::char_traits<char>, std::allocator<char> >"

Hmm.  If I force the testcase to use the old ABI to emulate what was being seeing
back then, with:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git c/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp w/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
index c54daf33c8b..4718191b471 100644
--- c/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
+++ w/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
@@ -19,7 +19,7 @@ standard_testfile .cc
 
 if { [skip_cplus_tests] } { continue }
 
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {c++ debug}] != "" } {
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {c++ debug additional_flags=-D_GLIBCXX_USE_CXX11_ABI=0}] != "" } {
      untested "failed to compile"
      return -1
 }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'd expect that the breakpoint at the non-typedef'd name, like in:

 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"

... would actually manage to be set, and thus that test would fail.  Obviously it didn't
back then.  But why?

Turns out that with the old ABI we get this mangled name for the function:

 $ nm -A testsuite/outputs/gdb.cp/no-dmgl-verbose/no-dmgl-verbose |grep _Z 
 testsuite/outputs/gdb.cp/no-dmgl-verbose/no-dmgl-verbose:0000000000001129 T _Z1fSs

and running through c++filt does give us:

 $ echo _Z1fSs | c++filt
 f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)

which is the demangled name Jan used in the test.

Interestingly, _Z1fSs seems very short.  That "S" seems like a shortcut for std::string.  Here's what
the mangled name looks like WITHOUT -D_GLIBCXX_USE_CXX11_ABI=0, thus with modern ABI:

 testsuite/outputs/gdb.cp/no-dmgl-verbose/no-dmgl-verbose:0000000000001129 T _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE


If I run the gdb.cp/no-dmgl-verbose.exp test with -D_GLIBCXX_USE_CXX11_ABI=0, plus the hack in cp-support.c
to disable the std::string typedef thing, the breakpoint at f(std::string) surprisingly fails again!

 (gdb) PASS: gdb.cp/no-dmgl-verbose.exp: set breakpoint pending off
 break 'f(std::string)'
 Function "f(std::string)" not defined.
 (gdb) FAIL: gdb.cp/no-dmgl-verbose.exp: gdb_breakpoint: set breakpoint at 'f(std::string)'
 break 'f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)'
 Function "f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)" not defined.
 (gdb) PASS: gdb.cp/no-dmgl-verbose.exp: DMGL_VERBOSE-demangled f(std::string) is not defined

But note, if I debug it manually, I see this:

  (gdb) b f( [TAB]
  (gdb) b f( std::string) 

Ah!  That's different with the modern ABI, where TAB completion gives you the
whole full basic_string non-typedefed name.   Ah!  That's because GDB demangles the old
ABI's _Z1fSs as f(std::string):

 (gdb) demangle _Z1fSs
 f(std::string)

 (gdb) demangle _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
 f(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)

And note this:

 $ echo "_Z1fSs" | c++filt
 f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)

 $ echo "_Z1fSs" | c++filt --no-verbose
 f(std::string)

Bingo.  c++filt uses DMGL_VERBOSE by default, unless you pass it --no-verbose.

I don't know why the test would fail if I force the old ABI, and I hack out the 
std::string typedef replacement stuff in cp-support.c.  That's weird.


And now the fun thing -- reverting the cp-support.c hack, and just adding the -D_GLIBCXX_USE_CXX11_ABI=0
makes the testcase behave like it used to when it was written, and it passes cleanly
for me, as below.

GDB should be fixed so that setting a breakpoint at "f(std::string)" should work
with new ABI too, but that can be a separate thing.

From 4a9a9b0bcff56254b7897a3820bc06d9987aeac3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 29 Apr 2022 19:28:13 +0100
Subject: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp

Change-Id: I196568fd92c2a6747a0467f12510714f549be2b7
---
 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
index 14f11ddcf04..fd4e56f40b4 100644
--- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
+++ b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
@@ -19,7 +19,7 @@ standard_testfile .cc
 
 if { [skip_cplus_tests] } { continue }
 
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug}] != "" } {
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug additional_flags=-D_GLIBCXX_USE_CXX11_ABI=0}] != "" } {
      untested "failed to compile"
      return -1
 }

-- 
2.36.0

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

[Carl Love]

GDB maintaners, Pedro, Lancelot:

I am not much of a C++ programer.  I will have to admit that there is a
fair bit of the deep C++ behaviour with old/new APIs in Pedro's post
that is beyond me.  :-)  Sorry.  Additionally, there is concern that
the name of the test is not accurate, etc.

So at this point, not sure where to go with fixing this test.  I seem
to have opened a real can of worms.  Suggestions on how to move
forward, remove the test, go with a minimal change to get the test to
pass, do a rewrite/name change, .... ?

                      Carl Love

[PATCH] Fix "b func(std::string)", use DMGL_VERBOSE (was: Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test)

[Pedro Alves]

On 2022-04-29 20:13, Carl Love wrote:
> GDB maintaners, Pedro, Lancelot:
> 
> I am not much of a C++ programer.  I will have to admit that there is a
> fair bit of the deep C++ behaviour with old/new APIs in Pedro's post
> that is beyond me.  :-)  Sorry.  Additionally, there is concern that
> the name of the test is not accurate, etc.
> 
> So at this point, not sure where to go with fixing this test.  I seem
> to have opened a real can of worms.  Suggestions on how to move
> forward, remove the test, go with a minimal change to get the test to
> pass, do a rewrite/name change, .... ?
> 

I knew I wouldn't be able to sleep if I didn't give this a try, so here it goes...

I think we should fix it properly such that "b f(std::string)" also works
with the modern C++11 ABI.  Given with the modern ABI, we don't even
end up with standard substitutions in the mangled name, avoiding DMGL_VERBOSE
seems pointless, as everyone who doesn't force old ABI in their compilations is getting
expanded std::string typedefs anyhow.  I think we should instead enforce usage of
DMGL_VERBOSE throughout GDB, and remove the special treatment for std::string and
others from cp-support.c.  This way, old ABI behaves as new ABI.  If we do this, then
gdb.cp/no-dmgl-verbose.exp loses it's main purpose (making sure DMGL_VERBOSE doesn't
creep back in), and it should be renamed/adjusted to test that setting a
breakpoint at f(std::string) works, _and_ that setting a breakpoint at
the expanded non-typedefed version of that function works too, similar to what
Lancelot had originally suggested.  (I think we should be able to use whatis instead
of info types, though.)

I'll have to leave writing a proper commit log for later.  Meanwhile, here's a patch
for comments.

No regressions on x86-64 GNU/Linux, and setting a breakpoint at "f(std::string)"
now works with either old or new ABI.

From d298d3c78f2d19bb70b312674b5ef9e5dbce2fde Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 29 Apr 2022 23:21:18 +0100
Subject: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE

Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
---
 gdb/cp-name-parser.y                          |   5 +-
 gdb/cp-support.c                              |  52 ++++++---
 gdb/cp-support.h                              |  10 ++
 ...no-dmgl-verbose.cc => break-std-string.cc} |   0
 gdb/testsuite/gdb.cp/break-std-string.exp     | 108 ++++++++++++++++++
 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp      |  35 ------
 6 files changed, 156 insertions(+), 54 deletions(-)
 rename gdb/testsuite/gdb.cp/{no-dmgl-verbose.cc => break-std-string.cc} (100%)
 create mode 100644 gdb/testsuite/gdb.cp/break-std-string.exp
 delete mode 100644 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 6410363c87f..ceb4c968935 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -1959,8 +1959,7 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len)
 {
   size_t err;
 
-  char *res = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI,
-				    result, estimated_len, &err);
+  char *res = gdb_cplus_demangle_print (result, estimated_len, &err);
   return gdb::unique_xmalloc_ptr<char> (res);
 }
 
@@ -2060,7 +2059,7 @@ cp_print (struct demangle_component *result)
   char *str;
   size_t err = 0;
 
-  str = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI, result, 64, &err);
+  str = gdb_cplus_demangle_print (result, 64, &err);
   if (str == NULL)
     return;
 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index f52055893d2..b3eb775f717 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -70,18 +70,16 @@ static void add_symbol_overload_list_qualified
 
 struct cmd_list_element *maint_cplus_cmd_list = NULL;
 
-/* A list of typedefs which should not be substituted by replace_typedefs.  */
-static const char * const ignore_typedefs[] =
-  {
-    "std::istream", "std::iostream", "std::ostream", "std::string"
-  };
-
 static void
   replace_typedefs (struct demangle_parse_info *info,
 		    struct demangle_component *ret_comp,
 		    canonicalization_ftype *finder,
 		    void *data);
 
+static struct demangle_component *
+  gdb_cplus_demangle_v3_components (const char *mangled,
+				    int options, void **mem);
+
 /* A convenience function to copy STRING into OBSTACK, returning a pointer
    to the newly allocated string and saving the number of bytes saved in LEN.
 
@@ -146,13 +144,6 @@ inspect_type (struct demangle_parse_info *info,
   memcpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len);
   name[ret_comp->u.s_name.len] = '\0';
 
-  /* Ignore any typedefs that should not be substituted.  */
-  for (const char *ignorable : ignore_typedefs)
-    {
-      if (strcmp (name, ignorable) == 0)
-	return 0;
-    }
-
   sym = NULL;
 
   try
@@ -238,6 +229,14 @@ inspect_type (struct demangle_parse_info *info,
 	  string_file buf;
 	  try
 	    {
+	      /* If the current language is C, and type is a
+		 struct/class, type_print prefixes the type name with
+		 "struct" or "class", which we don't want when we're
+		 expanding a C++ typedef.  Force language temporarily
+		 to avoid it.  */
+	      scoped_restore_current_language restore_language;
+	      set_language (language_cplus);
+
 	      type_print (type, "", &buf, -1);
 	    }
 	  /* If type_print threw an exception, there is little point
@@ -670,8 +669,8 @@ mangled_name_to_comp (const char *mangled_name, int options,
     {
       struct demangle_component *ret;
 
-      ret = cplus_demangle_v3_components (mangled_name,
-					  options, memory);
+      ret = gdb_cplus_demangle_v3_components (mangled_name,
+					      options, memory);
       if (ret)
 	{
 	  std::unique_ptr<demangle_parse_info> info (new demangle_parse_info);
@@ -1635,7 +1634,7 @@ gdb_demangle (const char *name, int options)
 #endif
 
   if (crash_signal == 0)
-    result.reset (bfd_demangle (NULL, name, options));
+    result.reset (bfd_demangle (NULL, name, options | DMGL_VERBOSE));
 
 #ifdef HAVE_WORKING_FORK
   if (catch_demangler_crashes)
@@ -1670,6 +1669,27 @@ gdb_demangle (const char *name, int options)
 
 /* See cp-support.h.  */
 
+char *
+gdb_cplus_demangle_print (struct demangle_component *tree,
+			  int estimated_length,
+			  size_t *p_allocated_size)
+{
+  return cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI | DMGL_VERBOSE,
+			       tree, estimated_length, p_allocated_size);
+}
+
+/* A wrapper for cplus_demangle_v3_components that forces
+   DMGL_VERBOSE.  */
+
+static struct demangle_component *
+gdb_cplus_demangle_v3_components (const char *mangled,
+				  int options, void **mem)
+{
+  return cplus_demangle_v3_components (mangled, options | DMGL_VERBOSE, mem);
+}
+
+/* See cp-support.h.  */
+
 unsigned int
 cp_search_name_hash (const char *search_name)
 {
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 4fbd53c8923..0a30e59d36d 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -186,10 +186,20 @@ extern void cp_merge_demangle_parse_infos (struct demangle_parse_info *,
 
 extern struct cmd_list_element *maint_cplus_cmd_list;
 
+/* Wrappers for bfd and libiberty demangling entry points.  Note they
+   all force DMGL_VERBOSE so that callers don't need to.  This is so
+   that GDB consistently uses DMGL_VERBOSE throughout.  */
+
 /* A wrapper for bfd_demangle.  */
 
 gdb::unique_xmalloc_ptr<char> gdb_demangle (const char *name, int options);
 
+/* A wrapper for cplus_demangle_print.  Uses DMGL_PARAMS and
+   DMGL_ANSI, in addition to DMGL_VERBOSE.  */
+extern char *gdb_cplus_demangle_print (struct demangle_component *tree,
+				       int estimated_length,
+				       size_t *p_allocated_size);
+
 /* Find an instance of the character C in the string S that is outside
    of all parenthesis pairs, single-quoted strings, and double-quoted
    strings.  Also, ignore the char within a template name, like a ','
diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc b/gdb/testsuite/gdb.cp/break-std-string.cc
similarity index 100%
rename from gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
rename to gdb/testsuite/gdb.cp/break-std-string.cc
diff --git a/gdb/testsuite/gdb.cp/break-std-string.exp b/gdb/testsuite/gdb.cp/break-std-string.exp
new file mode 100644
index 00000000000..edfe4fb92ae
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/break-std-string.exp
@@ -0,0 +1,108 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test setting a breakpoint at "f(std::string)".
+#
+# GDB should be able to expand the std::string typedef, and then set
+# the breakpoint using the full-qualified and expanded parameter name.
+# std::string, std::istream, std::iostream, std::ostream are special
+# for the demangler, though, they have corresponding "standard
+# substitutions" elements.  The libiberty demangler only expands the
+# standard substitutions to their full non-typedef underlying type if
+# the DMGL_VERBOSE option is requested.  GDB didn't use to use that
+# option, and would instead prevent expansion of the std::string
+# typedefs at breakpoint-set type, such that the function name used
+# for function lookup would match with the "std::string" present in
+# the function's non-DMGL_VERBOSE demangled name.
+#
+# For example (DMGL_VERBOSE):
+#
+#  $ echo "_Z1fSs" | c++filt
+#  f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)
+#
+# vs (no DMGL_VERBOSE):
+#
+#  $ echo "_Z1fSs" | c++filt --no-verbose
+#  f(std::string)
+#
+# This design however broke setting a breakpoint at std:string when
+# the new libstdc++ C++11 ABI was introduced, as the "f(std::string)"
+# function's mangled name no longer uses a standard substitution for
+# std::string...
+#
+# I.e., with the libstdc++ C++11 ABI, we now have (and DMGL_VERBOSE
+# makes no difference):
+#
+#  $ echo _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE | c++filt 
+#  f(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
+#
+# So nowadays, GDB always uses DMGL_VERBOSE and no longer prevents
+# std::string (etc.) typedef expansion.  This test exercises both old
+# and new C++11 ABI for this reason.
+
+standard_testfile .cc
+
+if { [skip_cplus_tests] } { continue }
+
+# CXX11_ABI specifies the value to define _GLIBCXX_USE_CXX11_ABI as.
+
+proc test {cxx11_abi} {
+    global srcdir subdir srcfile binfile testfile
+
+    set options \
+	[list c++ debug additional_flags=-D_GLIBCXX_USE_CXX11_ABI=$cxx11_abi]
+    if { [gdb_compile \
+	      "${srcdir}/${subdir}/${srcfile}" "${binfile}-${cxx11_abi}.o" \
+	      object $options] != "" } {
+	untested "failed to compile"
+	return -1
+    }
+
+    clean_restart ${testfile}-${cxx11_abi}.o
+
+    gdb_test_no_output "set breakpoint pending off"
+
+    # So that "whatis" expands the C++ typedef.  Since we're debugging
+    # an .o file, GDB doesn't figure out were debugging C++ code and
+    # the current language when auto, is guessed as C.
+    gdb_test_no_output "set language c++"
+
+    # Get the type std::string is a typedef for.
+    set realtype ""
+    set type "std::string"
+    gdb_test_multiple "whatis /r $type" "" {
+	-re -wrap "type = (\[^\r\n\]+)" {
+	    set realtype $expect_out(1,string)
+	    gdb_assert \
+		{![string eq "$realtype" "$type"] && ![string eq "$realtype" ""]} \
+		$gdb_test_name
+	}
+    }
+
+    # Setting the breakpoint should still work even if GDB guesses the
+    # current language is C.  Make sure to exercise C and C++ even if
+    # GDB someday changes to autodetect the current language as C++.
+    foreach_with_prefix lang {"c" "c++" "auto"} {
+	gdb_breakpoint "f($type)" message
+
+	if { $realtype != "" } {
+	    gdb_breakpoint "f($realtype)" message
+	}
+    }
+}
+
+foreach_with_prefix cxx11_abi {0 1} {
+    test $cxx11_abi
+}
diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
deleted file mode 100644
index 14f11ddcf04..00000000000
--- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
+++ /dev/null
@@ -1,35 +0,0 @@
-# Copyright 2011-2022 Free Software Foundation, Inc.
-
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-# Test loading symbols from unrelocated C++ object files.
-
-standard_testfile .cc
-
-if { [skip_cplus_tests] } { continue }
-
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug}] != "" } {
-     untested "failed to compile"
-     return -1
-}
-
-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\.} \
-	 "DMGL_VERBOSE-demangled f(std::string) is not defined"

base-commit: 2e920d702b43c6d21ebd1e8a49c9e976a0d2cde6
-- 
2.36.0

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

[Pedro Alves]

On 2022-04-29 19:40, Pedro Alves wrote:
> Thanks for pointing at "ignore_typedefs" in cp-support.c, Keith.  So if I hack out
> that special casing for std::string, like in the patch below, then GDB replaces the
> typedef, and setting the breakpoint works.  However, that only works if we debug
> a fully linked binary for some reason...  If we debug just the .o file, then the symbol
> still isn't found...  That seems yet another bug somewhere.

The fully-linked vs .o file difference was because with just the .o file, there's no main function,
so GDB's language is set to "auto (currently c)".  While with a fully-linked binary, the language is
"auto (currently c++)".  When the language is C, the typedef is expanded to "class $realtype".
That "class " prefix is not part of the demangled name of the f function, so the breakpoint
setting fails.

Re: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE (was: Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test)

[Carl Love]

Pedro:

I think the patch may need to be re-generated.  It didn't apply cleanly
to a fresh gdb tree.  Specifically, the removal of the file
gdb/testsuite/gdb.cp/no-dmgl-verbose.exp complained about being
"Reversed (or previously applied)".  I had to manually remove the file.

Other than that, the new test  gdb/testsuite/gdb.cp/break-std-
string.exp runs.  I got 18 passes on Power 10.

                  Carl Love

Re: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE (was: Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test)

[Lancelot SIX]

Hi Pedro,

> I knew I wouldn't be able to sleep if I didn't give this a try, so here it goes...

Thanks for having a look at this!

I only had a quick look and have minor comments/questions below.

> From d298d3c78f2d19bb70b312674b5ef9e5dbce2fde Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 29 Apr 2022 23:21:18 +0100
> Subject: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE
> 
> Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
> ---
>  gdb/cp-name-parser.y                          |   5 +-
>  gdb/cp-support.c                              |  52 ++++++---
>  gdb/cp-support.h                              |  10 ++
>  ...no-dmgl-verbose.cc => break-std-string.cc} |   0
>  gdb/testsuite/gdb.cp/break-std-string.exp     | 108 ++++++++++++++++++
>  gdb/testsuite/gdb.cp/no-dmgl-verbose.exp      |  35 ------
>  6 files changed, 156 insertions(+), 54 deletions(-)
>  rename gdb/testsuite/gdb.cp/{no-dmgl-verbose.cc => break-std-string.cc} (100%)
>  create mode 100644 gdb/testsuite/gdb.cp/break-std-string.exp
>  delete mode 100644 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> 
> diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
> index 6410363c87f..ceb4c968935 100644
> --- a/gdb/cp-name-parser.y
> +++ b/gdb/cp-name-parser.y
> @@ -1959,8 +1959,7 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len)
>  {
>    size_t err;
>  
> -  char *res = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI,
> -				    result, estimated_len, &err);
> +  char *res = gdb_cplus_demangle_print (result, estimated_len, &err);
>    return gdb::unique_xmalloc_ptr<char> (res);
>  }
>  
> @@ -2060,7 +2059,7 @@ cp_print (struct demangle_component *result)
>    char *str;
>    size_t err = 0;
>  
> -  str = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI, result, 64, &err);
> +  str = gdb_cplus_demangle_print (result, 64, &err);
>    if (str == NULL)
>      return;
>  
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index f52055893d2..b3eb775f717 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -70,18 +70,16 @@ static void add_symbol_overload_list_qualified
>  
>  struct cmd_list_element *maint_cplus_cmd_list = NULL;
>  
> -/* A list of typedefs which should not be substituted by replace_typedefs.  */
> -static const char * const ignore_typedefs[] =
> -  {
> -    "std::istream", "std::iostream", "std::ostream", "std::string"
> -  };
> -
>  static void
>    replace_typedefs (struct demangle_parse_info *info,
>  		    struct demangle_component *ret_comp,
>  		    canonicalization_ftype *finder,
>  		    void *data);
>  
> +static struct demangle_component *
> +  gdb_cplus_demangle_v3_components (const char *mangled,
> +				    int options, void **mem);
> +
>  /* A convenience function to copy STRING into OBSTACK, returning a pointer
>     to the newly allocated string and saving the number of bytes saved in LEN.
>  
> @@ -146,13 +144,6 @@ inspect_type (struct demangle_parse_info *info,
>    memcpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len);
>    name[ret_comp->u.s_name.len] = '\0';
>  
> -  /* Ignore any typedefs that should not be substituted.  */
> -  for (const char *ignorable : ignore_typedefs)
> -    {
> -      if (strcmp (name, ignorable) == 0)
> -	return 0;
> -    }
> -
>    sym = NULL;
>  
>    try
> @@ -238,6 +229,14 @@ inspect_type (struct demangle_parse_info *info,
>  	  string_file buf;
>  	  try
>  	    {
> +	      /* If the current language is C, and type is a

Not sure about comment conventions, but shouldn't type be upper case
here?

> +		 struct/class, type_print prefixes the type name with
> +		 "struct" or "class", which we don't want when we're
> +		 expanding a C++ typedef.  Force language temporarily
> +		 to avoid it.  */
> +	      scoped_restore_current_language restore_language;
> +	      set_language (language_cplus);
> +
>  	      type_print (type, "", &buf, -1);
>  	    }
>  	  /* If type_print threw an exception, there is little point
> @@ -670,8 +669,8 @@ mangled_name_to_comp (const char *mangled_name, int options,
>      {
>        struct demangle_component *ret;
>  
> -      ret = cplus_demangle_v3_components (mangled_name,
> -					  options, memory);
> +      ret = gdb_cplus_demangle_v3_components (mangled_name,
> +					      options, memory);
>        if (ret)
>  	{
>  	  std::unique_ptr<demangle_parse_info> info (new demangle_parse_info);
> @@ -1635,7 +1634,7 @@ gdb_demangle (const char *name, int options)
>  #endif
>  
>    if (crash_signal == 0)
> -    result.reset (bfd_demangle (NULL, name, options));
> +    result.reset (bfd_demangle (NULL, name, options | DMGL_VERBOSE));
>  
>  #ifdef HAVE_WORKING_FORK
>    if (catch_demangler_crashes)
> @@ -1670,6 +1669,27 @@ gdb_demangle (const char *name, int options)
>  
>  /* See cp-support.h.  */
>  
> +char *
> +gdb_cplus_demangle_print (struct demangle_component *tree,
> +			  int estimated_length,
> +			  size_t *p_allocated_size)
> +{
> +  return cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI | DMGL_VERBOSE,
> +			       tree, estimated_length, p_allocated_size);
> +}
> +
> +/* A wrapper for cplus_demangle_v3_components that forces
> +   DMGL_VERBOSE.  */
> +
> +static struct demangle_component *
> +gdb_cplus_demangle_v3_components (const char *mangled,
> +				  int options, void **mem)
> +{
> +  return cplus_demangle_v3_components (mangled, options | DMGL_VERBOSE, mem);
> +}
> +
> +/* See cp-support.h.  */
> +
>  unsigned int
>  cp_search_name_hash (const char *search_name)
>  {
> diff --git a/gdb/cp-support.h b/gdb/cp-support.h
> index 4fbd53c8923..0a30e59d36d 100644
> --- a/gdb/cp-support.h
> +++ b/gdb/cp-support.h
> @@ -186,10 +186,20 @@ extern void cp_merge_demangle_parse_infos (struct demangle_parse_info *,
>  
>  extern struct cmd_list_element *maint_cplus_cmd_list;
>  
> +/* Wrappers for bfd and libiberty demangling entry points.  Note they
> +   all force DMGL_VERBOSE so that callers don't need to.  This is so
> +   that GDB consistently uses DMGL_VERBOSE throughout.  */
> +
>  /* A wrapper for bfd_demangle.  */
>  
>  gdb::unique_xmalloc_ptr<char> gdb_demangle (const char *name, int options);
>  
> +/* A wrapper for cplus_demangle_print.  Uses DMGL_PARAMS and
> +   DMGL_ANSI, in addition to DMGL_VERBOSE.  */
> +extern char *gdb_cplus_demangle_print (struct demangle_component *tree,
> +				       int estimated_length,
> +				       size_t *p_allocated_size);
> +
>  /* Find an instance of the character C in the string S that is outside
>     of all parenthesis pairs, single-quoted strings, and double-quoted
>     strings.  Also, ignore the char within a template name, like a ','
> diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc b/gdb/testsuite/gdb.cp/break-std-string.cc
> similarity index 100%
> rename from gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
> rename to gdb/testsuite/gdb.cp/break-std-string.cc
> diff --git a/gdb/testsuite/gdb.cp/break-std-string.exp b/gdb/testsuite/gdb.cp/break-std-string.exp
> new file mode 100644
> index 00000000000..edfe4fb92ae
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/break-std-string.exp
> @@ -0,0 +1,108 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test setting a breakpoint at "f(std::string)".
> +#
> +# GDB should be able to expand the std::string typedef, and then set
> +# the breakpoint using the full-qualified and expanded parameter name.

s/full-qualified/fully-qualified/ maybe?

> +# std::string, std::istream, std::iostream, std::ostream are special
> +# for the demangler, though, they have corresponding "standard
> +# substitutions" elements.  The libiberty demangler only expands the
> +# standard substitutions to their full non-typedef underlying type if
> +# the DMGL_VERBOSE option is requested.  GDB didn't use to use that
> +# option, and would instead prevent expansion of the std::string
> +# typedefs at breakpoint-set type, such that the function name used
> +# for function lookup would match with the "std::string" present in
> +# the function's non-DMGL_VERBOSE demangled name.
> +#
> +# For example (DMGL_VERBOSE):
> +#
> +#  $ echo "_Z1fSs" | c++filt
> +#  f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)
> +#
> +# vs (no DMGL_VERBOSE):
> +#
> +#  $ echo "_Z1fSs" | c++filt --no-verbose
> +#  f(std::string)
> +#
> +# This design however broke setting a breakpoint at std:string when
                                                          ^
One ':' is missing.

> +# the new libstdc++ C++11 ABI was introduced, as the "f(std::string)"
> +# function's mangled name no longer uses a standard substitution for
> +# std::string...
> +#
> +# I.e., with the libstdc++ C++11 ABI, we now have (and DMGL_VERBOSE
> +# makes no difference):
> +#
> +#  $ echo _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE | c++filt 

Extra space at the end of the line.

> +#  f(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
> +#
> +# So nowadays, GDB always uses DMGL_VERBOSE and no longer prevents
> +# std::string (etc.) typedef expansion.  This test exercises both old
> +# and new C++11 ABI for this reason.
> +
> +standard_testfile .cc
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +# CXX11_ABI specifies the value to define _GLIBCXX_USE_CXX11_ABI as.

I guess that this option is libstdc++ only, right?  When using another
standard library (libc++ for example), does this option have an impact?

It is probably not really an issue, though.  If this has no effect, we
just end up running the test twice with whatever ABI is used.  We still
want the tested behavior to be the same, it is just that we have test
names which do not really reflect what is tested.

Or maybe we assume libstdc++ in gdb.cp, I do not know.

> +
> +proc test {cxx11_abi} {
> +    global srcdir subdir srcfile binfile testfile
> +
> +    set options \
> +	[list c++ debug additional_flags=-D_GLIBCXX_USE_CXX11_ABI=$cxx11_abi]
> +    if { [gdb_compile \
> +	      "${srcdir}/${subdir}/${srcfile}" "${binfile}-${cxx11_abi}.o" \
> +	      object $options] != "" } {
> +	untested "failed to compile"
> +	return -1
> +    }
> +
> +    clean_restart ${testfile}-${cxx11_abi}.o
> +
> +    gdb_test_no_output "set breakpoint pending off"

Is this forced to OFF just in case `gdb_breakpoint` change in the future
to have the "allow-pending" behavior by default?  In the current state
of gdb_breakpoint, this is not necessary.

> +
> +    # So that "whatis" expands the C++ typedef.  Since we're debugging
> +    # an .o file, GDB doesn't figure out were debugging C++ code and

s/were/we’re/

Best,
Lancelot.

> +    # the current language when auto, is guessed as C.
> +    gdb_test_no_output "set language c++"
> +
> +    # Get the type std::string is a typedef for.
> +    set realtype ""
> +    set type "std::string"
> +    gdb_test_multiple "whatis /r $type" "" {
> +	-re -wrap "type = (\[^\r\n\]+)" {
> +	    set realtype $expect_out(1,string)
> +	    gdb_assert \
> +		{![string eq "$realtype" "$type"] && ![string eq "$realtype" ""]} \
> +		$gdb_test_name
> +	}
> +    }
> +
> +    # Setting the breakpoint should still work even if GDB guesses the
> +    # current language is C.  Make sure to exercise C and C++ even if
> +    # GDB someday changes to autodetect the current language as C++.
> +    foreach_with_prefix lang {"c" "c++" "auto"} {
> +	gdb_breakpoint "f($type)" message
> +
> +	if { $realtype != "" } {
> +	    gdb_breakpoint "f($realtype)" message
> +	}
> +    }
> +}
> +
> +foreach_with_prefix cxx11_abi {0 1} {
> +    test $cxx11_abi
> +}
> diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> deleted file mode 100644
> index 14f11ddcf04..00000000000
> --- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -# Copyright 2011-2022 Free Software Foundation, Inc.
> -
> -# This program is free software; you can redistribute it and/or modify
> -# it under the terms of the GNU General Public License as published by
> -# the Free Software Foundation; either version 3 of the License, or
> -# (at your option) any later version.
> -#
> -# This program is distributed in the hope that it will be useful,
> -# but WITHOUT ANY WARRANTY; without even the implied warranty of
> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> -# GNU General Public License for more details.
> -#
> -# You should have received a copy of the GNU General Public License
> -# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> -
> -# Test loading symbols from unrelocated C++ object files.
> -
> -standard_testfile .cc
> -
> -if { [skip_cplus_tests] } { continue }
> -
> -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug}] != "" } {
> -     untested "failed to compile"
> -     return -1
> -}
> -
> -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\.} \
> -	 "DMGL_VERBOSE-demangled f(std::string) is not defined"
> 
> base-commit: 2e920d702b43c6d21ebd1e8a49c9e976a0d2cde6
> -- 
> 2.36.0
> 

Re: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE (was: Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test)

[Pedro Alves]

On 2022-04-30 22:11, Lancelot SIX wrote:

>> @@ -238,6 +229,14 @@ inspect_type (struct demangle_parse_info *info,
>>  	  string_file buf;
>>  	  try
>>  	    {
>> +	      /* If the current language is C, and type is a
> 
> Not sure about comment conventions, but shouldn't type be upper case
> here?

Yes, fixed.

> 
>> +		 struct/class, type_print prefixes the type name with
>> +		 "struct" or "class", which we don't want when we're
>> +		 expanding a C++ typedef.  Force language temporarily
>> +		 to avoid it.  */
>> +	      scoped_restore_current_language restore_language;
>> +	      set_language (language_cplus);

Also, here we can use the symbol's language instead of hardcoding C++.

>> +
>>  	      type_print (type, "", &buf, -1);
>>  	    }
>>  	  /* If type_print threw an exception, there is little point
>> @@ -670,8 +669,8 @@ mangled_name_to_comp (const char *mangled_name, int options,
>>      {
>>        struct demangle_component *ret;
>>  
>> -      ret = cplus_demangle_v3_components (mangled_name,
>> -					  options, memory);
>> +      ret = gdb_cplus_demangle_v3_components (mangled_name,
>> +					      options, memory);
>>        if (ret)
>>  	{
>>  	  std::unique_ptr<demangle_parse_info> info (new demangle_parse_info);
>> @@ -1635,7 +1634,7 @@ gdb_demangle (const char *name, int options)
>>  #endif
>>  
>>    if (crash_signal == 0)
>> -    result.reset (bfd_demangle (NULL, name, options));
>> +    result.reset (bfd_demangle (NULL, name, options | DMGL_VERBOSE));
>>  
>>  #ifdef HAVE_WORKING_FORK
>>    if (catch_demangler_crashes)

...

>> +# Test setting a breakpoint at "f(std::string)".
>> +#
>> +# GDB should be able to expand the std::string typedef, and then set
>> +# the breakpoint using the full-qualified and expanded parameter name.
> 
> s/full-qualified/fully-qualified/ maybe?

Right, fixed.

> 
>> +# std::string, std::istream, std::iostream, std::ostream are special
>> +# for the demangler, though, they have corresponding "standard
>> +# substitutions" elements.  The libiberty demangler only expands the
>> +# standard substitutions to their full non-typedef underlying type if
>> +# the DMGL_VERBOSE option is requested.  GDB didn't use to use that
>> +# option, and would instead prevent expansion of the std::string
>> +# typedefs at breakpoint-set type, such that the function name used
>> +# for function lookup would match with the "std::string" present in
>> +# the function's non-DMGL_VERBOSE demangled name.
>> +#
>> +# For example (DMGL_VERBOSE):
>> +#
>> +#  $ echo "_Z1fSs" | c++filt
>> +#  f(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)
>> +#
>> +# vs (no DMGL_VERBOSE):
>> +#
>> +#  $ echo "_Z1fSs" | c++filt --no-verbose
>> +#  f(std::string)
>> +#
>> +# This design however broke setting a breakpoint at std:string when
>                                                           ^
> One ':' is missing.

Fixed.  And this should say "f(std::string)" instead of std::string.  Fix that
too.

> 
>> +# the new libstdc++ C++11 ABI was introduced, as the "f(std::string)"
>> +# function's mangled name no longer uses a standard substitution for
>> +# std::string...
>> +#
>> +# I.e., with the libstdc++ C++11 ABI, we now have (and DMGL_VERBOSE
>> +# makes no difference):
>> +#
>> +#  $ echo _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE | c++filt 
> 
> Extra space at the end of the line.

Fixed.

> 
>> +#  f(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
>> +#
>> +# So nowadays, GDB always uses DMGL_VERBOSE and no longer prevents
>> +# std::string (etc.) typedef expansion.  This test exercises both old
>> +# and new C++11 ABI for this reason.
>> +
>> +standard_testfile .cc
>> +
>> +if { [skip_cplus_tests] } { continue }
>> +
>> +# CXX11_ABI specifies the value to define _GLIBCXX_USE_CXX11_ABI as.
> 
> I guess that this option is libstdc++ only, right?  When using another
> standard library (libc++ for example), does this option have an impact?
> 
> It is probably not really an issue, though.  If this has no effect, we
> just end up running the test twice with whatever ABI is used.  We still
> want the tested behavior to be the same, it is just that we have test
> names which do not really reflect what is tested.

Right.  I've now mentioned this in a comment, and changed the prefix used in the
test names, to be the full name of the define to make this a bit more obvious:

 ...
 PASS: gdb.cp/break-std-string.exp: _GLIBCXX_USE_CXX11_ABI=0: lang=c: gdb_breakpoint: set breakpoint at f(std::string)
 ...
 PASS: gdb.cp/break-std-string.exp: _GLIBCXX_USE_CXX11_ABI=1: lang=c: gdb_breakpoint: set breakpoint at f(std::string)
 ...

> 
> Or maybe we assume libstdc++ in gdb.cp, I do not know.

We don't.

> 
>> +
>> +proc test {cxx11_abi} {
>> +    global srcdir subdir srcfile binfile testfile
>> +
>> +    set options \
>> +	[list c++ debug additional_flags=-D_GLIBCXX_USE_CXX11_ABI=$cxx11_abi]
>> +    if { [gdb_compile \
>> +	      "${srcdir}/${subdir}/${srcfile}" "${binfile}-${cxx11_abi}.o" \
>> +	      object $options] != "" } {
>> +	untested "failed to compile"
>> +	return -1
>> +    }
>> +
>> +    clean_restart ${testfile}-${cxx11_abi}.o
>> +
>> +    gdb_test_no_output "set breakpoint pending off"
> 
> Is this forced to OFF just in case `gdb_breakpoint` change in the future
> to have the "allow-pending" behavior by default?  In the current state
> of gdb_breakpoint, this is not necessary.

I had just blindly copied this from gdb.cp/no-dmgl-verbose.exp.

I removed it, as indeed it's not necessary.  Actually, I stopped using gdb_breakpoint
too.  There's no real advantage to using it here, and gdb_breakpoint is a bit too
lax with expected regexps to my liking when we're actually testing a break command
rather than setting a breakpoint to run to it to set up for some other test.

> 
>> +
>> +    # So that "whatis" expands the C++ typedef.  Since we're debugging
>> +    # an .o file, GDB doesn't figure out were debugging C++ code and
> 
> s/were/we’re/
> 

Thanks, fixed.

I'll send a v2 with these fixes once I manage to write an actual commit log.

Re: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE (was: Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test)

[Pedro Alves]

On 2022-05-02 16:46, Pedro Alves wrote:


> I'll send a v2 with these fixes once I manage to write an actual commit log.

Finally found a moment to do this.  I split the 'Make "b f(std::string)" even in C work' part
to a separate patch, and made it not rely on the current language.  So it is now a 3 part series.
You can find it here:

  [PATCH 0/3] Fix "b func(std::string)", DMGL_VERBOSE, gdb.cp/no-dmgl-verbose.exp
  https://sourceware.org/pipermail/gdb-patches/2022-May/188743.html

Comments more than welcome!