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: [PATCH 1/4] gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls
Date: Sat, 24 Jan 2026 11:29:11 +0000	[thread overview]
Message-ID: <d756e1c27ccf1dc12bf4516671b743678808c602.1769253423.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1769253423.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.
---
 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 8cb1d0a5c42..f80b3e281b6 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-01-24 11:30 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 ` Andrew Burgess [this message]
2026-01-27 16:37   ` [PATCH 1/4] gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls 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   ` [PATCHv2 1/4] gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls Andrew Burgess
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=d756e1c27ccf1dc12bf4516671b743678808c602.1769253423.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