[PATCH 1/2] gdb/testsuite: fix max-completions regex in test_gdb_complete_cmd_multiple

[PATCH 1/2] gdb/testsuite: fix max-completions regex in test_gdb_complete_cmd_multiple

[Simon Marchi]

When called with MAX_COMPLETIONS true and a non-empty COMPLETION_WORD,
test_gdb_complete_cmd_multiple fails to match the "List may be
truncated" line.  It expects that line to start with CMD_PREFIX only
("macro expand " in the example below), but GDB prints CMD_PREFIX
("macro expand ") followed by COMPLETION_WORD ("BEFORE_"):

    (gdb) complete macro expand BEFORE_
    macro expand BEFORE_MACSCP1_1
    macro expand BEFORE_MACSCP1_2
    macro expand BEFORE_ *** List may be truncated, max-completions reached. ***

Fix it by including COMPLETION_WORD in the expected line.  The next
patch adds a test that depends on this.  I suppose that no other test
uses this proc in this exact way currently, I don't see any regression
following this change.

Change-Id: I645197f625212dd6c654081742b3bd42fac36229
---
 gdb/testsuite/lib/completion-support.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp
index 112c48df6cf0..0c4b72b1c073 100644
--- a/gdb/testsuite/lib/completion-support.exp
+++ b/gdb/testsuite/lib/completion-support.exp
@@ -246,9 +246,9 @@ proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list
     set expected_re [make_cmd_completion_list_re $cmd_prefix $completion_list $start_quote_char $end_quote_char]
 
     if {$max_completions} {
-	set cmd_prefix_re [string_to_regexp $cmd_prefix]
+	set input_re [string_to_regexp $cmd_prefix$completion_word]
 	append expected_re \
-	    "$cmd_prefix_re \\*\\*\\* List may be truncated, max-completions reached\\. \\*\\*\\*.*\r\n"
+	    "$input_re \\*\\*\\* List may be truncated, max-completions reached\\. \\*\\*\\*.*\r\n"
     }
 
     set cmd_re [string_to_regexp "complete $cmd_prefix$completion_word"]

base-commit: 50b7267f00c094614ab4efb43f7aac0507a436e7
-- 
2.56.0

[PATCH 2/2] gdb/macrotab: catch exception and rethrow around splay_tree_foreach

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

Bug 34876 talks about a crash that happens with "-O3 -flto", when
completing and reaching the maximum number of completions.

It can be reproduced by building GDB in this configuration:

    $ ./configure CFLAGS="-march=x86-64-v3 -flto -O3" CXXFLAGS="-march=x86-64-v3 -flto -O3" --disable-nls

Then, using a dummy C program compiled with "gcc -g3", this crashes GDB:

    $ ./gdb -q -nx --data-directory data-directory ./a.out -ex list -ex "complete macro expand "

The crash manifests as an invalid free, reported by glibc:

    free(): invalid pointer

