[PATCH 0/3] Fix "b func(std::string)", DMGL_VERBOSE, gdb.cp/no-dmgl-verbose.exp

[PATCH 0/3] Fix "b func(std::string)", DMGL_VERBOSE, gdb.cp/no-dmgl-verbose.exp

[Pedro Alves]

This series evolved from the discussion starting at:

  [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test
  https://sourceware.org/pipermail/gdb-patches/2022-April/188542.html

Turns out gdb.cp/no-dmgl-verbose.exp is right as long as GDB doesn't
use DMGL_VERBOSE (which it doesn't today), _and_ on libstdc++, it is
build for pre-C++11 ABI.

See "GCC5 and the C++11 ABI", at
<https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi>.

And "Dual ABI" at
<https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html>.

On systems with compilers that default to modern C++11 ABI
(practically everywhere these days), gdb.cp/no-dmgl-verbose.exp
exposes that "b func(std::string)" doesn't work any more.

This series fixes this, ensuring that the scenario works on both
pre-C++11, and C++11 ABIs.  It does this by switching GDB to use
DMGL_VERBOSE, and removing some related code from gdb/cp-support.c.

This is done in patch #1.

Patch #2 is preparatory work for patch #3.

Patch #3 is about making it so that "b f(std::string)" also works if
the current language is C.  I noticed that it doesn't work, because
the testcase debugs a .o file, and in such case, GDB doesn't figure
out that the program's language is C++, and so it assumes the language
is C.  Without doing "set language c++", "b f(std::string)" would
still fail.  But I think that should have not been necessary, I think
GDB should have been able to set the breakpoint even with "set
language c".

Tested on x86_64 GNU/Linux with no regressions.

Pedro Alves (3):
  Fix "b f(std::string)", always use DMGL_VERBOSE
  Always pass an explicit language down to c_type_print
  Fix "b f(std::string)" when current language is C

 gdb/ada-typeprint.c                           |   2 +-
 gdb/c-exp.y                                   |   1 +
 gdb/c-lang.c                                  |   8 +-
 gdb/c-lang.h                                  |  17 +--
 gdb/c-typeprint.c                             |  25 +---
 gdb/cp-name-parser.y                          |   5 +-
 gdb/cp-support.c                              |  54 ++++++---
 gdb/cp-support.h                              |  12 ++
 gdb/d-lang.c                                  |   2 +-
 gdb/gnu-v3-abi.c                              |   3 +-
 gdb/go-typeprint.c                            |   2 +-
 gdb/objc-lang.c                               |   2 +-
 gdb/opencl-lang.c                             |   2 +-
 gdb/rust-lang.c                               |   5 +-
 ...-dmgl-verbose.cc => break-f-std-string.cc} |   0
 gdb/testsuite/gdb.cp/break-f-std-string.exp   | 112 ++++++++++++++++++
 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp      |  35 ------
 17 files changed, 192 insertions(+), 95 deletions(-)
 rename gdb/testsuite/gdb.cp/{no-dmgl-verbose.cc => break-f-std-string.cc} (100%)
 create mode 100644 gdb/testsuite/gdb.cp/break-f-std-string.exp
 delete mode 100644 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp


base-commit: 8e1ada9e0bd4f57597b835b2d09b100d24c604d8
-- 
2.36.0

[PATCH 1/3] Fix "b f(std::string)", always use DMGL_VERBOSE

[Pedro Alves]

Currently, on any remotely modern GNU/Linux system,
gdb.cp/no-dmgl-verbose.exp fails like so:

  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
  testcase /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp completed in 0 seconds

This testcase was added back in 2011, here:

  [patch] Remove DMGL_VERBOSE
  https://sourceware.org/pipermail/gdb-patches/2011-June/083081.html

Back then, the testcase would pass cleanly.  It turns out that the
reason it fails today is that the testcase is exercising something in
GDB that only makes sense if the program is built for the pre-C++11
libstc++ ABI.  Back then the C++11 ABI didn't exist yet, but nowadays,
you need to compile with -D_GLIBCXX_USE_CXX11_ABI=0 to use the old
ABI.  See "Dual ABI" in the libstdc++ manual, at
<https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html>.

If we tweak the gdb.cp/no-dmgl-verbose.exp testcase to force the old
ABI with -D_GLIBCXX_USE_CXX11_ABI=0, then it passes cleanly.

So why is it that setting a breakpoint at "f(std::string)" fails with
modern ABI, but passes with old ABI?

This is where libiberty demangler's DMGL_VERBOSE option comes in.  The
Itanium ABI mangling scheme has a shorthand form for std::string (and
some other types).  See
<https://itanium-cxx-abi.github.io/cxx-abi/abi.html>:

  "In addition, the following catalog of abbreviations of the form "Sx" are used:

     <substitution> ::= St # ::std::
     <substitution> ::= Sa # ::std::allocator
     <substitution> ::= Sb # ::std::basic_string
     <substitution> ::= Ss # ::std::basic_string < char,
						   ::std::char_traits<char>,
						   ::std::allocator<char> >
     <substitution> ::= Si # ::std::basic_istream<char,  std::char_traits<char> >
     <substitution> ::= So # ::std::basic_ostream<char,  std::char_traits<char> >
     <substitution> ::= Sd # ::std::basic_iostream<char, std::char_traits<char> >
  "

When the libiberty demangler encounters such a abbreviation, by
default, it expands it to the user-friendly typedef "std::string",
"std::iostream", etc..  If OTOH DMGL_VERBOSE is specified, the
abbreviation is expanded to the underlying, non-typeded fullname
"std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
etc. as documented in the Itanium ABI, and pasted above.  You can see
the standard abbreviations/substitutions in
libiberty/cp-demangle.c:standard_subs.

Back before Jan's patch in 2011, there were parts of GDB that used
DMGL_VERBOSE, and others that did not, leading to mismatches.  The
solution back then was to stop using DMGL_VERBOSE throughout.

GDB has code in place to let users set a breakpoint at a function with
typedefs in its parameters, like "b f(uint32_t)".  Demangled function
names as they appear in the symbol tables almost (more on this is in a
bit) never have typedefs in them, so when processing "b f(uint32_t)"
GDB first replaces "uint32_t" for its underlying type, and then sets a
breakpoint on the resulting prototype, in this case "f(unsigned int)".

Now, if DMGL_VERBOSE is _not_ used, then the demangler demangles the
mangled name of a function such as "void f(std::string)" thas was
mangled using the standard abbreviations mentioned above really as:

  "void f(std::string)".

For example, the mangled name of "void f(std::string)" if you compile
with old pre-C++11 ABI is "_Z1fSs".  That uses the abbreviation "Ss",
so if you demangle that without DMGL_VERBOSE, you get:

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

while with DMGL_VERBOSE you'd get:

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

If, when the user sets a breakpoint at "f(std::string)", GDB would
replace the std::string typedef for its underlying type using the same
mechanism I mentioned for the "f(uint32_t)" example above, then GDB
would really try to set a breakpoint at "f(std::basic_string<char,
std::char_traits<char>, std::allocator<char> >)", and that would fail,
as the function symbol GDB knows about for that function, given no
DMGL_VERBOSE, is "f(std::string)".

