]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb/python: handle saving user registers in a frame unwinder
authorAndrew Burgess <andrew.burgess@embecosm.com>
Wed, 26 May 2021 14:24:04 +0000 (15:24 +0100)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Mon, 21 Jun 2021 15:09:05 +0000 (16:09 +0100)
This patch came about because I wanted to write a frame unwinder that
would corrupt the backtrace in a particular way.  In order to achieve
what I wanted I ended up trying to write an unwinder like this:

  class FrameId(object):
      .... snip class definition ....

  class TestUnwinder(Unwinder):
      def __init__(self):
          Unwinder.__init__(self, "some name")

      def __call__(self, pending_frame):
          pc_desc = pending_frame.architecture().registers().find("pc")
          pc = pending_frame.read_register(pc_desc)

          sp_desc = pending_frame.architecture().registers().find("sp")
          sp = pending_frame.read_register(sp_desc)

          # ... snip code to decide if this unwinder applies or not.

          fid = FrameId(pc, sp)
          unwinder = pending_frame.create_unwind_info(fid)
          unwinder.add_saved_register(pc_desc, pc)
          unwinder.add_saved_register(sp_desc, sp)
          return unwinder

The important things here are the two calls:

          unwinder.add_saved_register(pc_desc, pc)
          unwinder.add_saved_register(sp_desc, sp)