It is reported here:

    #0  0x00007ffff6a9a11c in ?? () from /usr/lib/libc.so.6
    #1  0x00007ffff6a3e5d0 in raise () from /usr/lib/libc.so.6
    #2  0x00007ffff6a25685 in abort () from /usr/lib/libc.so.6
    #3  0x00007ffff6a26704 in ?? () from /usr/lib/libc.so.6
    #4  0x00007ffff6aa47ec in ?? () from /usr/lib/libc.so.6
    #5  0x00007ffff6aa4804 in ?? () from /usr/lib/libc.so.6
    #6  0x00005555556e07ac in operator delete (p=<optimized out>) at /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:108
    #7  operator delete (p=<optimized out>) at /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:114
    #8  operator delete (p=<optimized out>) at /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:120
    #9  std::__new_allocator<char>::deallocate (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, __p=<optimized out>, __n=<optimized out>) at /usr/include/c++/16/bits/new_allocator.h:183
    #10 std::allocator<char>::deallocate (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, __p=<optimized out>, __n=<optimized out>) at /usr/include/c++/16/bits/allocator.h:218
    #11 std::allocator_traits<std::allocator<char> >::deallocate (__a=..., __p=<optimized out>, __n=<optimized out>) at /usr/include/c++/16/bits/alloc_traits.h:689
    #12 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_destroy (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, __size=<optimized out>) at /usr/include/c++/16/bits/basic_string.h:304
    #13 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>) at /usr/include/c++/16/bits/basic_string.h:298
    #14 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/bits/basic_string.h:920
    #15 demangle_for_lookup_info::~demangle_for_lookup_info (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.h:183
    #16 std::_Optional_payload_base<demangle_for_lookup_info>::_M_destroy (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>) at /usr/include/c++/16/optional:325
    #17 std::_Optional_payload_base<demangle_for_lookup_info>::_M_reset (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>) at /usr/include/c++/16/optional:361
    #18 std::_Optional_payload<demangle_for_lookup_info, false, false, false>::~_Optional_payload (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/optional:484
    #19 std::_Optional_base<demangle_for_lookup_info, false, false>::~_Optional_base (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/optional:508
    #20 std::optional<demangle_for_lookup_info>::~optional (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/optional:812
    #21 lookup_name_info::~lookup_name_info (this=0x555555caa440 <gdb::function_view<void (char const*, macro_definition const*, macro_source_file*, int)>::bind<default_collect_symbol_completion_matches_break_on(completion_tracker&, complete_symbol_mode, symbol_name_match_type, char const*, char const*, char const*, type_code)::{lambda(char const*, macro_definition const*, macro_source_file*, int)#1}>(default_collect_symbol_completion_matches_break_on(completion_tracker&, complete_symbol_mode, symbol_name_match_type, char const*, char const*, char const*, type_code)::{lambda(char const*, macro_definition const*, macro_source_file*, int)#1}&)::{lambda(gdb::fv_detail::erased_callable, char const*, macro_definition const*, macro_source_file*, int)#1}::_FUN(gdb::fv_detail::erased_callable, char const*, macro_definition const*, macro_source_file*, int)>, this=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.h:210
    #22 0x000055555569ca89 in default_collect_symbol_completion_matches_break_on (tracker=..., mode=<optimized out>, name_match_type=<optimized out>, text=<optimized out>, word=<optimized out>, break_on=<optimized out>, code=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.c:6111
    #23 0x0000555555826c22 in language_defn::collect_symbol_completion_matches (this=<optimized out>, tracker=..., mode=<optimized out>, name_match_type=<optimized out>, text=<optimized out>, word=<optimized out>, code=<optimized out>, this=<optimized out>, tracker=..., mode=<optimized out>, name_match_type=<optimized out>, text=<optimized out>, word=<optimized out>, code=<optimized out>) at /home/simark/src/binutils-gdb/gdb/language.h:477
    #24 collect_symbol_completion_matches (tracker=..., mode=complete_symbol_mode::EXPRESSION, name_match_type=symbol_name_match_type::EXPRESSION, text=<optimized out>, word=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.c:6122
    #25 symbol_completer (ignore=<optimized out>, tracker=..., text=<optimized out>, word=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:1533
    #26 0x000055555583ef85 in complete_line_internal_normal_command (command=0x7fffffffd880 "macro expand ", tracker=..., word=0x7fffffffd88d "", cmd_args=<optimized out>, reason=handle_completions, c=0x5555563b02c0) at /home/simark/src/binutils-gdb/gdb/completer.c:1620
    #27 complete_line_internal_1 (tracker=..., text=text@entry=0x7fffffffe7e2 "", line_buffer=line_buffer@entry=0x7fffffffe7d5 "macro expand ", point=<optimized out>, reason=reason@entry=handle_completions) at /home/simark/src/binutils-gdb/gdb/completer.c:1843
    #28 0x000055555583fe81 in complete_line_internal (tracker=..., text=0x7fffffffe7e2 "", line_buffer=<optimized out>, point=<optimized out>, reason=handle_completions) at /home/simark/src/binutils-gdb/gdb/completer.c:1862
    #29 complete_line (tracker=..., text=0x7fffffffe7e2 "", line_buffer=<optimized out>, point=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:2157
    #30 complete_line (tracker=..., text=0x7fffffffe7e2 "", line_buffer=<optimized out>, point=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:2152
    #31 complete(char const*, char const**, int*) [clone .constprop.0] (line=0x7fffffffe7d5 "macro expand ", word=0x7fffffffdb88, quote_char=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:2126
    #32 0x00005555557e16db in complete_command (arg=0x7fffffffe7d5 "macro expand ", from_tty=<optimized out>) at /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:425

I asked Claude to find the root cause, and it came up with the following
explanation:

  1. With -batch, or after 200 matches interactively,
     completion_tracker::add_completion throws
     MAX_COMPLETIONS_REACHED_ERROR.

  2. That throw happens inside the add_macro_name callback, which is called
     from libiberty's splay_tree_foreach (C code) via
     foreach_macro_in_scope in gdb/macrotab.c.

  3. libiberty is compiled as C without -fexceptions, so GCC treats
     splay_tree_foreach as nothrow. With LTO, GCC can see that across
     translation units, so it decides macro_for_each_in_scope can't throw
     either. It then compiles the call at symtab.c:6052 as non-throwing.

  4. At runtime the exception still unwinds, because x86-64 has CFI
     everywhere. But it lands in a landing pad that doesn't belong to that
     call site. That pad runs ~lookup_name_info() with this = %rbx. At the
     throwing call, %rbx actually holds the function_view invoker address
     (0x555555b32780, a code address), so it destroys garbage and hands a
     bogus pointer to free().

