[PATCH] gdb: fix "frame function" issue when call is last instruction

[PATCH] gdb: fix "frame function" issue when call is last instruction

[Sahil Siddiq]

Currently, the "frame function" fails when the last
instruction is a call. In such a case, the $rip
register points to an address that lies outside the
frame.

Using "get_frame_address_in_block" instead of
"get_frame_pc" resolves this issue.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30929
---
Hi,

This is my first time contributing to gdb. I have
tried to adhere to the styling convention. Please
let me know if I have missed something.

Instead of extending the existing frame-selection.exp
test, I thought I would create a new test since the C
file is new and it seemed like the implementation of
the test isn't similar enough to be kept in the existing
exp file. Please let me know if this is undesirable.

Thanks,
Sahil

 gdb/stack.c                                   |   4 +-
 .../frame-selection-last-instr-call.c         |  28 ++++
 .../frame-selection-last-instr-call.exp       | 139 ++++++++++++++++++
 3 files changed, 169 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/frame-selection-last-instr-call.c
 create mode 100644 gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp

diff --git a/gdb/stack.c b/gdb/stack.c
index b36193be2f..8249866468 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2860,8 +2860,8 @@ find_frame_for_function (const char *function_name)
   do
     {
       for (size_t i = 0; (i < sals.size () && !found); i++)
-	found = (get_frame_pc (frame) >= func_bounds[i].low
-		 && get_frame_pc (frame) < func_bounds[i].high);
+	found = (get_frame_address_in_block (frame) >= func_bounds[i].low
+		 && get_frame_address_in_block (frame) < func_bounds[i].high);
       if (!found)
 	{
 	  level = 1;
diff --git a/gdb/testsuite/gdb.base/frame-selection-last-instr-call.c b/gdb/testsuite/gdb.base/frame-selection-last-instr-call.c
new file mode 100644
index 0000000000..4f056332af
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frame-selection-last-instr-call.c
@@ -0,0 +1,28 @@
+/* Copyright 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+void
+frame_1 (void)
+{
+  __builtin_abort();
+}
+
+int
+main (void)
+{
+  frame_1 ();
+}
diff --git a/gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp b/gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp
new file mode 100644
index 0000000000..e5e86e0d6f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp
@@ -0,0 +1,139 @@
+# Copyright 2024 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 tests GDB's frame selection as used by the 'frame',
+# 'select-frame', and 'info frame' commands in the corner case
+# when the last instruction in a frame is a call.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}]} {
+    return -1
+}
+
+runto_main
+gdb_test "continue" "Continuing.*Program received signal SIGABRT,.*" \
+    "received signal SIGABRT"
+
+gdb_test "bt" \
+    ".*#3  $hex in frame_1 .*#4  $hex in main .*" "backtrace at breakpoint"
+
+# Perform "info frame" to extract the frame's address.
+proc get_frame_address { {testname ""} } {
+    global hex gdb_prompt
+
+    set frame_address "unknown"
+    set testname "get_frame_address: ${testname}"
+    gdb_test_multiple "info frame" $testname {
+	-re ", frame at ($hex):\r\n.*\r\n$gdb_prompt $" {
+	    set frame_address $expect_out(1,string)
+	    pass $testname
+	}
+    }
+
+    return $frame_address
+}
+
+# Check that the current frame is at stack depth LEVEL, at frame
+# address ADDRESS, and is in FUNCTION.
+proc check_frame { level address function } {
+    global hex gdb_prompt
+
+    if {$function == "abort"} {
+      set re [multi_line \
+	"Stack level ${level}, frame at ($address):" \
+	".* = $hex in ${function}; saved .* = $hex" \
+	".*\r\n$gdb_prompt $" ]
+
+    } else {
+      set re [multi_line \
+	"Stack level ${level}, frame at ($address):" \
+	".* = $hex in ${function} \(\[^\r\n\]*\); saved .* = $hex" \
+	".*\r\n$gdb_prompt $" ]
+    }
+
+    set testname "check frame level ${level}"
+    gdb_test_multiple "info frame" $testname {
+	-re $re {
+	    pass $testname
+	}
+    }
+}
+
+# Select frame using level, but relying on this being the default
+# action, so "frame 0" performs "frame level 0".
+gdb_test "frame 2" "#2  $hex in abort.*"
+set frame_2_address [ get_frame_address "frame 2" ]
+gdb_test "frame 3" "#3  $hex in frame_1.*"
+set frame_3_address [ get_frame_address "frame 3" ]
+gdb_test "frame 4" "#4  $hex in main.*"
+set frame_4_address [ get_frame_address "frame 4" ]
+
+# Select frame using 'level' specification.
+gdb_test "frame level 2" "#2  $hex in abort.*"
+gdb_test "frame level 3" "#3  $hex in frame_1.*"
+gdb_test "frame level 4" "#4  $hex in main.*"
+
+# Select frame by address.
+gdb_test "frame address ${frame_2_address}" "#2  $hex in abort.*" \
+    "select frame 2 by address"
+gdb_test "frame address ${frame_3_address}" "#3  $hex in frame_1.*" \
+    "select frame 3 by address"
+gdb_test "frame address ${frame_4_address}" "#4  $hex in main.*" \
+    "select frame 4 by address"
+
+# Select frame by function.
+gdb_test "frame function abort" "#2  $hex in abort.*"
+gdb_test "frame function frame_1" "#3  $hex in frame_1.*"
+gdb_test "frame function main" "#4  $hex in main.*"
+
+with_test_prefix "select-frame, no keyword" {
+    gdb_test_no_output "select-frame 2"
+    check_frame "2" "${frame_2_address}" "abort"
+    gdb_test_no_output "select-frame 3"
+    check_frame "3" "${frame_3_address}" "frame_1"
+    gdb_test_no_output "select-frame 4"
+    check_frame "4" "${frame_4_address}" "main"
+}
+
+with_test_prefix "select-frame, keyword=level" {
+    gdb_test_no_output "select-frame level 2"
+    check_frame "2" "${frame_2_address}" "abort"
+    gdb_test_no_output "select-frame level 3"
+    check_frame "3" "${frame_3_address}" "frame_1"
+    gdb_test_no_output "select-frame level 4"
+    check_frame "4" "${frame_4_address}" "main"
+}
+
+with_test_prefix "select-frame, keyword=address" {
+    gdb_test_no_output "select-frame address ${frame_2_address}" \
+	"select frame 2 by address"
+    check_frame "2" "${frame_2_address}" "abort"
+    gdb_test_no_output "select-frame address ${frame_3_address}" \
+	"select frame 3 by address"
+    check_frame "3" "${frame_3_address}" "frame_1"
+    gdb_test_no_output "select-frame address ${frame_4_address}" \
+	"select frame 4 by address"
+    check_frame "4" "${frame_4_address}" "main"
+}
+
+with_test_prefix "select-frame, keyword=function" {
+    gdb_test_no_output "select-frame function abort"
+    check_frame "2" "${frame_2_address}" "abort"
+    gdb_test_no_output "select-frame function frame_1"
+    check_frame "3" "${frame_3_address}" "frame_1"
+    gdb_test_no_output "select-frame function main"
+    check_frame "4" "${frame_4_address}" "main"
+}
-- 
2.45.2

Re: [PATCH] gdb: fix "frame function" issue when call is last instruction

[Sahil]

Hi,

On Sunday, June 30, 2024 7:38:08 PM GMT+5:30 Sahil Siddiq wrote:
> Currently, the "frame function" fails when the last
> instruction is a call. In such a case, the $rip
> register points to an address that lies outside the
> frame.
> 
> Using "get_frame_address_in_block" instead of
> "get_frame_pc" resolves this issue.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30929
> ---
> Hi,
> 
> This is my first time contributing to gdb. I have
> tried to adhere to the styling convention. Please
> let me know if I have missed something.
> 
> Instead of extending the existing frame-selection.exp
> test, I thought I would create a new test since the C
> file is new and it seemed like the implementation of
> the test isn't similar enough to be kept in the existing
> exp file. Please let me know if this is undesirable.
> 
> Thanks,
> Sahil
> 
>  gdb/stack.c                                   |   4 +-
>  .../frame-selection-last-instr-call.c         |  28 ++++
>  .../frame-selection-last-instr-call.exp       | 139 ++++++++++++++++++
>  3 files changed, 169 insertions(+), 2 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/frame-selection-last-instr-call.c
> create mode 100644
> gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp
> [...]

I noticed that the test introduced in this patch fails in some environments. See
for example:
1. regressions on arm [1]
2. regressions on aarch64 [2]

I have submitted a new patch where the test should run successfully regardless
of the build environment.

Thanks,
Sahil

[1] https://ci.linaro.org/job/tcwg_gdb_check--master-arm-precommit/2800/
[2] https://ci.linaro.org/job/tcwg_gdb_check--master-aarch64-precommit/2814/