For this reason, the code that expands typedefs in function parameter
names has an exception for std::string (and other standard
abbreviation types), such that "std::string" is never
typedef-expanded.

And here lies the problem when you try to do "b f(std::string)" with a
program compiled with the C++11 ABI.  In that case, std::string
expands to a different underlying type, like so:

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

and this symbol naturally mangles differently, as:

  $ echo _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE

and then because this doesn't use the shorthand mangling abbreviation
for "std::string" anymore, it always demangles as:

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

Now, when using the C++11 ABI, and you set a breakpoint at
"f(std::string)", GDB's typedefs-in-parameters expansion code hits the
exception for "std::string" and doesn't expand it, so the breakpoint
fails to be inserted, because the symbol that exists is really the

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

one, not "f(std::string)".

So to fix things for C++11 ABI, clearly we need to remove the
"std::string" exception from the typedef-in-parameters expansion
logic.  If we do just that, then "b f(std::string)" starts working
with the C++11 ABI.

However, if we do _just_ that, and nothing else, then we break
pre-C++11 ABI...

The solution is then to in addition switch GDB to always use
DMGL_VERBOSE.  If we do this, then pre-C++11 ABI symbols works the
same as C++11 ABI symbols overall -- the demangler expands the
standard abbreviation for "std::string" as "std::basic_string<char,
std::char_traits<char>, std::allocator<char> >" and letting GDB expand
the "std::string" typedef (etc.) too is no longer a problem.

To avoid getting in the situation where some parts of GDB use
DMGL_VERBOSE and others not, this patch adds wrappers around the
demangler's entry points that GDB uses, and makes those force
DMGL_VERBOSE.

The point of the gdb.cp/no-dmgl-verbose.exp testcase was to try to
ensure that DMGL_VERBOSE doesn't creep back 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"

This obviously no longer makes sense to have, since we now depend on
DMGL_VERBOSE.  So the patch replaces gdb.cp/no-dmgl-verbose.exp with a
new gdb.cp/break-f-std-string.exp testcase whose purpose is to make
sure that setting a breakpoint at "f(std::string)" works.  It
exercises both pre-C++11 ABI and C++11 ABI.

Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
---
 gdb/cp-name-parser.y                          |   5 +-
 gdb/cp-support.c                              |  44 +++++---
 gdb/cp-support.h                              |  12 ++
 ...-dmgl-verbose.cc => break-f-std-string.cc} |   0
 gdb/testsuite/gdb.cp/break-f-std-string.exp   | 105 ++++++++++++++++++
 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp      |  35 ------
 6 files changed, 147 insertions(+), 54 deletions(-)
 rename gdb/testsuite/gdb.cp/{no-dmgl-verbose.cc => break-f-std-string.cc} (100%)
 create mode 100644 gdb/testsuite/gdb.cp/break-f-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..1ddc2045808 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
@@ -670,8 +661,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 +1626,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 +1661,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..75e0d8cb0a0 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -186,10 +186,22 @@ 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 -- we want
+   libiberty's demangler to expand standard substitutions to their
+   full template name.  */
+
 /* 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-f-std-string.cc
similarity index 100%
rename from gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
rename to gdb/testsuite/gdb.cp/break-f-std-string.cc
diff --git a/gdb/testsuite/gdb.cp/break-f-std-string.exp b/gdb/testsuite/gdb.cp/break-f-std-string.exp
new file mode 100644
index 00000000000..0869912bb29
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/break-f-std-string.exp
@@ -0,0 +1,105 @@
+# 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 resulting name.  In the Itanium ABI's
+# mangling scheme, "std::string", "std::istream", "std::iostream",
+# "std::ostream" are special, though, they have corresponding standard
+# abbreviations.  The libiberty demangler only expands these standard
+# abbreviations to their full non-typedef underlying type if the
+# DMGL_VERBOSE option is requested.  By default it expands them to the
+# user-friendly "std::string", etc. typedefs.  GDB didn't use to use
+# that option, and would instead prevent expansion of the
+# "std::string" (etc.) standard-abbreviation typedefs at
+# breakpoint-set type, such that the function name used for function
+# lookup would match 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 broke setting a breakpoint at "f(std::string)" when the
+# 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
+# pre-C++11 and C++11 ABIs for this reason.  On non-libstdc++ systems
+# where _GLIBCXX_USE_CXX11_ABI has no effect, we just end up running
+# the test twice with whatever ABI is used.
+
+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
+
+    # Since we're debugging an .o file, GDB doesn't figure out we're
+    # 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.  We'll try to set a
+    # breakpoint using the expanded type too.
+    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"]} \
+		$gdb_test_name
+	}
+    }
+
+    gdb_test "break f($type)" "$srcfile, line $::decimal\\."
+
+    if { $realtype != "" } {
+	gdb_test "break f($realtype)" "$srcfile, line $::decimal\\."
+    }
+}
+
+foreach_with_prefix _GLIBCXX_USE_CXX11_ABI {0 1} {
+    test $_GLIBCXX_USE_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"
-- 
2.36.0

Re: [PATCH 1/3] Fix "b f(std::string)", always use DMGL_VERBOSE

[Keith Seitz]

On 5/5/22 11:50, Pedro Alves wrote:
> Currently, on any remotely modern GNU/Linux system,
> gdb.cp/no-dmgl-verbose.exp fails like so:

[snip]

What an amazingly complete history of this. Thank you!

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

I see nothing to make me pause. I think you should approve your patch.

Keith

Re: [PATCH 1/3] Fix "b f(std::string)", always use DMGL_VERBOSE

[Carl Love]

Pedro:

I pulled down a fresh copy of gdb and tried to apply the first patch. 
There were issues with patching the files:

 patch -p 1 < Fix__b_f_std_string__,_always_use_DMGL_VERBOSE_1.patch
(Stripping trailing CRs from patch; use --binary to disable.)
patching file gdb/cp-name-parser.y
(Stripping trailing CRs from patch; use --binary to disable.)
patching file gdb/cp-support.c
(Stripping trailing CRs from patch; use --binary to disable.)
patching file gdb/cp-support.h
patching file gdb/testsuite/gdb.cp/break-f-std-string.cc (renamed from
gdb/testsuite/gdb.cp/no-dmgl-verbose.cc)
(Stripping trailing CRs from patch; use --binary to disable.)
patching file gdb/testsuite/gdb.cp/break-f-std-string.exp
(Stripping trailing CRs from patch; use --binary to disable.)
patching file gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
Reversed (or previously applied) patch detected!  Assume -R? [n] n
Apply anyway? [n] n
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file
gdb/testsuite/gdb.cp/no-dmgl-verbose.exp.rej

The issue seems to be with deleting the file: 
    gdb/testsuite/gdb.cp/no-dmgl-verbose.exp

I manually deleted it.  I then used git to commit the patch to my local
git directory so I could add the second patch.

The second patch applied cleanly.

The third patch applied cleanly.

I ran the test gdb.cp/break-f-std-string.exp.  It runs without errors
on a Power 10 system.

  testcase gdb/testsuite/gdb.cp/break-f-std-string.exp completed in 1  
  seconds

                === gdb Summary ===

  # of expected passes            16

I also ran the full regression suite with and without the patch.  No
regression failures were found.

Other than the patch not cleanly removing gdb/testsuite/gdb.cp/no-dmgl-
verbose.exp it all looks good.

Thanks for fixing this test.

                           Carl Love

Re: [PATCH 1/3] Fix "b f(std::string)", always use DMGL_VERBOSE

[Pedro Alves]

On 2022-05-05 22:49, Carl Love wrote:
> Pedro:
> 
> I pulled down a fresh copy of gdb and tried to apply the first patch. 
> There were issues with patching the files:
> 
>  patch -p 1 < Fix__b_f_std_string__,_always_use_DMGL_VERBOSE_1.patch
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file gdb/cp-name-parser.y
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file gdb/cp-support.c
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file gdb/cp-support.h
> patching file gdb/testsuite/gdb.cp/break-f-std-string.cc (renamed from
> gdb/testsuite/gdb.cp/no-dmgl-verbose.cc)
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file gdb/testsuite/gdb.cp/break-f-std-string.exp
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> Reversed (or previously applied) patch detected!  Assume -R? [n] n
> Apply anyway? [n] n
> Skipping patch.
> 1 out of 1 hunk ignored -- saving rejects to file
> gdb/testsuite/gdb.cp/no-dmgl-verbose.exp.rej
> 
> The issue seems to be with deleting the file: 
>     gdb/testsuite/gdb.cp/no-dmgl-verbose.exp
> 
> I manually deleted it.  I then used git to commit the patch to my local
> git directory so I could add the second patch.

This looks like something on your end.  I sent the patches using git send-email,
which typically is the best way to send emails in the right format.  To check,
I downloaded the whole email from the copy I got via the list (in Thunderbird for
example, just save the email to a file), and used "git am", and it applied cleanly:

 $ git reset --hard upstream/master
 ...
 $ git am /tmp/patch1.mbox
 Applying: Fix "b f(std::string)", always use DMGL_VERBOSE

As a second experiment, I also tried copy/pasting the patch to a patch file,
and apply that with "patch" as you're doing, and it applies cleanly:

 $ git reset --hard upstream/master
 ...
 $ patch  -p1 -i /tmp/patch.diff
 patching file gdb/cp-name-parser.y
 patching file gdb/cp-support.c
 patching file gdb/cp-support.h
 patching file gdb/testsuite/gdb.cp/break-f-std-string.cc (renamed from gdb/testsuite/gdb.cp/no-dmgl-verbose.cc)
 patching file gdb/testsuite/gdb.cp/break-f-std-string.exp
 patching file gdb/testsuite/gdb.cp/no-dmgl-verbose.exp

> 
> The second patch applied cleanly.
> 
> The third patch applied cleanly.
> 
> I ran the test gdb.cp/break-f-std-string.exp.  It runs without errors
> on a Power 10 system.
> 
>   testcase gdb/testsuite/gdb.cp/break-f-std-string.exp completed in 1  
>   seconds
> 
>                 === gdb Summary ===
> 
>   # of expected passes            16
> 
> I also ran the full regression suite with and without the patch.  No
> regression failures were found.
> 
> Other than the patch not cleanly removing gdb/testsuite/gdb.cp/no-dmgl-
> verbose.exp it all looks good.
> 
> Thanks for fixing this test.

Thanks for testing.

Re: [PATCH 1/3] Fix "b f(std::string)", always use DMGL_VERBOSE

[Lancelot SIX]

Hi,

I have minor comments inlined below.

> [...]
>
> When the libiberty demangler encounters such a abbreviation, by
> default, it expands it to the user-friendly typedef "std::string",
> "std::iostream", etc..  If OTOH DMGL_VERBOSE is specified, the
> abbreviation is expanded to the underlying, non-typeded fullname

s/non-typeded/non-typedefed/ ?

> [...]
> 
> Now, if DMGL_VERBOSE is _not_ used, then the demangler demangles the
> mangled name of a function such as "void f(std::string)" thas was

s/thas/that/

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

The comment for this function is:

    /* Convert RESULT to a string.  The return value is allocated
       using xmalloc.  ESTIMATED_LEN is used only as a guide to the
       length of the result.  This functions handles a few cases that
       cplus_demangle_print does not, specifically the global destructor
       and constructor labels.  */

The last sentence (which references cplus_demangle_print) is not
accurate anymore.  This was out of sync before your change, but while at
it, do you mind updating the comment?

>  {
>    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);
>  }

Best,
Lancelot.

Re: [PATCH 1/3] Fix "b f(std::string)", always use DMGL_VERBOSE

[Pedro Alves]

On 2022-05-06 09:10, Lancelot SIX wrote:
> Hi,
> 
> I have minor comments inlined below.
> 
>> [...]
>>
>> When the libiberty demangler encounters such a abbreviation, by
>> default, it expands it to the user-friendly typedef "std::string",
>> "std::iostream", etc..  If OTOH DMGL_VERBOSE is specified, the
>> abbreviation is expanded to the underlying, non-typeded fullname
> 
> s/non-typeded/non-typedefed/ ?

Fixed.

> 
>> [...]
>>
>> Now, if DMGL_VERBOSE is _not_ used, then the demangler demangles the
>> mangled name of a function such as "void f(std::string)" thas was
> 
> s/thas/that/

Fixed.

> 
>> 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)
> 
> The comment for this function is:
> 
>     /* Convert RESULT to a string.  The return value is allocated
>        using xmalloc.  ESTIMATED_LEN is used only as a guide to the
>        length of the result.  This functions handles a few cases that
>        cplus_demangle_print does not, specifically the global destructor
>        and constructor labels.  */
> 
> The last sentence (which references cplus_demangle_print) is not
> accurate anymore.  This was out of sync before your change, but while at
> it, do you mind updating the comment?
> 
>>  {
>>    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);
>>  }
> 

Hmm.  I added back an "options" parameter to gdb_cplus_demangle_print, keeping
it as a pure wrapper for cplus_demangle_print + DMGL_VERBOSE, otherwise cp_comp_to_string
would just  look like yet another wrapper around gdb_cplus_demangle_print, just for
unique_xmalloc_ptr, which doesn't feel right.

I then updated the description of cp_comp_to_string removing the stale bits and moved
description to the header file.

Here's the updated patch.

From 823135ae122a5de2d284c527395f7d06c3669f50 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 f(std::string)", always use DMGL_VERBOSE

Currently, on any remotely modern GNU/Linux system,
gdb.cp/no-dmgl-verbose.exp fails like so:

  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

This testcase was added back in 2011, here:

  [patch] Remove DMGL_VERBOSE
  https://sourceware.org/pipermail/gdb-patches/2011-June/083081.html

Back then, the testcase would pass cleanly.  It turns out that the
reason it fails today is that the testcase is exercising something in
GDB that only makes sense if the program is built for the pre-C++11
libstc++ ABI.  Back then the C++11 ABI didn't exist yet, but nowadays,
you need to compile with -D_GLIBCXX_USE_CXX11_ABI=0 to use the old
ABI.  See "Dual ABI" in the libstdc++ manual, at
<https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html>.

If we tweak the gdb.cp/no-dmgl-verbose.exp testcase to force the old
ABI with -D_GLIBCXX_USE_CXX11_ABI=0, then it passes cleanly.

So why is it that setting a breakpoint at "f(std::string)" fails with
modern ABI, but passes with old ABI?

This is where libiberty demangler's DMGL_VERBOSE option comes in.  The
Itanium ABI mangling scheme has a shorthand form for std::string (and
some other types).  See
<https://itanium-cxx-abi.github.io/cxx-abi/abi.html>:

  "In addition, the following catalog of abbreviations of the form "Sx" are used:

     <substitution> ::= St # ::std::
     <substitution> ::= Sa # ::std::allocator
     <substitution> ::= Sb # ::std::basic_string
     <substitution> ::= Ss # ::std::basic_string < char,
						   ::std::char_traits<char>,
						   ::std::allocator<char> >
     <substitution> ::= Si # ::std::basic_istream<char,  std::char_traits<char> >
     <substitution> ::= So # ::std::basic_ostream<char,  std::char_traits<char> >
     <substitution> ::= Sd # ::std::basic_iostream<char, std::char_traits<char> >
  "

When the libiberty demangler encounters such a abbreviation, by
default, it expands it to the user-friendly typedef "std::string",
"std::iostream", etc..  If OTOH DMGL_VERBOSE is specified, the
abbreviation is expanded to the underlying, non-typedefed fullname
"std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
etc. as documented in the Itanium ABI, and pasted above.  You can see
the standard abbreviations/substitutions in
libiberty/cp-demangle.c:standard_subs.

Back before Jan's patch in 2011, there were parts of GDB that used
DMGL_VERBOSE, and others that did not, leading to mismatches.  The
solution back then was to stop using DMGL_VERBOSE throughout.

GDB has code in place to let users set a breakpoint at a function with
typedefs in its parameters, like "b f(uint32_t)".  Demangled function
names as they appear in the symbol tables almost (more on this is in a
bit) never have typedefs in them, so when processing "b f(uint32_t)"
GDB first replaces "uint32_t" for its underlying type, and then sets a
breakpoint on the resulting prototype, in this case "f(unsigned int)".

Now, if DMGL_VERBOSE is _not_ used, then the demangler demangles the
mangled name of a function such as "void f(std::string)" that was
mangled using the standard abbreviations mentioned above really as:

  "void f(std::string)".

For example, the mangled name of "void f(std::string)" if you compile
with old pre-C++11 ABI is "_Z1fSs".  That uses the abbreviation "Ss",
so if you demangle that without DMGL_VERBOSE, you get:

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

while with DMGL_VERBOSE you'd get:

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

If, when the user sets a breakpoint at "f(std::string)", GDB would
replace the std::string typedef for its underlying type using the same
mechanism I mentioned for the "f(uint32_t)" example above, then GDB
would really try to set a breakpoint at "f(std::basic_string<char,
std::char_traits<char>, std::allocator<char> >)", and that would fail,
as the function symbol GDB knows about for that function, given no
DMGL_VERBOSE, is "f(std::string)".

For this reason, the code that expands typedefs in function parameter
names has an exception for std::string (and other standard
abbreviation types), such that "std::string" is never
typedef-expanded.

And here lies the problem when you try to do "b f(std::string)" with a
program compiled with the C++11 ABI.  In that case, std::string
expands to a different underlying type, like so:

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

and this symbol naturally mangles differently, as:

  _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE

and then because this doesn't use the shorthand mangling abbreviation
for "std::string" anymore, it always demangles as:

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

Now, when using the C++11 ABI, and you set a breakpoint at
"f(std::string)", GDB's typedefs-in-parameters expansion code hits the
exception for "std::string" and doesn't expand it, so the breakpoint
fails to be inserted, because the symbol that exists is really the

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

one, not "f(std::string)".

So to fix things for C++11 ABI, clearly we need to remove the
"std::string" exception from the typedef-in-parameters expansion
logic.  If we do just that, then "b f(std::string)" starts working
with the C++11 ABI.

However, if we do _just_ that, and nothing else, then we break
pre-C++11 ABI...

The solution is then to in addition switch GDB to always use
DMGL_VERBOSE.  If we do this, then pre-C++11 ABI symbols works the
same as C++11 ABI symbols overall -- the demangler expands the
standard abbreviation for "std::string" as "std::basic_string<char,
std::char_traits<char>, std::allocator<char> >" and letting GDB expand
the "std::string" typedef (etc.) too is no longer a problem.

