Re: [PATCH 1/4] gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls

[Andrew Burgess <aburgess@redhat.com>]

In another thread Tom T. pointed my to bug PR gdb/28683 which should (I
think) be fixed by this patch.

I've updated the commit message to mention this bug, and add the Bug:
link.  I've not changed anything else about the patch.

Thanks,
Andrew

---

commit 4222ade61869c7550a4ed28ab81ff58dc70069c5
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Jan 22 17:51:11 2026 +0000

    gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls
    
    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

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