Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv2 1/4] gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls
Date: Thu,  5 Mar 2026 13:37:15 +0000	[thread overview]
Message-ID: <1259b09af0b2ca1e326d81aac4a25bc0ae73c111.1772717770.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1772717769.git.aburgess@redhat.com>

The 3 frame_unwind_caller_WHAT functions:

  + frame_unwind_caller_id
  + frame_unwind_caller_pc
  + frame_unwind_caller_arch

Are, I believe, currently all broken with respect to inline and tail
call functions.

The Python FinishBreakpoint type creates a breakpoint in the caller
function which, when triggered, indicates that the FinishBreakpoint
has gone out of scope.

I was writing a test for the FinishBreakpoint type which included a
tail call function, and the FinishBreakpoint was being created for the
tail call function frame.  What I observed is that the out of scope
breakpoint was never being hit.

The call stack in my new test looked like this:

  main -> tailcall_function -> normal_function

I would stop in normal_function, and then create a FinishBreakpoint
for the parent (tailcall_function) frame.  The FinishBreakpoint's out
of scope breakpoint was being correctly placed in the 'main' function,
but would never trigger.

The problem is that the breakpoint placed in 'main' holds a frame-id.
This frame-id is the frame in which the breakpoint should trigger.
This frame-id exists to prevent premature stops due to recursion.  But
in this case, when the breakpoint in 'main' was hit, despite no
recursion having occurred, the frame-id didn't match, and so the
breakpoint was ignored.

The problem is that in bpfinishpy_init we call frame_unwind_caller_id
to compute the frame-id of the frame in which we should stop, and
frame_unwind_caller_id was returning the wrong frame-id.  As far as I
can tell frame_unwind_caller_id has been broken since it was updated
for inline functions in commit edb3359dff90ef8a.

The frame_unwind_caller_id function, and all the
frame_unwind_caller_WHAT functions, are intended to return the
previous frame, but should skip over any inline, or tail call frames.
Let's look at an example call stack:

  #0 A		// A normal function.
  #1 B		// An inline function.
  #2 C		// An inline function.
  #3 D		// A normal function.
  #4 E		// A normal function.

Starting from #0, a normal function, frame_unwind_caller_id, should
return the frame-id for #3, and this is what happens.

But if we start in #1 and call frame_unwind_caller_id, then we should
still return the frame-id for #3, but this is not what happens.
Instead we return the frame-id for #4, skipping a frame.

The problem is that frame_unwind_caller_id starts by calling
skip_artificial_frames, which calls get_prev_frame_always until we
reach a non-inline (or non-tail call) frame, this moves us from #1 to

Then, back in frame_unwind_caller_id we call get_prev_frame_always,
which moves us to #4.

Then frame_unwind_caller_id finishes with a call to
skip_artificial_frames, this could potentially result in additional
frames being skipped, but in my example above this isn't the case.

The problem here is that if skip_artificial_frames skips anything,
then we have already unwound to the caller frame, and the
get_prev_frame_always call in frame_unwind_caller_id is unnecessary.

I propose to add a new helper function frame_unwind_caller_frame,
which should do the correct thing; it unwinds one frame and then calls
skip_artificial_frames.  This should do exactly what is needed.

Then all the frame_unwind_caller_WHAT functions will be updated to use
this helper function, and just extract the required property from the
resulting frame.

With this fix in place I could then write the FinishBreakpoint test,
which now works.

I took a look for other places where frame_unwind_caller_id is used
and spotted that the 'until' command does much the same thing, placing
a breakpoint in the caller frame.  As predicted, the 'until' command
is also broken when used within a tail call frame.  This patch fixes
that issue too.  There's also a test for the until command.

The bug PR gdb/28683 seems to describe this exact problem with a
specific AArch64 case given.  I haven't actually setup the environment
needed to test this bug, but I'm reasonably sure that this patch will
fix the bug.  Even if it doesn't then it's certainly related and worth
linking into the bug report.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28683
---
 gdb/frame.c                                   |  35 ++++--
 gdb/testsuite/gdb.base/until-in-tailcall.c    |  53 +++++++++
 gdb/testsuite/gdb.base/until-in-tailcall.exp  | 108 ++++++++++++++++++
 .../py-finish-breakpoint-tailcall.c           |  45 ++++++++
 .../py-finish-breakpoint-tailcall.exp         |  93 +++++++++++++++
 .../py-finish-breakpoint-tailcall.py          |  26 +++++
 6 files changed, 349 insertions(+), 11 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/until-in-tailcall.c
 create mode 100644 gdb/testsuite/gdb.base/until-in-tailcall.exp
 create mode 100644 gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.c
 create mode 100644 gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.exp
 create mode 100644 gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.py