To avoid getting in the situation where some parts of GDB use
DMGL_VERBOSE and others not, this patch adds wrappers around the
demangler's entry points that GDB uses, and makes those force
DMGL_VERBOSE.

The point of the gdb.cp/no-dmgl-verbose.exp testcase was to try to
ensure that DMGL_VERBOSE doesn't creep back 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"

This obviously no longer makes sense to have, since we now depend on
DMGL_VERBOSE.  So the patch replaces gdb.cp/no-dmgl-verbose.exp with a
new gdb.cp/break-f-std-string.exp testcase whose purpose is to make
sure that setting a breakpoint at "f(std::string)" works.  It
exercises both pre-C++11 ABI and C++11 ABI.

Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
---
 gdb/cp-name-parser.y                          |  12 +-
 gdb/cp-support.c                              |  45 +++++---
 gdb/cp-support.h                              |  16 +++
 ...-dmgl-verbose.cc => break-f-std-string.cc} |   0
 gdb/testsuite/gdb.cp/break-f-std-string.exp   | 105 ++++++++++++++++++
 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp      |  35 ------
 6 files changed, 154 insertions(+), 59 deletions(-)
 rename gdb/testsuite/gdb.cp/{no-dmgl-verbose.cc => break-f-std-string.cc} (100%)
 create mode 100644 gdb/testsuite/gdb.cp/break-f-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..4d33e7d4a07 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -1948,19 +1948,15 @@ allocate_info (void)
   return info;
 }
 
-/* Convert RESULT to a string.  The return value is allocated
-   using xmalloc.  ESTIMATED_LEN is used only as a guide to the
-   length of the result.  This functions handles a few cases that
-   cplus_demangle_print does not, specifically the global destructor
-   and constructor labels.  */
+/* See cp-support.h.  */
 
 gdb::unique_xmalloc_ptr<char>
 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 (DMGL_PARAMS | DMGL_ANSI,
+					result, estimated_len, &err);
   return gdb::unique_xmalloc_ptr<char> (res);
 }
 
@@ -2060,7 +2056,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..71c14635e38 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
@@ -670,8 +661,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 +1626,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 +1661,28 @@ gdb_demangle (const char *name, int options)
 
 /* See cp-support.h.  */
 
