[PATCH 5/5] gdb: Fix Python unwinders and inline frames
Andrew Burgess
andrew.burgess@embecosm.com
Wed Jun 17 17:38:09 GMT 2020
I observed that when making use of a Python frame unwinder, if the
frame sniffing code accessed any registers within an inline frame then
GDB would crash with this error:
gdb/frame.c:579: internal-error: frame_id get_frame_id(frame_info*): Assertion `fi->level == 0' failed.
The problem is that, when in the Python unwinder I write this:
pending_frame.read_register ("register-name")
This is translated internally into a call to `value_of_register',
which in turn becomes a call to `value_of_register_lazy'.
Usually this isn't a problem, `value_of_register_lazy' requires the
next frame (more inner) to have a valid frame_id, which will be the
case (if we're sniffing frame #1, then frame #0 will have had its
frame-id figured out).
Unfortunately if frame #0 is inline within frame #1, then the frame-id
for frame #0 _is_ same as the frame-id for frame #1 - the frame-id for
frame #0 requires us to first compute the frame-id for frame #1. As a
result it is not same to call `value_of_register_lazy' before the
frame-id of the current frame is known.
The solution I propose here is that `value_of_register' stops making a
lazy register value (the step that requires the frame-id, and thus
causes the problems), and instead calls `get_frame_register_value'.
This avoids the need to calculate the frame-id and so avoids the
problem.
This bug has crept in because we allowed calls to the function
`value_of_register_lazy' at a time when the frame-id of the frame
passed to the function didn't have its frame-id calculated. This is
only ever a problem for inline frames. To prevent bugs like this
getting into GDB in the future I've added an assert to the function
`value_of_register_lazy' to require the current frame have its id
calculated.
If we are calling from outside of the frame sniffer then this will
_always_ be true, so is not a problem. If we are calling this
function from within a frame sniffer then it will always trigger.
gdb/ChangeLog:
* findvar.c (value_of_register): Use get_frame_register_value
rather than value_of_register_lazy.
(value_of_register_lazy): Add new assert, and update comments.
gdb/testsuite/ChangeLog:
* gdb.python/py-unwind-inline.c: New file.
* gdb.python/py-unwind-inline.exp: New file.
* gdb.python/py-unwind-inline.py: New file.
---
gdb/ChangeLog | 6 ++
gdb/findvar.c | 28 ++++++--
gdb/testsuite/ChangeLog | 6 ++
gdb/testsuite/gdb.python/py-unwind-inline.c | 37 ++++++++++
gdb/testsuite/gdb.python/py-unwind-inline.exp | 49 +++++++++++++
gdb/testsuite/gdb.python/py-unwind-inline.py | 71 +++++++++++++++++++
6 files changed, 192 insertions(+), 5 deletions(-)
create mode 100644 gdb/testsuite/gdb.python/py-unwind-inline.c
create mode 100644 gdb/testsuite/gdb.python/py-unwind-inline.exp
create mode 100644 gdb/testsuite/gdb.python/py-unwind-inline.py
diff --git a/gdb/findvar.c b/gdb/findvar.c
index c7cd31ce1a6..e5445b34930 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -263,16 +263,13 @@ struct value *
value_of_register (int regnum, struct frame_info *frame)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
- struct value *reg_val;
/* User registers lie completely outside of the range of normal
registers. Catch them early so that the target never sees them. */
if (regnum >= gdbarch_num_cooked_regs (gdbarch))
return value_of_user_reg (regnum, frame);
- reg_val = value_of_register_lazy (frame, regnum);
- value_fetch_lazy (reg_val);
- return reg_val;
+ return get_frame_register_value (frame, regnum);
}
/* Return a `value' with the contents of (virtual or cooked) register
@@ -292,7 +289,28 @@ value_of_register_lazy (struct frame_info *frame, int regnum)
next_frame = get_next_frame_sentinel_okay (frame);
- /* We should have a valid next frame. */
+ /* If NEXT_FRAME is inline within FRAME, then calculating the frame-id
+ for the next frame will require that FRAME has a valid frame-id.
+ Usually this is not a problem, however, if this function is ever
+ called from a frame sniffer, the small window of time when not all
+ frames have yet had their frame-id calculated, this function will
+ trigger an assert within get_frame_id that a frame at a level > 0
+ doesn't have a frame id.
+
+ Instead of waiting for an inline frame to reveal an invalid use of
+ this function, we instead demand that FRAME must have had its frame-id
+ calculated before we use this function.
+
+ The one time it is safe to call this function when FRAME does not yet
+ have a frame id is when FRAME is at level 0, in this case the
+ assertion in get_frame_id doesn't fire, and instead get_frame_id will
+ correctly compute the frame id for us. */
+ gdb_assert (frame_relative_level (frame) == 0
+ || frame_id_computed_p (frame));
+
+ /* We should have a valid next frame too. Given that FRAME is more
+ outer than NEXT_FRAME, and we know that FRAME has its ID calculated,
+ then this must always be true. */
gdb_assert (frame_id_p (get_frame_id (next_frame)));
reg_val = allocate_value_lazy (register_type (gdbarch, regnum));
diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.c b/gdb/testsuite/gdb.python/py-unwind-inline.c
new file mode 100644
index 00000000000..0cfe06dd273
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind-inline.c
@@ -0,0 +1,37 @@
+/* Copyright 2019-2020 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/>. */
+
+volatile int global_var;
+
+int __attribute__ ((noinline))
+bar ()
+{
+ return global_var;
+}
+
+static inline int __attribute__ ((always_inline))
+foo ()
+{
+ return bar ();
+}
+
+int
+main ()
+{
+ int ans;
+ global_var = 0;
+ ans = foo ();
+ return ans;
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.exp b/gdb/testsuite/gdb.python/py-unwind-inline.exp
new file mode 100644
index 00000000000..f7c65f6afc8
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind-inline.exp
@@ -0,0 +1,49 @@
+# Copyright (C) 2020 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 script tests GDB's handling of using a Python unwinder in the
+# presence of inlined frames.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+ debug] } {
+ return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+# The following tests require execution.
+if ![runto_main] then {
+ fail "can't run to main"
+ return 0
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+gdb_breakpoint "foo"
+
+gdb_test "source ${pyfile}" "Python script imported" \
+ "import python scripts"
+
+gdb_continue_to_breakpoint "foo"
+
+gdb_test_sequence "backtrace" "backtrace with dummy unwinder" {
+ "\\r\\n#0 foo \\(\\)"
+ "\\r\\n#1 main \\(\\)"
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.py b/gdb/testsuite/gdb.python/py-unwind-inline.py
new file mode 100644
index 00000000000..7206a0b2830
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind-inline.py
@@ -0,0 +1,71 @@
+# Copyright (C) 2020 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/>.
+
+# A dummy stack unwinder used for testing the Python unwinders when we
+# have inline frames. This unwinder will never claim any frames,
+# instead, all it does it try to read all registers possible target
+# registers as part of the frame sniffing process..
+
+import gdb
+from gdb.unwinder import Unwinder
+
+apb_global = None
+
+class dummy_unwinder (Unwinder):
+ """ A dummy unwinder that looks at a bunch of registers as part of
+ the unwinding process."""
+
+ class frame_id (object):
+ """ Basic frame id."""
+
+ def __init__ (self, sp, pc):
+ """ Create the frame id."""
+ self.sp = sp
+ self.pc = pc
+
+ def __init__ (self):
+ """Create the unwinder."""
+ Unwinder.__init__ (self, "dummy stack unwinder")
+ self.void_ptr_t = gdb.lookup_type("void").pointer()
+ self.regs = None
+
+ def get_regs (self, pending_frame):
+ """Return a list of register names that should be read. Only
+ gathers the list once, then caches the result."""
+ if (self.regs != None):
+ return self.regs
+
+ # Collect the names of all registers to read.
+ self.regs = list (pending_frame.architecture ()
+ .register_names ())
+
+ return self.regs
+
+ def __call__ (self, pending_frame):
+ """Actually performs the unwind, or at least sniffs this frame
+ to see if the unwinder should claim it, which is never does."""
+ try:
+ for r in (self.get_regs (pending_frame)):
+ v = pending_frame.read_register (r).cast (self.void_ptr_t)
+ except:
+ print ("Dummy unwinder, exception")
+ raise
+
+ return None
+
+# Register the ComRV stack unwinder.
+gdb.unwinder.register_unwinder (None, dummy_unwinder (), True)
+
+print ("Python script imported")
--
2.25.4
More information about the Gdb-patches
mailing list