diff --git a/gdb/frame.c b/gdb/frame.c
index 5509e5ab7d7..14d43f93e7f 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -699,6 +699,23 @@ get_stack_frame_id (const frame_info_ptr &next_frame)
   return get_frame_id (skip_artificial_frames (next_frame));
 }
 
+/* Helper for the various frame_unwind_caller_* functions.  Unwind
+   INITIAL_NEXT_FRAME at least one frame, but skip any artificial frames,
+   that is inline or tailcall frames.
+
+   Return the Nth previous frame (as required to find a non-artificial
+   frame), or NULL if no previous frame could be found.  */
+
+static frame_info_ptr
+frame_unwind_caller_frame (const frame_info_ptr &initial_next_frame)
+{
+  frame_info_ptr this_frame = get_prev_frame_always (initial_next_frame);
+  if (this_frame == nullptr)
+    return nullptr;
+
+  return skip_artificial_frames (this_frame);
+}
+
 struct frame_id
 frame_unwind_caller_id (const frame_info_ptr &initial_next_frame)
 {
@@ -707,15 +724,11 @@ frame_unwind_caller_id (const frame_info_ptr &initial_next_frame)
      unintentionally returning a null_frame_id (e.g., when a caller
      requests the frame ID of "main()"s caller.  */
 
-  frame_info_ptr next_frame = skip_artificial_frames (initial_next_frame);
-  if (next_frame == NULL)
+  frame_info_ptr this_frame = frame_unwind_caller_frame (initial_next_frame);
+  if (this_frame == nullptr)
     return null_frame_id;
 
-  frame_info_ptr this_frame = get_prev_frame_always (next_frame);
-  if (this_frame)
-    return get_frame_id (skip_artificial_frames (this_frame));
-  else
-    return null_frame_id;
+  return get_frame_id (this_frame);
 }
 
 const struct frame_id null_frame_id = { 0 }; /* All zeros.  */
@@ -1074,14 +1087,14 @@ frame_unwind_pc (const frame_info_ptr &this_frame)
 CORE_ADDR
 frame_unwind_caller_pc (const frame_info_ptr &initial_this_frame)
 {
-  frame_info_ptr this_frame = skip_artificial_frames (initial_this_frame);
+  frame_info_ptr this_frame = frame_unwind_caller_frame (initial_this_frame);
 
   /* We must have a non-artificial frame.  The caller is supposed to check
      the result of frame_unwind_caller_id (), which returns NULL_FRAME_ID
      in this case.  */
   gdb_assert (this_frame != nullptr);
 
-  return frame_unwind_pc (this_frame);
+  return get_frame_pc (this_frame);
 }
 
 bool
@@ -3135,14 +3148,14 @@ frame_unwind_arch (const frame_info_ptr &next_frame)
 struct gdbarch *
 frame_unwind_caller_arch (const frame_info_ptr &initial_next_frame)
 {
-  frame_info_ptr next_frame = skip_artificial_frames (initial_next_frame);
+  frame_info_ptr next_frame = frame_unwind_caller_frame (initial_next_frame);
 
   /* We must have a non-artificial frame.  The caller is supposed to check
      the result of frame_unwind_caller_id (), which returns NULL_FRAME_ID
      in this case.  */
   gdb_assert (next_frame != nullptr);
 
-  return frame_unwind_arch (next_frame);
+  return get_frame_arch (next_frame);
 }
 
 /* Gets the language of FRAME.  */
diff --git a/gdb/testsuite/gdb.base/until-in-tailcall.c b/gdb/testsuite/gdb.base/until-in-tailcall.c
new file mode 100644
index 00000000000..3e3853ba6df
--- /dev/null
+++ b/gdb/testsuite/gdb.base/until-in-tailcall.c
@@ -0,0 +1,53 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 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/>.  */
+
+/* This test relies on tailcall_function actually compiling to a tail call
+   function.  we try to force this by preventing tailcall_function and
+   normal_function from being inlined, then compiling this file at -O2.
+   Still, that's no guarantee.  If tailcall_function isn't a tail call,
+   then the test becomes pointless.  */
+
+volatile int global_var = 42;
+
+int __attribute__ ((noinline, noclone))
+normal_function (int x)
+{
+  return x + 1;
+}
+
+int __attribute__ ((noinline, noclone))
+tailcall_function (int x)
+{
+  return normal_function (x);
+}
+
+void __attribute__ ((noinline, noclone))
+worker_func (void)
+{
+  ++global_var;
+  ++global_var;			/* Run until here.  */
+  ++global_var;
+}
+
+int
+main (void)
+{
+  int result = tailcall_function (42);
+  result -= global_var;		/* Should stop here.  */
+  worker_func ();
+  return result;
+}
diff --git a/gdb/testsuite/gdb.base/until-in-tailcall.exp b/gdb/testsuite/gdb.base/until-in-tailcall.exp
new file mode 100644
index 00000000000..cd66d0ceaf8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/until-in-tailcall.exp
@@ -0,0 +1,108 @@
+# Copyright (C) 2026 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/>.
+
+# Run until we have a tail call frame in the stack.  The stack looks like this:
+#
+# main -> tailcall_function -> normal_function
+#
+# We stop on normal_function, and then use 'up' to go to the
+# tailcall_function frame.  We then use 'until NN' where 'NN' is a
+# line that should not be reached until after the inferior has
+# returned to main.
+#
+# However, when the inferior returns to main, GDB should stop the
+# inferior as the 'until' has gone out of scope.
+#
+# At one point this was not happening as GDB would place the out of
+# scope 'until' guard breakpoint in the wrong place.
+#
+# This test relies on the source file compiling with a tail call in
+# place.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 {debug optimize=-O2}]} {
+    return
+}
+
+# Run until there is a tail call frame in the backtrace.  Then use the
+# 'until' command with a line that will not be reached until after the
+# current stack has unwound back to main.  We expect the until's out
+# of scope breakpoint to be hit first.
+#
+# When USE_PARENT_FRAME_P is true we move up to the tail call frame
+# and issue the 'until' command while that frame is selected.
+#
+# When USE_PARENT_FRAME_P is false we issue the 'until' command while
+# normal_function is selected, this function is called from the tail
+# call function.
+#
+# In both cases, we expect the out of scope breakpoint to be placed
+# back in main, which is where the inferior should be stopped.
+proc run_test { use_parent_frame_p } {
+    clean_restart $::testfile
+
+    if {![runto normal_function]} {
+	return
+    }
+
+    # Check the stack looks correct.
+    gdb_test "bt" \
+	[multi_line \
+	     "#0\\s+normal_function\[^\r\n\]+" \
+	     "#1\\s+(?:$::hex in )?tailcall_function\[^\r\n\]+" \
+	     "#2\\s+(?:$::hex in )?main\[^\r\n\]+"] \
+	"check call stack"
+
+    # Move up to tailcall_function if needed.
+    if { $use_parent_frame_p } {
+	gdb_test "up" ".* in tailcall_function .*" \
+	    "move up to tailcall_function"
+    }
+
+    # Where we ask the 'until' to run to.
+    set until_lineno [gdb_get_line_number "Run until here."]
+
+    # Where we actually expect the inferior to stop.
+    set stop_lineno [gdb_get_line_number "Should stop here."]
+
+    # Issue the 'until' command and check that the inferior stops in main
+    # when the until guard breakpoint notices we have left the
+    # tailcall_function.
+    set saw_stop_location false
+    set saw_stop_source false
+    gdb_test_multiple "until $until_lineno" "run until worker_func" {
+	-re "^until $until_lineno\r\n" {
+	    exp_continue
+	}
+	-re "^$::gdb_prompt $" {
+	    gdb_assert { $saw_stop_location && $saw_stop_source } \
+		$gdb_test_name
+	}
+	-re "^main \\(\\) at \[^\r\n\]+/$::srcfile:$stop_lineno\r\n" {
+	    set saw_stop_location true
+	    exp_continue
+	}
+	-re "^$stop_lineno\\s+\[^\r\n\]+\r\n" {
+	    set saw_stop_source true
+	    exp_continue
+	}
+    }
+}
+
+foreach_with_prefix use_parent_frame { true false } {
+    run_test $use_parent_frame
+}
diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.c b/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.c
new file mode 100644
index 00000000000..d129d7e41b4
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.c
@@ -0,0 +1,45 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 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/>.  */
+
+/* This test relies on tailcall_function actually compiling to a tail call
+   function.  we try to force this by preventing tailcall_function and
+   normal_function from being inlined, then compiling this file at -O2.
+   Still, that's no guarantee.  If tailcall_function isn't a tail call,
+   then the test becomes pointless.  */
+
+volatile int global_var = 42;
+
+int __attribute__ ((noinline, noclone))
+normal_function (int x)
+{
+  return x + 1;
+}
+
+int __attribute__ ((noinline, noclone))
+tailcall_function (int x)
+{
+  ++global_var;
+  return normal_function (x);
+}
+
+int
+main (void)
+{
+  int result = tailcall_function (42);
+  result -= global_var;			/* Temporary breakpoint here.  */
+  return result;
+}
diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.exp
new file mode 100644
index 00000000000..e5b5ebc5183
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.exp
@@ -0,0 +1,93 @@
+# Copyright (C) 2026 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/>.
+
+# Check that FinishBreakpoints are reached when set for a tailcall
+# frame.
+#
+# Frame #0 is never a tailcall frame.  As such, we need to do more
+# than just stop in a tailcall function and create a finish
+# breakpoint.  Instead, we need to stop in a function called from a
+# tail call function, then walk back up the stack and create the
+# finish breakpoint on the tail call frame.
+#
+# At one point, GDB would create the breakpoint in the correct
+# location, but set the frame-id on the breakpoint for the wrong
+# frame, with the result that the breakpoint would never trigger.
+
+load_lib gdb-python.exp
+
+require allow_python_tests
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile \
+	 {debug optimize=-O2}]} {
+    return
+}
+
+# For remote host testing.
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+# Run to 'normal_function' and then try to create a FinishBreakpoint
+# in the parent frame.
+proc run_test {} {
+    clean_restart $::testfile
+
+    if {![runto normal_function]} {
+	return
+    }
+
+    gdb_test "bt" \
+	[multi_line \
+	     "#0\\s+normal_function\[^\r\n\]+" \
+	     "#1\\s+(?:$::hex in )?tailcall_function\[^\r\n\]+" \
+	     "#2\\s+(?:$::hex in )?main\[^\r\n\]+"] \
+	"check call stack"
+
+    gdb_test "source $::pyfile" "Python script imported" "import python scripts"
+
+    set lineno [gdb_get_line_number "Temporary breakpoint here."]
+    gdb_test "python MyFinishBreakpoint(gdb.selected_frame().older())" \
+	"Temporary breakpoint $::decimal at $::hex: file \[^\r\n\]+/$::srcfile, line $lineno\\." \
+	"create finish breakpoint"
+
+    set saw_stopped_message false
+    set saw_breakpoint_line false
+    set saw_source_line false
+    gdb_test_multiple "continue" "" {
+	-re "^Stopped at MyFinishBreakpoint\r\n" {
+	    set saw_stopped_message true
+	    exp_continue
+	}
+	-re "^Breakpoint $::decimal, main \\(\\) at \[^\r\n\]+/$::srcfile:$lineno\r\n" {
+	    set saw_breakpoint_line true
+	    exp_continue
+	}
+	-re "^$lineno\\s+\[^\r\n\]+\r\n" {
+	    set saw_source_line true
+	    exp_continue
+	}
+	-re "^$::gdb_prompt $" {
+	    gdb_assert { $saw_stopped_message \
+			     && $saw_breakpoint_line \
+			     && $saw_source_line } $gdb_test_name
+	}
+	-re "^\[^\r\n\]*\r\n" {
+	    exp_continue
+	}
+    }
+}
+
+run_test
diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.py b/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.py
new file mode 100644
index 00000000000..bd10109a3dc
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint-tailcall.py
@@ -0,0 +1,26 @@
+# Copyright (C) 2026 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/>.
+
+
+import gdb
+
+
+class MyFinishBreakpoint(gdb.FinishBreakpoint):
+    def stop(self):
+        print("Stopped at MyFinishBreakpoint")
+        return True
+
+
+print("Python script imported")
-- 
2.25.4


  reply	other threads:[~2026-03-05 13:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-24 11:29 [PATCH 0/4] Fixes for tail call, until, and FinishBreakpoints Andrew Burgess
2026-01-24 11:29 ` [PATCH 1/4] gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls Andrew Burgess
2026-01-27 16:37   ` Andrew Burgess
2026-01-24 11:29 ` [PATCH 2/4] gdb/python: fix FinishBreakpoint.return_value for tail call functions Andrew Burgess
2026-01-24 11:29 ` [PATCH 3/4] gdb/python: don't allow FinishBreakpoints for inline frames Andrew Burgess
2026-01-24 12:23   ` Eli Zaretskii
2026-01-24 11:29 ` [PATCH 4/4] gdb/python: fix gdb.FinishBreakpoint returning to a tail call frame Andrew Burgess
2026-03-05 13:37 ` [PATCHv2 0/4] Fixes for tail call, until, and FinishBreakpoints Andrew Burgess
2026-03-05 13:37   ` Andrew Burgess [this message]
2026-03-05 13:37   ` [PATCHv2 2/4] gdb/python: fix FinishBreakpoint.return_value for tail call functions Andrew Burgess
2026-03-05 13:37   ` [PATCHv2 3/4] gdb/python: don't allow FinishBreakpoints for inline frames Andrew Burgess
2026-03-05 13:37   ` [PATCHv2 4/4] gdb/python: fix gdb.FinishBreakpoint returning to a tail call frame Andrew Burgess
2026-03-05 15:59   ` [PATCHv2 0/4] Fixes for tail call, until, and FinishBreakpoints Tom Tromey
2026-03-05 17:49     ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1259b09af0b2ca1e326d81aac4a25bc0ae73c111.1772717770.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox