public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Use get_frame_address_in_block in print_frame
@ 2023-08-04 17:36 Tom Tromey
  2023-08-26  2:33 ` Kevin Buettner
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Tromey @ 2023-08-04 17:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The author of 'mold' pointed out that with a certain shared library,
gdb would fail to find the shared library's name in 'bt'.

The function in question appeared at the end of the .so's .text
segment and ended with a call to 'abort'.

This turned out to be a classic case of calling get_frame_pc when
get_frame_address_in_block is needed -- the former will be off-by-one
for purposes of finding the enclosing function or shared library.

The included test fails without the patch on my system.  However, I
imagine it can't be assumed to reliably fail.  Nevertheless it seemed
worth doing.

Regression tested on x86-64 Fedora 38.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29074
---
 gdb/stack.c                              |  2 +-
 gdb/testsuite/gdb.base/solib-abort-lib.c | 23 ++++++++++
 gdb/testsuite/gdb.base/solib-abort.c     | 25 +++++++++++
 gdb/testsuite/gdb.base/solib-abort.exp   | 56 ++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/solib-abort-lib.c
 create mode 100644 gdb/testsuite/gdb.base/solib-abort.c
 create mode 100644 gdb/testsuite/gdb.base/solib-abort.exp

diff --git a/gdb/stack.c b/gdb/stack.c
index 002bf580634..0b35d62f82f 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1420,7 +1420,7 @@ print_frame (const frame_print_options &fp_opts,
       {
 	const char *lib
 	  = solib_name_from_address (get_frame_program_space (frame),
-				     get_frame_pc (frame));
+				     get_frame_address_in_block (frame));
 
 	if (lib)
 	  {
diff --git a/gdb/testsuite/gdb.base/solib-abort-lib.c b/gdb/testsuite/gdb.base/solib-abort-lib.c
new file mode 100644
index 00000000000..22543230d3b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-abort-lib.c
@@ -0,0 +1,23 @@
+/* Copyright 2023 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/>.
+*/
+
+#include <stdlib.h>
+
+void
+callee (void)
+{
+  abort ();
+}
diff --git a/gdb/testsuite/gdb.base/solib-abort.c b/gdb/testsuite/gdb.base/solib-abort.c
new file mode 100644
index 00000000000..42f4dc54ed6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-abort.c
@@ -0,0 +1,25 @@
+/* Copyright 2023 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/>.
+*/
+
+extern void callee (void);
+
+int
+main ()
+{
+  callee ();
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/solib-abort.exp b/gdb/testsuite/gdb.base/solib-abort.exp
new file mode 100644
index 00000000000..fcc229623bc
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-abort.exp
@@ -0,0 +1,56 @@
+# Copyright 2023 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/>.
+
+# Test corner case of printing solib name when unwinding.
+# https://sourceware.org/bugzilla/show_bug.cgi?id=29074
+
+require allow_shlib_tests
+
+# Library file.
+set libname "solib-abort-lib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib [standard_output_file ${libname}.so]
+# Note: no debugging info here, since this will assure that the solib
+# name is printed in the stack trace.
+set lib_flags {}
+
+# Binary file.
+set testfile "solib-abort"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile [standard_output_file ${testfile}]
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != ""
+     || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+  untested "failed to compile"
+  return -1
+}
+
+clean_restart $binfile
+
+if {![runto_main]} {
+  return 0
+}
+
+# Run until the program dies.
+gdb_test "cont" "Program received signal SIGABRT,.*"
+
+# The solib name should show up in the stack trace.  The bug here was
+# that if the function calling abort appeared last in the text
+# section, and if GCC didn't emit an epilogue after the call, then gdb
+# would use the wrong PC to find the solib name.  This test doesn't
+# exactly test this in all situations, but with the correct
+# environment it is sufficient.
+gdb_test "bt" "#$decimal .* in callee .* from .*${libname}\\.so.*"
-- 
2.41.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Use get_frame_address_in_block in print_frame
  2023-08-04 17:36 [PATCH] Use get_frame_address_in_block in print_frame Tom Tromey
@ 2023-08-26  2:33 ` Kevin Buettner
  0 siblings, 0 replies; 2+ messages in thread
From: Kevin Buettner @ 2023-08-26  2:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Fri,  4 Aug 2023 11:36:35 -0600
Tom Tromey <tom@tromey.com> wrote:

> The author of 'mold' pointed out that with a certain shared library,
> gdb would fail to find the shared library's name in 'bt'.
> 
> The function in question appeared at the end of the .so's .text
> segment and ended with a call to 'abort'.
> 
> This turned out to be a classic case of calling get_frame_pc when
> get_frame_address_in_block is needed -- the former will be off-by-one
> for purposes of finding the enclosing function or shared library.
> 
> The included test fails without the patch on my system.  However, I
> imagine it can't be assumed to reliably fail.  Nevertheless it seemed
> worth doing.
> 
> Regression tested on x86-64 Fedora 38.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29074

LGTM.

Reviewed-by: Kevin Buettner <kevinb@redhat.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-08-26  2:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04 17:36 [PATCH] Use get_frame_address_in_block in print_frame Tom Tromey
2023-08-26  2:33 ` Kevin Buettner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).