I can't tell for sure that it's correct, but it sounds very plausible to
me (the address 0x555555b32780 is from another run on another computer, not
related to the backtrace above).

On x86-64, the psABI mandates that all programs and libraries have an async
unwind table (.eh_frame), so the exception is able to go through the C
frames unnoticed most of the time.  But on ARM, for example, programs don't
have unwind tables by default.  I was able to reproduce the bug on my old
Raspberry Pi running an outdated Debian:

    (gdb) show version
    GNU gdb (Raspbian 10.1-1.7) 10.1.90.20210103-git
    ...
    (gdb) list
    1	int main()
    2	{
    3	}
    (gdb) complete macro expand
    terminate called after throwing an instance of 'gdb_exception_error'
    Aborted

There, the exception causes std::terminate to be called as soon as it
reaches the libiberty C frame.

This patch updates the two callbacks passed to splay_tree_foreach in
this file (foreach_macro and foreach_macro_in_scope) to catch any
exception that would escape, stash it in macro_for_each_data::exception,
and return 1 to stop the splay tree iteration.  The caller of
splay_tree_foreach then rethrows the exception if there is one.

Since I'm re-indenting the code anyway, I took the opportunity to
modernize it a bit.

Improve gdb.base/macscp.exp to kind of cover this.  The test won't catch
the bug on a regular x86-64 build, but it would in an LTO build like in
the bug report, or on ARM without unwind tables.  It would also catch it
if we just added "noexcept" to the callbacks.

There is another latent instance of this bug in addrmap.c, where we call
splay_tree_foreach and the callback can throw.  And probably more when
using other APIs.  As future work, I propose that we go over all the
callbacks that we pass to a C API to mark them "noexcept", so that these
cases of exceptions crossing C frames consistently crash GDB on any
configuration.  And fix them if necessary to catch and rethrow
exceptions like done in this patch.

Change-Id: Ie7c4515a4dee1c93b04271f1d5057be23c96c039
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34876
---
 gdb/macrotab.c                    | 73 ++++++++++++++++++++-----------
 gdb/testsuite/gdb.base/macscp.exp | 27 ++++++++++++
 2 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 078eb5cb5aa7..a9b678b32e70 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -945,22 +945,31 @@ struct macro_for_each_data
   gdb::function_view<macro_callback_fn> fn;
   struct macro_source_file *file;
   int line;
+  std::exception_ptr exception;
 };
 
 /* Helper function for macro_for_each.  */
 static int
-foreach_macro (splay_tree_node node, void *arg)
+foreach_macro (splay_tree_node node, void *arg) noexcept
 {
-  struct macro_for_each_data *datum = (struct macro_for_each_data *) arg;
-  struct macro_key *key = (struct macro_key *) node->key;
-  struct macro_definition *def;
+  auto datum = static_cast<macro_for_each_data *> (arg);
 
-  std::string key_fullname = macro_source_fullname (key->start_file);
-  def = fixup_definition (key_fullname.c_str (), key->start_line,
-			  (struct macro_definition *) node->value);
+  try
+    {
+      auto key = reinterpret_cast<macro_key *> (node->key);
+      auto value = reinterpret_cast<macro_definition *> (node->value);
+      std::string key_fullname = macro_source_fullname (key->start_file);
+      macro_definition *def = fixup_definition (key_fullname.c_str (),
+						key->start_line, value);
 
-  datum->fn (key->name, def, key->start_file, key->start_line);
-  return 0;
+      datum->fn (key->name, def, key->start_file, key->start_line);
+      return 0;
+    }
+  catch (...)
+    {
+      datum->exception = std::current_exception ();
+      return 1;
+    }
 }
 
 /* Call FN for every macro in TABLE.  */
@@ -974,28 +983,39 @@ macro_for_each (struct macro_table *table,
   datum.file = NULL;
   datum.line = 0;
   splay_tree_foreach (table->definitions, foreach_macro, &datum);
+
+  if (datum.exception != nullptr)
+    std::rethrow_exception (datum.exception);
 }
 
 static int
-foreach_macro_in_scope (splay_tree_node node, void *info)
+foreach_macro_in_scope (splay_tree_node node, void *info) noexcept
 {
-  struct macro_for_each_data *datum = (struct macro_for_each_data *) info;
-  struct macro_key *key = (struct macro_key *) node->key;
-  struct macro_definition *def;
+  auto datum = static_cast<macro_for_each_data *> (info);
 
-  std::string datum_fullname = macro_source_fullname (datum->file);
-  def = fixup_definition (datum_fullname.c_str (), datum->line,
-			  (struct macro_definition *) node->value);
+  try
+    {
+      auto key = reinterpret_cast<macro_key *> (node->key);
+      auto value = reinterpret_cast<macro_definition *> (node->value);
+      std::string datum_fullname = macro_source_fullname (datum->file);
+      macro_definition *def = fixup_definition (datum_fullname.c_str (),
+						datum->line, value);
 
-  /* See if this macro is defined before the passed-in line, and
-     extends past that line.  */
-  if (compare_locations (key->start_file, key->start_line,
-			 datum->file, datum->line) < 0
-      && (!key->end_file
-	  || compare_locations (key->end_file, key->end_line,
-				datum->file, datum->line) >= 0))
-    datum->fn (key->name, def, key->start_file, key->start_line);
-  return 0;
+      /* See if this macro is defined before the passed-in line, and
+	 extends past that line.  */
+      if (compare_locations (key->start_file, key->start_line,
+			     datum->file, datum->line) < 0
+	  && (key->end_file == nullptr
+	      || compare_locations (key->end_file, key->end_line,
+				    datum->file, datum->line) >= 0))
+	datum->fn (key->name, def, key->start_file, key->start_line);
+      return 0;
+    }
+  catch (...)
+    {
+      datum->exception = std::current_exception ();
+      return 1;
+    }
 }
 
 /* Call FN for every macro is visible in SCOPE.  */
@@ -1010,6 +1030,9 @@ macro_for_each_in_scope (struct macro_source_file *file, int line,
   datum.line = line;
   splay_tree_foreach (file->table->definitions,
 		      foreach_macro_in_scope, &datum);
+
+  if (datum.exception != nullptr)
+    std::rethrow_exception (datum.exception);
 }
 
 
diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp
index e40bfb633378..e0bcddb9312b 100644
--- a/gdb/testsuite/gdb.base/macscp.exp
+++ b/gdb/testsuite/gdb.base/macscp.exp
@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+load_lib completion-support.exp
 
 standard_testfile macscp1.c
 set objfile [standard_output_file ${testfile}.o]
@@ -750,3 +751,29 @@ gdb_test "macro expand siginfo.si_addr" \
 gdb_test "print __FILE__" " = \".*macscp1.c\""
 gdb_test "print __LINE__" \
     " = [gdb_get_line_number {stopping point for line test}]"
+
+# Regression test for bug 34876.
+
+with_test_prefix "max-completions reached" {
+    with_set max-completions 2 {
+	# Macros from the program, visited by foreach_macro_in_scope.  All
+	# the BEFORE_<func> macros are in scope at this point.  The macros
+	# are visited in alphabetical order.
+	test_gdb_complete_multiple "macro expand " "BEFORE_" "MACSCP1_" {
+	    "BEFORE_MACSCP1_1"
+	    "BEFORE_MACSCP1_2"
+	} "" "" true "program macros"
+
+	# User-defined macros, visited by foreach_macro.
+	gdb_test_no_output "macro define USER_MACRO_A 1"
+	gdb_test_no_output "macro define USER_MACRO_B 2"
+	gdb_test_no_output "macro define USER_MACRO_C 3"
+	test_gdb_complete_multiple "macro expand " "USER_MACRO_" "" {
+	    "USER_MACRO_A"
+	    "USER_MACRO_B"
+	} "" "" true "user macros"
+	gdb_test_no_output "macro undef USER_MACRO_A"
+	gdb_test_no_output "macro undef USER_MACRO_B"
+	gdb_test_no_output "macro undef USER_MACRO_C"
+    }
+}
-- 
2.56.0

Re: [PATCH 2/2] gdb/macrotab: catch exception and rethrow around splay_tree_foreach

[Andrew Burgess]

Simon Marchi <simon.marchi@efficios.com> writes:

