This started with me running into the bug described in python/22748,
in summary, 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 can't be computed until we have the frame-id for #1. As
a result we can't create a lazy register for frame #1 when frame #0 is
inline.
Initially I proposed a solution inline with that proposed in bugzilla,
changing value_of_register to avoid creating a lazy register value.
However, when this was discussed on the mailing list I got this reply:
https://sourceware.org/pipermail/gdb-patches/2020-June/169633.html
Which led me to look at these two patches:
[1] https://sourceware.org/pipermail/gdb-patches/2020-April/167612.html
[2] https://sourceware.org/pipermail/gdb-patches/2020-April/167930.html
When I considered patches [1] and [2] I saw that all of the issues
being addressed here were related, and that there was a single
solution that could address all of these issues.
First I wrote the new test gdb.opt/inline-frame-tailcall.exp, which
shows that [1] and [2] regress the inline tail-call unwinder, the
reason for this is that these two patches replace a call to
gdbarch_unwind_pc with a call to get_frame_register, however, this is
not correct. The previous call to gdbarch_unwind_pc takes THIS_FRAME
and returns the $pc value in the previous frame. In contrast
get_frame_register takes THIS_FRAME and returns the value of the $pc
in THIS_FRAME; these calls are not equivalent.
The reason these patches appear (or do) fix the regressions listed in
[1] is that the tail call sniffer depends on identifying the address
of a caller and a callee, GDB then looks for a tail-call sequence that
takes us from the caller address to the callee, if such a series is
found then tail-call frames are added.
The bug that was being hit, and which was address in patch [1] is that
in order to find the address of the caller, GDB ended up creating a
lazy register value for an inline frame with to frame-id. The
solution in patch [1] is to instead take the address of the callee and
treat this as the address of the caller. Getting the address of the
callee works, but we then end up looking for a tail-call series from
the callee to the callee, which obviously doesn't return any sane
results, so we don't insert any tail call frames.
The original patch [1] did cause some breakage, so patch [2] undid
patch [1] in all cases except those where we had an inline frame with
no frame-id. It just so happens that there were no tests that fitted
this description _and_ which required tail-call frames to be
successfully spotted, as a result patch [2] appeared to work.
The new test inline-frame-tailcall.exp, exposes the flaw in patch [2].
This commit undoes patch [1] and [2], and replaces them with a new
solution, which is also different to the solution proposed in the
python/22748 bug report.
In this solution I propose that we introduce some special case logic
to value_of_register_lazy. To understand what this logic is we must
first look at how inline frames unwind registers, this is very simple,
they do this:
static struct value *
inline_frame_prev_register (struct frame_info *this_frame,
void **this_cache, int regnum)
{
return get_frame_register_value (this_frame, regnum);
}
And remember:
struct value *
get_frame_register_value (struct frame_info *frame, int regnum)
{
return frame_unwind_register_value (frame->next, regnum);
}
So in all cases, unwinding a register in an inline frame just asks the
next frame to unwind the register, this makes sense, as an inline
frame doesn't really exist, when we unwind a register in an inline
frame, we're really just asking the next frame for the value of the
register in the previous, non-inline frame.
So, if we assume that we only get into the missing frame-id situation
when we try to unwind a register from an inline frame during the frame
sniffing process, then we can change value_of_register_lazy to not
create lazy register values for an inline frame.
Imagine this stack setup, where #1 is inline within #2.
#3 -> #2 -> #1 -> #0
\______/
inline
Now when trying to figure out the frame-id for #1, we need to compute
the frame-id for #2. If the frame sniffer for #2 causes a lazy
register read in #2, either due to a Python Unwinder, or for the
tail-call sniffer, then we call value_of_register_lazy passing in
frame #2.
In value_of_register_lazy, we grab the next frame, which is #1, and we
used to then ask for the frame-id of #1, which was not computed, and
this was our bug.
Now, I propose we spot that #1 is an inline frame, and so lookup the
next frame of #1, which is #0. As #0 is not inline it will have a
valid frame-id, and so we create a lazy register value using #0 as the
next-frame-id. This will give us the exact same result we had
previously (thanks to the code we inspected above).
Encoding into value_of_register_lazy the knowledge that reading an
inline frame register will always just forward to the next frame
feels.... not ideal, but this seems like the cleanest solution to this
recursive frame-id computation/sniffing issue that appears to crop
up.
The following two commits are fully reverted with this commit, these
correspond to patches [1] and [2] respectively:
commit
5939967b355ba6a940887d19847b7893a4506067
Date: Tue Apr 14 17:26:22 2020 -0300
Fix inline frame unwinding breakage
commit
991a3e2e9944a4b3a27bd989ac03c18285bd545d
Date: Sat Apr 25 00:32:44 2020 -0300
Fix remaining inline/tailcall unwinding breakage for x86_64
gdb/ChangeLog:
PR python/22748
* dwarf2/frame-tailcall.c (dwarf2_tailcall_sniffer_first): Remove
special handling for inline frames.
* findvar.c (value_of_register_lazy): Skip inline frames when
creating lazy register values.
* frame.c (frame_id_computed_p): Delete definition.
* frame.h (frame_id_computed_p): Delete declaration.
gdb/testsuite/ChangeLog:
PR python/22748
* gdb.opt/inline-frame-tailcall.c: New file.
* gdb.opt/inline-frame-tailcall.exp: New file.
* gdb.python/py-unwind-inline.c: New file.
* gdb.python/py-unwind-inline.exp: New file.
* gdb.python/py-unwind-inline.py: New file.
+2020-07-06 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ PR python/22748
+ * dwarf2/frame-tailcall.c (dwarf2_tailcall_sniffer_first): Remove
+ special handling for inline frames.
+ * findvar.c (value_of_register_lazy): Skip inline frames when
+ creating lazy register values.
+ * frame.c (frame_id_computed_p): Delete definition.
+ * frame.h (frame_id_computed_p): Delete declaration.
+
2020-07-06 Andrew Burgess <andrew.burgess@embecosm.com>
* NEWS: Mention additions to Python API.
prev_gdbarch = frame_unwind_arch (this_frame);
- /* The dwarf2 tailcall sniffer runs early, at the end of populating the
- dwarf2 frame cache for the current frame. If there exists inline
- frames inner (next) to the current frame, there is a good possibility
- of that inline frame not having a computed frame id yet.
-
- This is because computing such a frame id requires us to walk through
- the frame chain until we find the first normal frame after the inline
- frame and then compute the normal frame's id first.
-
- Some architectures' compilers generate enough register location
- information for a dwarf unwinder to fetch PC without relying on inner
- frames (x86_64 for example). In this case the PC is retrieved
- according to dwarf rules.
-
- But others generate less strict dwarf data for which assumptions are
- made (like interpreting DWARF2_FRAME_REG_UNSPECIFIED as
- DWARF2_FRAME_REG_SAME_VALUE). For such cases, GDB may attempt to
- create lazy values for registers, and those lazy values must be
- created with a valid frame id, but we potentially have no valid id.
-
- So, to avoid breakage, if we see a dangerous situation with inline
- frames without a computed id, use safer functions to retrieve the
- current frame's PC. Otherwise use the provided dwarf rules. */
- frame_info *next_frame = get_next_frame (this_frame);
-
/* Simulate frame_unwind_pc without setting this_frame->prev_pc.p. */
- if (next_frame != nullptr && get_frame_type (next_frame) == INLINE_FRAME
- && !frame_id_computed_p (next_frame))
- {
- /* The next frame is an inline frame and its frame id has not been
- computed yet. */
- get_frame_register (this_frame, gdbarch_pc_regnum (prev_gdbarch),
- (gdb_byte *) &prev_pc);
- prev_pc = gdbarch_addr_bits_remove (prev_gdbarch, prev_pc);
- }
- else
- prev_pc = gdbarch_unwind_pc (prev_gdbarch, this_frame);
+ prev_pc = gdbarch_unwind_pc (prev_gdbarch, this_frame);
/* call_site_find_chain can throw an exception. */
chain = call_site_find_chain (prev_gdbarch, prev_pc, this_pc);
next_frame = get_next_frame_sentinel_okay (frame);
+ /* In some cases NEXT_FRAME may not have a valid frame-id yet. This can
+ happen if we end up trying to unwind a register as part of the frame
+ sniffer. The only time that we get here without a valid frame-id is
+ if NEXT_FRAME is an inline frame. If this is the case then we can
+ avoid getting into trouble here by skipping past the inline frames. */
+ while (get_frame_type (next_frame) == INLINE_FRAME)
+ next_frame = get_next_frame_sentinel_okay (next_frame);
+
/* We should have a valid next frame. */
gdb_assert (frame_id_p (get_frame_id (next_frame)));
return id;
}
-bool
-frame_id_computed_p (struct frame_info *frame)
-{
- gdb_assert (frame != nullptr);
-
- return frame->this_id.p != 0;
-}
-
int
frame_id_p (struct frame_id l)
{
as the special identifier address are set to indicate wild cards. */
extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr);
-/* Returns true if FRAME's id has been computed.
- Returns false otherwise. */
-extern bool frame_id_computed_p (struct frame_info *frame);
-
/* Returns non-zero when L is a valid frame (a valid frame has a
non-zero .base). The outermost frame is valid even without an
ID. */
+2020-07-06 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ PR python/22748
+ * gdb.opt/inline-frame-tailcall.c: New file.
+ * gdb.opt/inline-frame-tailcall.exp: New file.
+ * gdb.python/py-unwind-inline.c: New file.
+ * gdb.python/py-unwind-inline.exp: New file.
+ * gdb.python/py-unwind-inline.py: New file.
+
2020-07-06 Andrew Burgess <andrew.burgess@embecosm.com>
* gdb.python/py-arch-reg-groups.exp: New file.
--- /dev/null
+/* 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;
+}
--- /dev/null
+# 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 \\(\\)"
+}
--- /dev/null
+# 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")
projects / binutils-gdb.git / commitdiff