+char *
+gdb_cplus_demangle_print (int options,
+			  struct demangle_component *tree,
+			  int estimated_length,
+			  size_t *p_allocated_size)
+{
+  return cplus_demangle_print (options | 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..2e0ad50d2b9 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -175,6 +175,9 @@ struct type *cp_find_type_baseclass_by_name (struct type *parent_type,
 extern std::unique_ptr<demangle_parse_info> cp_demangled_name_to_comp
      (const char *demangled_name, std::string *errmsg);
 
+/* Convert RESULT to a string.  ESTIMATED_LEN is used only as a guide
+   to the length of the result.  */
+
 extern gdb::unique_xmalloc_ptr<char> cp_comp_to_string
   (struct demangle_component *result, int estimated_len);
 
@@ -186,10 +189,23 @@ 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 -- we want
+   libiberty's demangler to expand standard substitutions to their
+   full template name.  */
+
 /* A wrapper for bfd_demangle.  */
 
 gdb::unique_xmalloc_ptr<char> gdb_demangle (const char *name, int options);
 
+/* A wrapper for cplus_demangle_print.  */
+
+extern char *gdb_cplus_demangle_print (int options,
+				       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-f-std-string.cc
similarity index 100%
rename from gdb/testsuite/gdb.cp/no-dmgl-verbose.cc
rename to gdb/testsuite/gdb.cp/break-f-std-string.cc
diff --git a/gdb/testsuite/gdb.cp/break-f-std-string.exp b/gdb/testsuite/gdb.cp/break-f-std-string.exp
new file mode 100644
index 00000000000..0869912bb29
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/break-f-std-string.exp
@@ -0,0 +1,105 @@
+# 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 resulting name.  In the Itanium ABI's
+# mangling scheme, "std::string", "std::istream", "std::iostream",
+# "std::ostream" are special, though, they have corresponding standard
+# abbreviations.  The libiberty demangler only expands these standard
+# abbreviations to their full non-typedef underlying type if the
+# DMGL_VERBOSE option is requested.  By default it expands them to the
+# user-friendly "std::string", etc. typedefs.  GDB didn't use to use
+# that option, and would instead prevent expansion of the
+# "std::string" (etc.) standard-abbreviation typedefs at
+# breakpoint-set type, such that the function name used for function
+# lookup would match 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 broke setting a breakpoint at "f(std::string)" when the
+# 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
+# pre-C++11 and C++11 ABIs for this reason.  On non-libstdc++ systems
+# where _GLIBCXX_USE_CXX11_ABI has no effect, we just end up running
+# the test twice with whatever ABI is used.
+
+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
+
+    # Since we're debugging an .o file, GDB doesn't figure out we're
+    # 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.  We'll try to set a
+    # breakpoint using the expanded type too.
+    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"]} \
+		$gdb_test_name
+	}
+    }
+
+    gdb_test "break f($type)" "$srcfile, line $::decimal\\."
+
+    if { $realtype != "" } {
+	gdb_test "break f($realtype)" "$srcfile, line $::decimal\\."
+    }
+}
+
+foreach_with_prefix _GLIBCXX_USE_CXX11_ABI {0 1} {
+    test $_GLIBCXX_USE_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: 0ee8858e7aeca5ba5f702204daad2ddd290ef229
-- 
2.36.0

[PATCH 2/3] Always pass an explicit language down to c_type_print

[Pedro Alves]

The next patch will want to do language->print_type(type, ...), to
print a type in a given language, avoiding a dependency on the current
language.  That doesn't work correctly currently, however, because
most language implementations of language_defn::print_type call
c_print_type without passing down the language.  There are two
overloads of c_print_type, one that takes a language, and one that
does not.  The one that does not uses the current language, defeating
the point of calling language->print_type()...

This commit removes the c_print_type overload that does not take a
language, and adjusts the codebase throughout to always pass down a
language.  In most places, there's already an enum language handy.
language_defn::print_type implementations naturally pass down
this->la_language.  In a couple spots, like in ada-typeprint.c and
rust-lang.c there's no enum language handy, but the code is written
for a specific language, so we just hardcode the language.

In gnuv3_print_method_ptr, I wasn't sure whether we could hardcode C++
here, and we don't have an enum language handy, so I made it use the
current language, just like today.  Can always be improved later.

Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
---
 gdb/ada-typeprint.c |  2 +-
 gdb/c-exp.y         |  1 +
 gdb/c-lang.c        |  8 ++++----
 gdb/c-lang.h        | 17 +++++++++--------
 gdb/c-typeprint.c   | 25 +++++--------------------
 gdb/d-lang.c        |  2 +-
 gdb/gnu-v3-abi.c    |  3 ++-
 gdb/go-typeprint.c  |  2 +-
 gdb/objc-lang.c     |  2 +-
 gdb/opencl-lang.c   |  2 +-
 gdb/rust-lang.c     |  5 +++--
 11 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index 0eb95cb0a73..05ffb8b8331 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -981,7 +981,7 @@ ada_print_type (struct type *type0, const char *varstring,
       {
       default:
 	gdb_printf (stream, "<");
-	c_print_type (type, "", stream, show, level, flags);
+	c_print_type (type, "", stream, show, level, language_ada, flags);
 	gdb_printf (stream, ">");
 	break;
       case TYPE_CODE_PTR:
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 517ab42b229..72f8dd32d93 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1783,6 +1783,7 @@ oper:	OPERATOR NEW
 			{
 			  string_file buf;
 			  c_print_type ($2, NULL, &buf, -1, 0,
+					pstate->language ()->la_language,
 					&type_print_raw_options);
 			  std::string name = buf.release ();
 
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index be9ee073f58..76896352954 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -820,7 +820,7 @@ class c_language : public language_defn
 		   struct ui_file *stream, int show, int level,
 		   const struct type_print_options *flags) const override
   {
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
@@ -966,7 +966,7 @@ class cplus_language : public language_defn
 		   struct ui_file *stream, int show, int level,
 		   const struct type_print_options *flags) const override
   {
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
@@ -1066,7 +1066,7 @@ class asm_language : public language_defn
 		   struct ui_file *stream, int show, int level,
 		   const struct type_print_options *flags) const override
   {
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
@@ -1118,7 +1118,7 @@ class minimal_language : public language_defn
 		   struct ui_file *stream, int show, int level,
 		   const struct type_print_options *flags) const override
   {
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
diff --git a/gdb/c-lang.h b/gdb/c-lang.h
index 46e562df055..c81c3bbc3b8 100644
--- a/gdb/c-lang.h
+++ b/gdb/c-lang.h
@@ -65,16 +65,17 @@ extern int c_parse (struct parser_state *);
 extern int c_parse_escape (const char **, struct obstack *);
 
 /* Defined in c-typeprint.c */
-extern void c_print_type (struct type *, const char *,
-			  struct ui_file *, int, int,
-			  const struct type_print_options *);
 
-/* Print a type but allow the precise language to be specified.  */
+/* LEVEL is the depth to indent lines by.  Allows the precise language
+   to be specified, because many languages defer to C type
+   printing.  */
 
-extern void c_print_type (struct type *, const char *,
-			  struct ui_file *, int, int,
-			  enum language,
-			  const struct type_print_options *);
+extern void c_print_type (struct type *type,
+			  const char *varstring,
+			  struct ui_file *stream,
+			  int show, int level,
+			  enum language language,
+			  const struct type_print_options *flags);
 
 extern void c_print_typedef (struct type *,
 			     struct symbol *,
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 3425c829e3f..4f45475dc4a 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -163,22 +163,6 @@ c_print_type_1 (struct type *type,
     }
 }
 
-/* LEVEL is the depth to indent lines by.  */
-
-void
-c_print_type (struct type *type,
-	      const char *varstring,
-	      struct ui_file *stream,
-	      int show, int level,
-	      const struct type_print_options *flags)
-{
-  struct print_offset_data podata (flags);
-
-  c_print_type_1 (type, varstring, stream, show, level,
-		  current_language->la_language, flags, &podata);
-}
-
-
 /* See c-lang.h.  */
 
 void
@@ -303,7 +287,7 @@ cp_type_print_method_args (struct type *mtype, const char *prefix,
 	  if (FIELD_ARTIFICIAL (arg))
 	    continue;
 
-	  c_print_type (arg.type (), "", stream, 0, 0, flags);
+	  c_print_type (arg.type (), "", stream, 0, 0, language, flags);
 
 	  if (i == nargs && varargs)
 	    gdb_printf (stream, ", ...");
@@ -872,7 +856,8 @@ c_type_print_varspec_suffix (struct type *type,
 
 static void
 c_type_print_template_args (const struct type_print_options *flags,
-			    struct type *type, struct ui_file *stream)
+			    struct type *type, struct ui_file *stream,
+			    enum language language)
 {
   int first = 1, i;
 
@@ -899,7 +884,7 @@ c_type_print_template_args (const struct type_print_options *flags,
 	  gdb_printf (stream, "%s = ", sym->linkage_name ());
 	}
 
-      c_print_type (sym->type (), "", stream, -1, 0, flags);
+      c_print_type (sym->type (), "", stream, -1, 0, language, flags);
     }
 
   if (!first)
@@ -1094,7 +1079,7 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
       struct type *basetype;
       int vptr_fieldno;
 
-      c_type_print_template_args (&local_flags, type, stream);
+      c_type_print_template_args (&local_flags, type, stream, language);
 
       /* Add in template parameters when printing derivation info.  */
       if (local_flags.local_typedefs != NULL)
diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index ec4a80a3223..ce6dc058412 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -148,7 +148,7 @@ class d_language : public language_defn
 		   struct ui_file *stream, int show, int level,
 		   const struct type_print_options *flags) const override
   {
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 7aac0ca4f9c..953645e7411 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -659,7 +659,8 @@ gnuv3_print_method_ptr (const gdb_byte *contents,
     {
       /* Found a non-virtual function: print out the type.  */
       gdb_puts ("(", stream);
-      c_print_type (type, "", stream, -1, 0, &type_print_raw_options);
+      c_print_type (type, "", stream, -1, 0, current_language->la_language,
+		    &type_print_raw_options);
       gdb_puts (") ", stream);
     }
 
diff --git a/gdb/go-typeprint.c b/gdb/go-typeprint.c
index f8f155fbb64..4995ac39186 100644
--- a/gdb/go-typeprint.c
+++ b/gdb/go-typeprint.c
@@ -59,5 +59,5 @@ go_language::print_type (struct type *type, const char *varstring,
     }
 
   /* Punt the rest to C for now.  */
-  c_print_type (type, varstring, stream, show, level, flags);
+  c_print_type (type, varstring, stream, show, level, la_language, flags);
 }
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 00c362c5c9c..37008da529a 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -270,7 +270,7 @@ class objc_language : public language_defn
 		   struct ui_file *stream, int show, int level,
 		   const struct type_print_options *flags) const override
   {
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 5145cd4062e..1034d1c91fe 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -969,7 +969,7 @@ class opencl_language : public language_defn
 	  show = 0;
       }
 
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
 
   /* See language.h.  */
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index bf8dba99ecd..746f565947b 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -669,7 +669,7 @@ rust_print_struct_def (struct type *type, const char *varstring,
 
   /* If we see a base class, delegate to C.  */
   if (TYPE_N_BASECLASSES (type) > 0)
-    c_print_type (type, varstring, stream, show, level, flags);
+    c_print_type (type, varstring, stream, show, level, language_rust, flags);
 
   if (flags->print_offsets)
     {
@@ -922,7 +922,8 @@ rust_internal_print_type (struct type *type, const char *varstring,
 
     default:
     c_printer:
-      c_print_type (type, varstring, stream, show, level, flags);
+      c_print_type (type, varstring, stream, show, level, language_rust,
+		    flags);
     }
 }
 
-- 
2.36.0

Re: [PATCH 2/3] Always pass an explicit language down to c_type_print

[Keith Seitz]

On 5/5/22 11:50, Pedro Alves wrote:

> In gnuv3_print_method_ptr, I wasn't sure whether we could hardcode C++
> here, and we don't have an enum language handy, so I made it use the
> current language, just like today.  Can always be improved later.

I like this philosophy. :-)

I just have one comment for you to double-check. Otherwise, I also
encourage you to approve your patch.

Keith

> Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
> ---
>   gdb/ada-typeprint.c |  2 +-
>   gdb/c-exp.y         |  1 +
>   gdb/c-lang.c        |  8 ++++----
>   gdb/c-lang.h        | 17 +++++++++--------
>   gdb/c-typeprint.c   | 25 +++++--------------------
>   gdb/d-lang.c        |  2 +-
>   gdb/gnu-v3-abi.c    |  3 ++-
>   gdb/go-typeprint.c  |  2 +-
>   gdb/objc-lang.c     |  2 +-
>   gdb/opencl-lang.c   |  2 +-
>   gdb/rust-lang.c     |  5 +++--
>   11 files changed, 29 insertions(+), 40 deletions(-)
> 
> diff --git a/gdb/c-lang.h b/gdb/c-lang.h
> index 46e562df055..c81c3bbc3b8 100644
> --- a/gdb/c-lang.h
> +++ b/gdb/c-lang.h
> @@ -65,16 +65,17 @@ extern int c_parse (struct parser_state *);
>   extern int c_parse_escape (const char **, struct obstack *);
>   
>   /* Defined in c-typeprint.c */
> -extern void c_print_type (struct type *, const char *,
> -			  struct ui_file *, int, int,
> -			  const struct type_print_options *);
>   
> -/* Print a type but allow the precise language to be specified.  */
> +/* LEVEL is the depth to indent lines by.  Allows the precise language
> +   to be specified, because many languages defer to C type
> +   printing.  */

The actual description of the function (if brief) has been removed. The new
comment explains the LEVEL argument, but the decl only contains types. A
casual reader (like myself) would have to go figure out to which argument this
refers.

I have to ask: Is this what you intended? It seems like a cut-n-paste
error?

> -extern void c_print_type (struct type *, const char *,
> -			  struct ui_file *, int, int,
> -			  enum language,
> -			  const struct type_print_options *);
> +extern void c_print_type (struct type *type,
> +			  const char *varstring,
> +			  struct ui_file *stream,
> +			  int show, int level,
> +			  enum language language,
> +			  const struct type_print_options *flags);
>   
>   extern void c_print_typedef (struct type *,
>   			     struct symbol *,

Re: [PATCH 2/3] Always pass an explicit language down to c_type_print

[Pedro Alves]

On 2022-05-05 21:47, Keith Seitz wrote:
> On 5/5/22 11:50, Pedro Alves wrote:

>> diff --git a/gdb/c-lang.h b/gdb/c-lang.h
>> index 46e562df055..c81c3bbc3b8 100644
>> --- a/gdb/c-lang.h
>> +++ b/gdb/c-lang.h
>> @@ -65,16 +65,17 @@ extern int c_parse (struct parser_state *);
>>   extern int c_parse_escape (const char **, struct obstack *);
>>     /* Defined in c-typeprint.c */
>> -extern void c_print_type (struct type *, const char *,
>> -              struct ui_file *, int, int,
>> -              const struct type_print_options *);
>>   -/* Print a type but allow the precise language to be specified.  */
>> +/* LEVEL is the depth to indent lines by.  Allows the precise language
>> +   to be specified, because many languages defer to C type
>> +   printing.  */
> 
> The actual description of the function (if brief) has been removed. The new
> comment explains the LEVEL argument, but the decl only contains types. A
> casual reader (like myself) would have to go figure out to which argument this
> refers.
> 
> I have to ask: Is this what you intended? It seems like a cut-n-paste
> error?

Yeah, I had just blindly moved the comment from the .c file to the .h file:

 -/* LEVEL is the depth to indent lines by.  */
 -
 -void
 -c_print_type (struct type *type,
 -             const char *varstring,

If it was good there, it must still be good in the .h file!  :-)  1/2 kidding.

The "Print a type but allow the precise language to be specified."
comment was saying that that overload was the same as the one without
the language, except it takes a language.  I dropped that one, and didn't 
even realize that the comment in the other overload didn't refer to
type printing.

I've changed this now to look like this:

--- c/gdb/c-lang.h
+++ w/gdb/c-lang.h
@@ -65,16 +65,17 @@ extern int c_parse (struct parser_state *);
 extern int c_parse_escape (const char **, struct obstack *);
 
 /* Defined in c-typeprint.c */
-extern void c_print_type (struct type *, const char *,
-                         struct ui_file *, int, int,
-                         const struct type_print_options *);
 
-/* Print a type but allow the precise language to be specified.  */
+/* Print TYPE to STREAM using syntax appropriate for LANGUAGE, a
+   C-like language.  The other parameters are like
+   type_language_defn::print_type's.  */
 
-extern void c_print_type (struct type *, const char *,
-                         struct ui_file *, int, int,
-                         enum language,
-                         const struct type_print_options *);
+extern void c_print_type (struct type *type,
+                         const char *varstring,
+                         struct ui_file *stream,
+                         int show, int level,
+                         enum language language,
+                         const struct type_print_options *flags);
 
 extern void c_print_typedef (struct type *,
                             struct symbol *,

[PATCH 3/3] Fix "b f(std::string)" when current language is C

[Pedro Alves]

If you try to set a breakpoint at a function such as "b
f(std::string)", and the current language is C, the breakpoint fails
to be set, like so:

  (gdb) set language c
  break f(std::string)
  Function "f(std::string)" not defined.
  Make breakpoint pending on future shared library load? (y or [n]) n
  (gdb)

The problem is that the code in GDB that expands the std::string
typedef hits this in c-typeprint.c:

      /* If we have "typedef struct foo {. . .} bar;" do we want to
	 print it as "struct foo" or as "bar"?  Pick the latter for
	 C++, because C++ folk tend to expect things like "class5
	 *foo" rather than "struct class5 *foo".  We rather
	 arbitrarily choose to make language_minimal work in a C-like
	 way. */
      if (language == language_c || language == language_minimal)
	{
	  if (type->code () == TYPE_CODE_UNION)
	    gdb_printf (stream, "union ");
	  else if (type->code () == TYPE_CODE_STRUCT)
	    {
	      if (type->is_declared_class ())
		gdb_printf (stream, "class ");
	      else
		gdb_printf (stream, "struct ");
	    }
	  else if (type->code () == TYPE_CODE_ENUM)
	    gdb_printf (stream, "enum ");
	}

I.e., std::string is expanded to "class std::..." instead of just
"std::...", and then the "f(class std::..." symbol doesn't exist.

Fix this by making cp-support.c:inspect_type print the expanded
typedef type using the language of the symbol whose type we're
expanding the typedefs for -- in the example in question, the
"std::string" typedef symbol, which is a C++ symbol.

Use type_print_raw_options as it seems to me that in this scenario we
always want raw types, to match the real symbol names.

Adjust the gdb.cp/break-std-string.exp testcase to try setting a
breakpoint at "f(std::string)" in both C and C++.

Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b
---
 gdb/cp-support.c                            | 10 +++++++++-
 gdb/testsuite/gdb.cp/break-f-std-string.exp | 13 ++++++++++---
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 1ddc2045808..06d541f0603 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -41,6 +41,7 @@
 #include <atomic>
 #include "event-top.h"
 #include "run-on-main-thread.h"
+#include "typeprint.h"
 
 #define d_left(dc) (dc)->u.s_binary.left
 #define d_right(dc) (dc)->u.s_binary.right
@@ -229,7 +230,14 @@ inspect_type (struct demangle_parse_info *info,
 	  string_file buf;
 	  try
 	    {
-	      type_print (type, "", &buf, -1);
+	      /* Avoid using the current language.  If the language is
+		 C, and TYPE is a struct/class, the printed type is
+		 prefixed with "struct " or "class ", which we don't
+		 want when we're expanding a C++ typedef.  Print using
+		 the type symbol's language to expand a C++ typedef
+		 the C++ way even if the current language is C.  */
+	      const language_defn *lang = language_def (sym->language ());
+	      lang->print_type (type, "", &buf, -1, 0, &type_print_raw_options);
 	    }
 	  /* If type_print threw an exception, there is little point
 	     in continuing, so just bow out gracefully.  */
diff --git a/gdb/testsuite/gdb.cp/break-f-std-string.exp b/gdb/testsuite/gdb.cp/break-f-std-string.exp
index 0869912bb29..e222bae8ab3 100644
--- a/gdb/testsuite/gdb.cp/break-f-std-string.exp
+++ b/gdb/testsuite/gdb.cp/break-f-std-string.exp
@@ -93,10 +93,17 @@ proc test {cxx11_abi} {
 	}
     }
 
-    gdb_test "break f($type)" "$srcfile, line $::decimal\\."
+    # GDB should be able to expand the std::string typedef in the
+    # function prototype using C++ logic even if the current language
+    # is C.
+    foreach_with_prefix lang {"c" "c++"} {
+	gdb_test_no_output "set language $lang"
 
-    if { $realtype != "" } {
-	gdb_test "break f($realtype)" "$srcfile, line $::decimal\\."
+	gdb_test "break f($type)" "$srcfile, line $::decimal\\."
+
+	if { $realtype != "" } {
+	    gdb_test "break f($realtype)" "$srcfile, line $::decimal\\."
+	}
     }
 }
 
-- 
2.36.0

Re: [PATCH 3/3] Fix "b f(std::string)" when current language is C

[Keith Seitz]

On 5/5/22 11:50, Pedro Alves wrote:

> Fix this by making cp-support.c:inspect_type print the expanded
> typedef type using the language of the symbol whose type we're
> expanding the typedefs for -- in the example in question, the
> "std::string" typedef symbol, which is a C++ symbol.
> 
> Use type_print_raw_options as it seems to me that in this scenario we
> always want raw types, to match the real symbol names.
> 
> Adjust the gdb.cp/break-std-string.exp testcase to try setting a
> breakpoint at "f(std::string)" in both C and C++.

If I may be so bold, this LGTM.

Keith

Re: [PATCH 3/3] Fix "b f(std::string)" when current language is C

[Lancelot SIX]

> [...]
> 
> Use type_print_raw_options as it seems to me that in this scenario we
> always want raw types, to match the real symbol names.
> 
> Adjust the gdb.cp/break-std-string.exp testcase to try setting a

Hi,

Just a nit, the testcase is "gdb.cp/break-f-std-string.exp".

Best,
Lancelot.

Re: [PATCH 3/3] Fix "b f(std::string)" when current language is C

[Pedro Alves]

On 2022-05-06 09:34, Lancelot SIX wrote:
>> [...]
>>
>> Use type_print_raw_options as it seems to me that in this scenario we
>> always want raw types, to match the real symbol names.
>>
>> Adjust the gdb.cp/break-std-string.exp testcase to try setting a
> 
> Hi,
> 
> Just a nit, the testcase is "gdb.cp/break-f-std-string.exp".

Thanks, fixed locally.

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

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This series evolved from the discussion starting at:
Pedro>   [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test
Pedro>   https://sourceware.org/pipermail/gdb-patches/2022-April/188542.html
[...]

These all look reasonable to me fwiw.

Tom