> From: Simon Marchi <simon.marchi@polymtl.ca>
>
> Bug 34876 talks about a crash that happens with "-O3 -flto", when
> completing and reaching the maximum number of completions.
>
> It can be reproduced by building GDB in this configuration:
>
>     $ ./configure CFLAGS="-march=x86-64-v3 -flto -O3" CXXFLAGS="-march=x86-64-v3 -flto -O3" --disable-nls
>
> Then, using a dummy C program compiled with "gcc -g3", this crashes GDB:
>
>     $ ./gdb -q -nx --data-directory data-directory ./a.out -ex list -ex "complete macro expand "
>
> The crash manifests as an invalid free, reported by glibc:
>
>     free(): invalid pointer
>
> It is reported here:
>
>     #0  0x00007ffff6a9a11c in ?? () from /usr/lib/libc.so.6
>     #1  0x00007ffff6a3e5d0 in raise () from /usr/lib/libc.so.6
>     #2  0x00007ffff6a25685 in abort () from /usr/lib/libc.so.6
>     #3  0x00007ffff6a26704 in ?? () from /usr/lib/libc.so.6
>     #4  0x00007ffff6aa47ec in ?? () from /usr/lib/libc.so.6
>     #5  0x00007ffff6aa4804 in ?? () from /usr/lib/libc.so.6
>     #6  0x00005555556e07ac in operator delete (p=<optimized out>) at /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:108
>     #7  operator delete (p=<optimized out>) at /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:114
>     #8  operator delete (p=<optimized out>) at /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:120
>     #9  std::__new_allocator<char>::deallocate (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, __p=<optimized out>, __n=<optimized out>) at /usr/include/c++/16/bits/new_allocator.h:183
>     #10 std::allocator<char>::deallocate (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, __p=<optimized out>, __n=<optimized out>) at /usr/include/c++/16/bits/allocator.h:218
>     #11 std::allocator_traits<std::allocator<char> >::deallocate (__a=..., __p=<optimized out>, __n=<optimized out>) at /usr/include/c++/16/bits/alloc_traits.h:689
>     #12 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_destroy (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, __size=<optimized out>) at /usr/include/c++/16/bits/basic_string.h:304
>     #13 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>) at /usr/include/c++/16/bits/basic_string.h:298
>     #14 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/bits/basic_string.h:920
>     #15 demangle_for_lookup_info::~demangle_for_lookup_info (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.h:183
>     #16 std::_Optional_payload_base<demangle_for_lookup_info>::_M_destroy (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>) at /usr/include/c++/16/optional:325
>     #17 std::_Optional_payload_base<demangle_for_lookup_info>::_M_reset (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>) at /usr/include/c++/16/optional:361
>     #18 std::_Optional_payload<demangle_for_lookup_info, false, false, false>::~_Optional_payload (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/optional:484
>     #19 std::_Optional_base<demangle_for_lookup_info, false, false>::~_Optional_base (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/optional:508
>     #20 std::optional<demangle_for_lookup_info>::~optional (this=0x555555caa4f8 <completion_list_add_fields(completion_tracker&, symbol*, lookup_name_info const&, char const*, char const*)+136>, this=<optimized out>) at /usr/include/c++/16/optional:812
>     #21 lookup_name_info::~lookup_name_info (this=0x555555caa440 <gdb::function_view<void (char const*, macro_definition const*, macro_source_file*, int)>::bind<default_collect_symbol_completion_matches_break_on(completion_tracker&, complete_symbol_mode, symbol_name_match_type, char const*, char const*, char const*, type_code)::{lambda(char const*, macro_definition const*, macro_source_file*, int)#1}>(default_collect_symbol_completion_matches_break_on(completion_tracker&, complete_symbol_mode, symbol_name_match_type, char const*, char const*, char const*, type_code)::{lambda(char const*, macro_definition const*, macro_source_file*, int)#1}&)::{lambda(gdb::fv_detail::erased_callable, char const*, macro_definition const*, macro_source_file*, int)#1}::_FUN(gdb::fv_detail::erased_callable, char const*, macro_definition const*, macro_source_file*, int)>, this=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.h:210
>     #22 0x000055555569ca89 in default_collect_symbol_completion_matches_break_on (tracker=..., mode=<optimized out>, name_match_type=<optimized out>, text=<optimized out>, word=<optimized out>, break_on=<optimized out>, code=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.c:6111
>     #23 0x0000555555826c22 in language_defn::collect_symbol_completion_matches (this=<optimized out>, tracker=..., mode=<optimized out>, name_match_type=<optimized out>, text=<optimized out>, word=<optimized out>, code=<optimized out>, this=<optimized out>, tracker=..., mode=<optimized out>, name_match_type=<optimized out>, text=<optimized out>, word=<optimized out>, code=<optimized out>) at /home/simark/src/binutils-gdb/gdb/language.h:477
>     #24 collect_symbol_completion_matches (tracker=..., mode=complete_symbol_mode::EXPRESSION, name_match_type=symbol_name_match_type::EXPRESSION, text=<optimized out>, word=<optimized out>) at /home/simark/src/binutils-gdb/gdb/symtab.c:6122
>     #25 symbol_completer (ignore=<optimized out>, tracker=..., text=<optimized out>, word=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:1533
>     #26 0x000055555583ef85 in complete_line_internal_normal_command (command=0x7fffffffd880 "macro expand ", tracker=..., word=0x7fffffffd88d "", cmd_args=<optimized out>, reason=handle_completions, c=0x5555563b02c0) at /home/simark/src/binutils-gdb/gdb/completer.c:1620
>     #27 complete_line_internal_1 (tracker=..., text=text@entry=0x7fffffffe7e2 "", line_buffer=line_buffer@entry=0x7fffffffe7d5 "macro expand ", point=<optimized out>, reason=reason@entry=handle_completions) at /home/simark/src/binutils-gdb/gdb/completer.c:1843
>     #28 0x000055555583fe81 in complete_line_internal (tracker=..., text=0x7fffffffe7e2 "", line_buffer=<optimized out>, point=<optimized out>, reason=handle_completions) at /home/simark/src/binutils-gdb/gdb/completer.c:1862
>     #29 complete_line (tracker=..., text=0x7fffffffe7e2 "", line_buffer=<optimized out>, point=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:2157
>     #30 complete_line (tracker=..., text=0x7fffffffe7e2 "", line_buffer=<optimized out>, point=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:2152
>     #31 complete(char const*, char const**, int*) [clone .constprop.0] (line=0x7fffffffe7d5 "macro expand ", word=0x7fffffffdb88, quote_char=<optimized out>) at /home/simark/src/binutils-gdb/gdb/completer.c:2126
>     #32 0x00005555557e16db in complete_command (arg=0x7fffffffe7d5 "macro expand ", from_tty=<optimized out>) at /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:425
>
> I asked Claude to find the root cause, and it came up with the following
> explanation:
>
>   1. With -batch, or after 200 matches interactively,
>      completion_tracker::add_completion throws
>      MAX_COMPLETIONS_REACHED_ERROR.
>
>   2. That throw happens inside the add_macro_name callback, which is called
>      from libiberty's splay_tree_foreach (C code) via
>      foreach_macro_in_scope in gdb/macrotab.c.
>
>   3. libiberty is compiled as C without -fexceptions, so GCC treats
>      splay_tree_foreach as nothrow. With LTO, GCC can see that across
>      translation units, so it decides macro_for_each_in_scope can't throw
>      either. It then compiles the call at symtab.c:6052 as non-throwing.
>
>   4. At runtime the exception still unwinds, because x86-64 has CFI
>      everywhere. But it lands in a landing pad that doesn't belong to that
>      call site. That pad runs ~lookup_name_info() with this = %rbx. At the
>      throwing call, %rbx actually holds the function_view invoker address
>      (0x555555b32780, a code address), so it destroys garbage and hands a
>      bogus pointer to free().
>
> I can't tell for sure that it's correct, but it sounds very plausible to
> me (the address 0x555555b32780 is from another run on another computer, not
> related to the backtrace above).
>
> On x86-64, the psABI mandates that all programs and libraries have an async
> unwind table (.eh_frame), so the exception is able to go through the C
> frames unnoticed most of the time.  But on ARM, for example, programs don't
> have unwind tables by default.  I was able to reproduce the bug on my old
> Raspberry Pi running an outdated Debian:
>
>     (gdb) show version
>     GNU gdb (Raspbian 10.1-1.7) 10.1.90.20210103-git
>     ...
>     (gdb) list
>     1	int main()
>     2	{
>     3	}
>     (gdb) complete macro expand
>     terminate called after throwing an instance of 'gdb_exception_error'
>     Aborted
>
> There, the exception causes std::terminate to be called as soon as it
> reaches the libiberty C frame.
>
> This patch updates the two callbacks passed to splay_tree_foreach in
> this file (foreach_macro and foreach_macro_in_scope) to catch any
> exception that would escape, stash it in macro_for_each_data::exception,
> and return 1 to stop the splay tree iteration.  The caller of
> splay_tree_foreach then rethrows the exception if there is one.
>
> Since I'm re-indenting the code anyway, I took the opportunity to
> modernize it a bit.
>
> Improve gdb.base/macscp.exp to kind of cover this.  The test won't catch
> the bug on a regular x86-64 build, but it would in an LTO build like in
> the bug report, or on ARM without unwind tables.  It would also catch it
> if we just added "noexcept" to the callbacks.
>
> There is another latent instance of this bug in addrmap.c, where we call
> splay_tree_foreach and the callback can throw.  And probably more when
> using other APIs.  As future work, I propose that we go over all the
> callbacks that we pass to a C API to mark them "noexcept", so that these
> cases of exceptions crossing C frames consistently crash GDB on any
> configuration.  And fix them if necessary to catch and rethrow
> exceptions like done in this patch.
>
> Change-Id: Ie7c4515a4dee1c93b04271f1d5057be23c96c039
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34876
> ---
>  gdb/macrotab.c                    | 73 ++++++++++++++++++++-----------
>  gdb/testsuite/gdb.base/macscp.exp | 27 ++++++++++++
>  2 files changed, 75 insertions(+), 25 deletions(-)
>
> diff --git a/gdb/macrotab.c b/gdb/macrotab.c
> index 078eb5cb5aa7..a9b678b32e70 100644
> --- a/gdb/macrotab.c
> +++ b/gdb/macrotab.c
> @@ -945,22 +945,31 @@ struct macro_for_each_data
>    gdb::function_view<macro_callback_fn> fn;
>    struct macro_source_file *file;
>    int line;
> +  std::exception_ptr exception;
>  };
>  
>  /* Helper function for macro_for_each.  */
>  static int
> -foreach_macro (splay_tree_node node, void *arg)
> +foreach_macro (splay_tree_node node, void *arg) noexcept
>  {
> -  struct macro_for_each_data *datum = (struct macro_for_each_data *) arg;
> -  struct macro_key *key = (struct macro_key *) node->key;
> -  struct macro_definition *def;
> +  auto datum = static_cast<macro_for_each_data *> (arg);
>  
> -  std::string key_fullname = macro_source_fullname (key->start_file);
> -  def = fixup_definition (key_fullname.c_str (), key->start_line,
> -			  (struct macro_definition *) node->value);
> +  try
> +    {
> +      auto key = reinterpret_cast<macro_key *> (node->key);
> +      auto value = reinterpret_cast<macro_definition *> (node->value);
> +      std::string key_fullname = macro_source_fullname (key->start_file);
> +      macro_definition *def = fixup_definition (key_fullname.c_str (),
> +						key->start_line, value);
>  
> -  datum->fn (key->name, def, key->start_file, key->start_line);
> -  return 0;
> +      datum->fn (key->name, def, key->start_file, key->start_line);
> +      return 0;
> +    }
> +  catch (...)
> +    {
> +      datum->exception = std::current_exception ();
> +      return 1;
> +    }
>  }
>  
>  /* Call FN for every macro in TABLE.  */
> @@ -974,28 +983,39 @@ macro_for_each (struct macro_table *table,
>    datum.file = NULL;

Given you are modernising the code anyway, maybe NULL -> nullptr here?

But otherwise, this looks great.  The approach and implementation make
sense to me.

Thanks for fixing this.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>    datum.line = 0;
>    splay_tree_foreach (table->definitions, foreach_macro, &datum);
> +
> +  if (datum.exception != nullptr)
> +    std::rethrow_exception (datum.exception);
>  }
>  
>  static int
> -foreach_macro_in_scope (splay_tree_node node, void *info)
> +foreach_macro_in_scope (splay_tree_node node, void *info) noexcept
>  {
> -  struct macro_for_each_data *datum = (struct macro_for_each_data *) info;
> -  struct macro_key *key = (struct macro_key *) node->key;
> -  struct macro_definition *def;
> +  auto datum = static_cast<macro_for_each_data *> (info);
>  
> -  std::string datum_fullname = macro_source_fullname (datum->file);
> -  def = fixup_definition (datum_fullname.c_str (), datum->line,
> -			  (struct macro_definition *) node->value);
> +  try
> +    {
> +      auto key = reinterpret_cast<macro_key *> (node->key);
> +      auto value = reinterpret_cast<macro_definition *> (node->value);
> +      std::string datum_fullname = macro_source_fullname (datum->file);
> +      macro_definition *def = fixup_definition (datum_fullname.c_str (),
> +						datum->line, value);
>  
> -  /* See if this macro is defined before the passed-in line, and
> -     extends past that line.  */
> -  if (compare_locations (key->start_file, key->start_line,
> -			 datum->file, datum->line) < 0
> -      && (!key->end_file
> -	  || compare_locations (key->end_file, key->end_line,
> -				datum->file, datum->line) >= 0))
> -    datum->fn (key->name, def, key->start_file, key->start_line);
> -  return 0;
> +      /* See if this macro is defined before the passed-in line, and
> +	 extends past that line.  */
> +      if (compare_locations (key->start_file, key->start_line,
> +			     datum->file, datum->line) < 0
> +	  && (key->end_file == nullptr
> +	      || compare_locations (key->end_file, key->end_line,
> +				    datum->file, datum->line) >= 0))
> +	datum->fn (key->name, def, key->start_file, key->start_line);
> +      return 0;
> +    }
> +  catch (...)
> +    {
> +      datum->exception = std::current_exception ();
> +      return 1;
> +    }
>  }
>  
>  /* Call FN for every macro is visible in SCOPE.  */
> @@ -1010,6 +1030,9 @@ macro_for_each_in_scope (struct macro_source_file *file, int line,
>    datum.line = line;
>    splay_tree_foreach (file->table->definitions,
>  		      foreach_macro_in_scope, &datum);
> +
> +  if (datum.exception != nullptr)
> +    std::rethrow_exception (datum.exception);
>  }
>  
>  
> diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp
> index e40bfb633378..e0bcddb9312b 100644
> --- a/gdb/testsuite/gdb.base/macscp.exp
> +++ b/gdb/testsuite/gdb.base/macscp.exp
> @@ -14,6 +14,7 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> +load_lib completion-support.exp
>  
>  standard_testfile macscp1.c
>  set objfile [standard_output_file ${testfile}.o]
> @@ -750,3 +751,29 @@ gdb_test "macro expand siginfo.si_addr" \
>  gdb_test "print __FILE__" " = \".*macscp1.c\""
>  gdb_test "print __LINE__" \
>      " = [gdb_get_line_number {stopping point for line test}]"
> +
> +# Regression test for bug 34876.
> +
> +with_test_prefix "max-completions reached" {
> +    with_set max-completions 2 {
> +	# Macros from the program, visited by foreach_macro_in_scope.  All
> +	# the BEFORE_<func> macros are in scope at this point.  The macros
> +	# are visited in alphabetical order.
> +	test_gdb_complete_multiple "macro expand " "BEFORE_" "MACSCP1_" {
> +	    "BEFORE_MACSCP1_1"
> +	    "BEFORE_MACSCP1_2"
> +	} "" "" true "program macros"
> +
> +	# User-defined macros, visited by foreach_macro.
> +	gdb_test_no_output "macro define USER_MACRO_A 1"
> +	gdb_test_no_output "macro define USER_MACRO_B 2"
> +	gdb_test_no_output "macro define USER_MACRO_C 3"
> +	test_gdb_complete_multiple "macro expand " "USER_MACRO_" "" {
> +	    "USER_MACRO_A"
> +	    "USER_MACRO_B"
> +	} "" "" true "user macros"
> +	gdb_test_no_output "macro undef USER_MACRO_A"
> +	gdb_test_no_output "macro undef USER_MACRO_B"
> +	gdb_test_no_output "macro undef USER_MACRO_C"
> +    }
> +}
> -- 
> 2.56.0