On x86-64 these would fail with an assertion error:

  gdb/regcache.c:168: internal-error: int register_size(gdbarch*, int): Assertion `regnum >= 0 && regnum < gdbarch_num_cooked_regs (gdbarch)' failed.

What happens is that in unwind_infopy_add_saved_register (py-unwind.c)
we call register_size, as register_size should only be called on
cooked (real or pseudo) registers, and 'pc' and 'sp' are implemented
as user registers (at least on x86-64), we trigger the assertion.

A simple fix would be to check in unwind_infopy_add_saved_register if
the register number we are handling is a cooked register or not, if
not we can throw a 'Bad register' error back to the Python code.

However, I think we can do better.

Consider that at the CLI we can do this:

  (gdb) set $pc=0x1234

This works because GDB first evaluates '$pc' to get a register value,
then evaluates '0x1234' to create a value encapsulating the
immediate.  The contents of the immediate value are then copied back
to the location of the register value representing '$pc'.

The value location for a user-register will (usually) be the location
of the real register that was accessed, so on x86-64 we'd expect this
to be $rip.

So, in this patch I propose that in the unwinder code, when
add_saved_register is called, if it is passed a
user-register (i.e. non-cooked) then we first fetch the register,
extract the real register number from the value's location, and use
that new register number when handling the add_saved_register call.

If either the value location that we get for the user-register is not
a cooked register then we can throw a 'Bad register' error back to the
Python code, but in most cases this will not happen.

gdb/ChangeLog:

* python/py-unwind.c (unwind_infopy_add_saved_register): Handle
saving user registers.

gdb/testsuite/ChangeLog:

* gdb.python/py-unwind-user-regs.c: New file.
* gdb.python/py-unwind-user-regs.exp: New file.
* gdb.python/py-unwind-user-regs.py: New file.

gdb/ChangeLog
gdb/python/py-unwind.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.python/py-unwind-user-regs.c [new file with mode: 0644]
gdb/testsuite/gdb.python/py-unwind-user-regs.exp [new file with mode: 0644]
gdb/testsuite/gdb.python/py-unwind-user-regs.py [new file with mode: 0644]

index 98c6ca7413a9ba23053979dac16ed2b200e90516..a221a841e0afd2dcd648003542e77fd3126d2201 100644 (file)
@@ -1,3 +1,8 @@
+2021-06-21  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       * python/py-unwind.c (unwind_infopy_add_saved_register): Handle
+       saving user registers.
+
 2021-06-19  Mike Frysinger  <vapier@gentoo.org>
 
        * acinclude.m4: Delete most m4_include's of ../config files.
index 7c195eb539daec7ba89bcea59e0fe1e8f0113853..d6e2f85dbc1928ef3d605273adbd98129dd50c10 100644 (file)
@@ -27,6 +27,7 @@
 #include "python-internal.h"
 #include "regcache.h"
 #include "valprint.h"
+#include "user-regs.h"
 
 /* Debugging of Python unwinders.  */
 
@@ -265,6 +266,26 @@ unwind_infopy_add_saved_register (PyObject *self, PyObject *args)
       PyErr_SetString (PyExc_ValueError, "Bad register");
       return NULL;
     }
+
+  /* If REGNUM identifies a user register then *maybe* we can convert this
+     to a real (i.e. non-user) register.  The maybe qualifier is because we
+     don't know what user registers each target might add, however, the
+     following logic should work for the usual style of user registers,
+     where the read function just forwards the register read on to some
+     other register with no adjusting the value.  */
+  if (regnum >= gdbarch_num_cooked_regs (pending_frame->gdbarch))
+    {
+      struct value *user_reg_value
+       = value_of_user_reg (regnum, pending_frame->frame_info);
+      if (VALUE_LVAL (user_reg_value) == lval_register)
+       regnum = VALUE_REGNUM (user_reg_value);
+      if (regnum >= gdbarch_num_cooked_regs (pending_frame->gdbarch))
+       {
+         PyErr_SetString (PyExc_ValueError, "Bad register");
+         return NULL;
+       }
+    }
+
   {
     struct value *value;
     size_t data_size;
index e8c797a8b85516ad267dff95fe51c3c2c39c58b1..ddd8b1391d15c0b322687c9443b493c08ce826db 100644 (file)
@@ -1,3 +1,9 @@
+2021-06-21  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       * gdb.python/py-unwind-user-regs.c: New file.
+       * gdb.python/py-unwind-user-regs.exp: New file.
+       * gdb.python/py-unwind-user-regs.py: New file.
+
 2021-06-17  Carl Love  <cel@us.ibm.com>
 
        * gdb.arch/powerpc-power8.exp(bctar, bctarl): Update mnemonics
diff --git a/gdb/testsuite/gdb.python/py-unwind-user-regs.c b/gdb/testsuite/gdb.python/py-unwind-user-regs.c
new file mode 100644 (file)
index 0000000..8d1efd1
--- /dev/null
@@ -0,0 +1,37 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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;
+
+void __attribute__ ((noinline))
+foo (void)
+{
+  ++global_var;                /* Break here.  */
+}
+
+void __attribute__ ((noinline))
+bar (void)
+{
+  foo ();
+}
+
+int
+main (void)
+{
+  bar ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind-user-regs.exp b/gdb/testsuite/gdb.python/py-unwind-user-regs.exp
new file mode 100644 (file)
index 0000000..7ae3a5b
--- /dev/null
@@ -0,0 +1,98 @@
+# Copyright (C) 2021 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/>.
+
+# Setup an unwinder that uses gdb.UnwindInfo.add_saved_register with
+# the register's 'pc' and 'sp'.  On some (all?) targets, these
+# registers are implemented as user-registers, and so can't normally
+# be written to directly.
+#
+# The Python unwinder now includes code similar to how the expression
+# evaluator would handle something like 'set $pc=0x1234', we fetch the
+# value of '$pc', and then use the value's location to tell us which
+# register to write to.
+#
+# The unwinder defined here deliberately breaks the unwind by setting
+# the unwound $pc and $sp to be equal to the current frame's $pc and
+# $sp.  GDB will spot this as a loop in the backtrace and terminate
+# the unwind.
+#
+# However, by the time the unwind terminates we have already shown
+# that it is possible to call add_saved_register with a user-register,
+# so the test is considered passed.
+#
+# For completeness this test checks two cases, calling
+# add_saved_register with a gdb.RegisterDescriptor and calling
+# add_saved_register with a string containing the register name.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+gdb_breakpoint [gdb_get_line_number "Break here"]
+gdb_continue_to_breakpoint "stop at test breakpoint"
+
+# Load the script containing the unwinders.  There are actually two
+# unwinders defined here that will catch the same function, so we
+# immediately disable one of the unwinders.
+gdb_test_no_output "source ${pyfile}"\
+    "import python scripts"
+gdb_test "disable unwinder global \"break unwinding using strings\"" \
+    "1 unwinder disabled" "disable the unwinder that uses strings"
+
+# At this point we are using the unwinder that passes a
+# gdb.RegisterDescriptor to add_saved_register.
+gdb_test_sequence "bt"  "Backtrace corrupted by descriptor based unwinder" {
+    "\\r\\n#0 \[^\r\n\]* foo \\(\\) at "
+    "\\r\\n#1 \[^\r\n\]* bar \\(\\) at "
+    "Backtrace stopped: previous frame inner to this frame \\(corrupt stack\\?\\)"
+}
+
+# Disable the unwinder that calls add_saved_register with a
+# gdb.RegisterDescriptor, and enable the unwinder that calls
+# add_saved_register with a string (containing the register name).
+gdb_test "disable unwinder global \"break unwinding using descriptors\"" \
+    "1 unwinder disabled" "disable the unwinder that uses descriptors"
+gdb_test "enable unwinder global \"break unwinding using strings\"" \
+    "1 unwinder enabled" "enable the unwinder that uses strings"
+gdb_test_sequence "bt"  "Backtrace corrupted by string based unwinder" {
+    "\\r\\n#0 \[^\r\n\]* foo \\(\\) at "
+    "\\r\\n#1 \[^\r\n\]* bar \\(\\) at "
+    "Backtrace stopped: previous frame inner to this frame \\(corrupt stack\\?\\)"
+}
+
+# Just for completeness, disable the string unwinder again (neither of
+# our special unwinders are now enabled), and check the backtrace.  We
+# now get the complete stack back to main.
+gdb_test "disable unwinder global \"break unwinding using strings\"" \
+    "1 unwinder disabled" "disable the unwinder that uses strings again"
+gdb_test_sequence "bt"  "Backtrace not corrupted when using no unwinder" {
+    "\\r\\n#0 \[^\r\n\]* foo \\(\\) at "
+    "\\r\\n#1 \[^\r\n\]* bar \\(\\) at "
+    "\\r\\n#2 \[^\r\n\]* main \\(\\) at "
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind-user-regs.py b/gdb/testsuite/gdb.python/py-unwind-user-regs.py
new file mode 100644 (file)
index 0000000..c64c6c0
--- /dev/null
@@ -0,0 +1,72 @@
+# Copyright (C) 2021 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
+from gdb.unwinder import Unwinder
+
+
+class FrameId(object):
+    def __init__(self, sp, pc):
+        self._sp = sp
+        self._pc = pc
+
+    @property
+    def sp(self):
+        return self._sp
+
+    @property
+    def pc(self):
+        return self._pc
+
+
+class TestUnwinder(Unwinder):
+    def __init__(self, use_descriptors):
+        if use_descriptors:
+            tag = "using descriptors"
+        else:
+            tag = "using strings"
+
+        Unwinder.__init__(self, "break unwinding %s" % tag)
+        self._use_descriptors = use_descriptors
+
+    def __call__(self, pending_frame):
+        pc_desc = pending_frame.architecture().registers().find("pc")
+        pc = pending_frame.read_register(pc_desc)
+
+        sp_desc = pending_frame.architecture().registers().find("sp")
+        sp = pending_frame.read_register(sp_desc)
+
+        block = gdb.block_for_pc(int(pc))
+        if block is None:
+            return None
+        func = block.function
+        if func is None:
+            return None
+        if str(func) != "bar":
+            return None
+
+        fid = FrameId(pc, sp)
+        unwinder = pending_frame.create_unwind_info(fid)
+        if self._use_descriptors:
+            unwinder.add_saved_register(pc_desc, pc)
+            unwinder.add_saved_register(sp_desc, sp)
+        else:
+            unwinder.add_saved_register("pc", pc)
+            unwinder.add_saved_register("sp", sp)
+        return unwinder
+
+
+gdb.unwinder.register_unwinder(None, TestUnwinder(True), True)
+gdb.unwinder.register_unwinder(None, TestUnwinder(False), True)