Re: [PATCH 2/2] gdb/macrotab: catch exception and rethrow around splay_tree_foreach

[Simon Marchi]

On 10/28/26 1:37 PM, Andrew Burgess wrote:
>> @@ -974,28 +983,39 @@ macro_for_each (struct macro_table *table,
>>    datum.file = NULL;
> 
> Given you are modernising the code anyway, maybe NULL -> nullptr here?
> 
> But otherwise, this looks great.  The approach and implementation make
> sense to me.
> 
> Thanks for fixing this.
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>
> 
> Thanks,
> Andrew

Thanks, pushed with that changed.

I'm also thinking that we could swap the splay stree for std::map, it
would make things simpler.

Simon

Re: [PATCH 1/2] gdb/testsuite: fix max-completions regex in test_gdb_complete_cmd_multiple

[Andrew Burgess]

Simon Marchi <simon.marchi@efficios.com> writes:

> When called with MAX_COMPLETIONS true and a non-empty COMPLETION_WORD,
> test_gdb_complete_cmd_multiple fails to match the "List may be
> truncated" line.  It expects that line to start with CMD_PREFIX only
> ("macro expand " in the example below), but GDB prints CMD_PREFIX
> ("macro expand ") followed by COMPLETION_WORD ("BEFORE_"):
>
>     (gdb) complete macro expand BEFORE_
>     macro expand BEFORE_MACSCP1_1
>     macro expand BEFORE_MACSCP1_2
>     macro expand BEFORE_ *** List may be truncated, max-completions reached. ***
>
> Fix it by including COMPLETION_WORD in the expected line.  The next
> patch adds a test that depends on this.  I suppose that no other test
> uses this proc in this exact way currently, I don't see any regression
> following this change.

LGTM.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> Change-Id: I645197f625212dd6c654081742b3bd42fac36229
> ---
>  gdb/testsuite/lib/completion-support.exp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp
> index 112c48df6cf0..0c4b72b1c073 100644
> --- a/gdb/testsuite/lib/completion-support.exp
> +++ b/gdb/testsuite/lib/completion-support.exp
> @@ -246,9 +246,9 @@ proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list
>      set expected_re [make_cmd_completion_list_re $cmd_prefix $completion_list $start_quote_char $end_quote_char]
>  
>      if {$max_completions} {
> -	set cmd_prefix_re [string_to_regexp $cmd_prefix]
> +	set input_re [string_to_regexp $cmd_prefix$completion_word]
>  	append expected_re \
> -	    "$cmd_prefix_re \\*\\*\\* List may be truncated, max-completions reached\\. \\*\\*\\*.*\r\n"
> +	    "$input_re \\*\\*\\* List may be truncated, max-completions reached\\. \\*\\*\\*.*\r\n"
>      }
>  
>      set cmd_re [string_to_regexp "complete $cmd_prefix$completion_word"]
>
> base-commit: 50b7267f00c094614ab4efb43f7aac0507a436e7
> -- 
> 2.56.0