* [PATCH] Fix assertion fi->level @ 2020-01-18 23:04 Mark Williams 2020-01-19 17:27 ` Mark Williams 0 siblings, 1 reply; 6+ messages in thread From: Mark Williams @ 2020-01-18 23:04 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 203 bytes --] This is a fix for https://sourceware.org/bugzilla/show_bug.cgi?id=22748. It avoids calling value_of_register_lazy, since that requires a frame id, and there may not be one yet for inlined frames. Mark [-- Attachment #2: 0001-gdb-Fix-Assertion-fi-level-0-failed.patch --] [-- Type: application/octet-stream, Size: 1610 bytes --] From 28221e7158acb4b3129e57fcb8bbf262b8d4ab3d Mon Sep 17 00:00:00 2001 From: mwilliams <mwilliams@fb.com> Date: Tue, 7 Jan 2020 07:02:31 -0800 Subject: [PATCH] gdb: Fix "Assertion 'fi->level == 0' failed." At the point that python unwinders are called, to determine if they want to handle a frame, calling value_of_register_lazy can fail because the next frame's id has not yet been computed (if its inline). Rather than calling value_of_register_lazy, then immediately calling value_fetch_lazy, just call get_frame_register_value directly. gdb/Changelog 2020-01-18 Mark Williams <mark@myosotissp.com> PR gdb/22748 * findvar.c call get_frame_register_value instead of value_of_register_lazy/value_fetch_lazy so it works without a frame id. --- gdb/findvar.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/gdb/findvar.c b/gdb/findvar.c index 5cf1cd4137..0cddebc12b 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 -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assertion fi->level 2020-01-18 23:04 [PATCH] Fix assertion fi->level Mark Williams @ 2020-01-19 17:27 ` Mark Williams 2020-02-03 3:05 ` Mark Williams 0 siblings, 1 reply; 6+ messages in thread From: Mark Williams @ 2020-01-19 17:27 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 55 bytes --] Add a test case, and more detail to the commit message [-- Attachment #2: 0001-gdb-Fix-Assertion-fi-level-0-failed.patch --] [-- Type: application/octet-stream, Size: 4548 bytes --] From ee4c0dafb5495c1492709cc606a0542771315fe2 Mon Sep 17 00:00:00 2001 From: mwilliams <mwilliams@fb.com> Date: Sun, 19 Jan 2020 09:14:12 -0800 Subject: [PATCH] gdb: Fix "Assertion 'fi->level == 0' failed." At the point that python unwinders are called to determine if they want to handle a frame, calling value_of_register_lazy can fail in the caller of an inlined function, because the inlined function's frame id has not yet been computed. However, there's no need to call value_of_register_lazy because the very next statement is value_fetch_lazy, and the two calls together are identical to a call to get_frame_register_value, except that the latter doesn't need a frame id. An existing test case already stopped in an inlined function and inspected various frames, so just add a dummy unwinder to it. The presence of the dummy unwinder should not affect the existing test, because all it does is tell gdb that it does not want to handle the current frame. But without this fix, gdb crashes, breaking the test. gdb/Changelog 2020-01-19 Mark Williams <mark@myosotissp.com> PR gdb/22748 * findvar.c call get_frame_register_value instead of value_of_register_lazy/value_fetch_lazy so it works without a frame id. gdb/testsuite/Changelog 2020-01-19 Mark Williams <mark@myosotissp.com> PR gdb/22748 * gdb.python/py-frame-inline.exp modified to load the new dummy unwinder to expose the bug * gdb.python/py-frame-inline.py new file, implementing a dummy unwinder. --- gdb/findvar.c | 5 +--- gdb/testsuite/gdb.python/py-frame-inline.exp | 4 ++++ gdb/testsuite/gdb.python/py-frame-inline.py | 24 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.python/py-frame-inline.py diff --git a/gdb/findvar.c b/gdb/findvar.c index 5cf1cd4137..0cddebc12b 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 diff --git a/gdb/testsuite/gdb.python/py-frame-inline.exp b/gdb/testsuite/gdb.python/py-frame-inline.exp index 71bffd375d..488346e651 100644 --- a/gdb/testsuite/gdb.python/py-frame-inline.exp +++ b/gdb/testsuite/gdb.python/py-frame-inline.exp @@ -24,6 +24,10 @@ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { # Skip all tests if Python scripting is not enabled. if { [skip_python_tests] } { continue } +set remote_python_file [gdb_remote_download host \ + ${srcdir}/${subdir}/${testfile}.py] +gdb_test_no_output "source ${remote_python_file}" "load python file" + if ![runto main] then { fail "can't run to function f" return 0 diff --git a/gdb/testsuite/gdb.python/py-frame-inline.py b/gdb/testsuite/gdb.python/py-frame-inline.py new file mode 100644 index 0000000000..56be6d3523 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-frame-inline.py @@ -0,0 +1,24 @@ +# 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 file is part of the GDB testsuite. +from gdb.unwinder import Unwinder, register_unwinder +class DummyUnwinder(Unwinder): + def __init__(self): + super(DummyUnwinder, self).__init__('dummy_unwinder') + def __call__(self, pending_frame): + fp = pending_frame.read_register('pc') + return None +register_unwinder(None, DummyUnwinder()) -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assertion fi->level 2020-01-19 17:27 ` Mark Williams @ 2020-02-03 3:05 ` Mark Williams 2020-02-07 21:04 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Mark Williams @ 2020-02-03 3:05 UTC (permalink / raw) To: gdb-patches Apparently I should ping the thread after 2 weeks with no response... Note that python unwinders are completely unusable in gdb-8.1 and later without this fix... On Sun, Jan 19, 2020 at 9:20 AM Mark Williams <mark@myosotissp.com> wrote: > > Add a test case, and more detail to the commit message ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assertion fi->level 2020-02-03 3:05 ` Mark Williams @ 2020-02-07 21:04 ` Tom Tromey 2020-02-07 21:09 ` Mark Williams 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2020-02-07 21:04 UTC (permalink / raw) To: Mark Williams; +Cc: gdb-patches >>>>> "Mark" == Mark Williams <mark@myosotissp.com> writes: Mark> Apparently I should ping the thread after 2 weeks with no response... Yeah. Mark> Note that python unwinders are completely unusable in gdb-8.1 and Mark> later without this fix... Thanks. I read the patch, and I think it looks reasonable. However, since we were considering it for gdb 9 in another thread, I applied it and ran the test suite. It caused a few regressions, like: gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: cannot a ssign to register: PASS => FAIL gdb.mi/mi-reg-undefined.exp: register values, format r, frame 1: PASS => FAIL gdb.mi/mi-reg-undefined.exp: register values, format r, frame 2: PASS => FAIL gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1: PASS => FAIL gdb.mi/mi-reg-undefined.exp: register values, format x, frame 2: PASS => FAIL Here's some stuff from gdb.log: me: print pseudo register print $rbp = -1 Left operand of assignment is not an lvalue. (gdb) FAIL: gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: cannot assign to register 221-data-list-register-values --thread 1 --frame 1 x 0 1 2 8 9 ~"../../binutils-gdb/gdb/value.c:389: internal-error: int value_bits_any_optimiz ed_out(const value*, int, int): Assertion `!value->lazy' failed.\nA problem inte rnal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit th is debugging session? (y or n) " FAIL: gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1 (timeout) I didn't investigate any further but there's still some issue to be worked out. thanks, Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assertion fi->level 2020-02-07 21:04 ` Tom Tromey @ 2020-02-07 21:09 ` Mark Williams 2020-02-09 18:57 ` Mark Williams 0 siblings, 1 reply; 6+ messages in thread From: Mark Williams @ 2020-02-07 21:09 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Thanks, I'll take a look. On Fri, Feb 7, 2020 at 1:05 PM Tom Tromey <tom@tromey.com> wrote: > > >>>>> "Mark" == Mark Williams <mark@myosotissp.com> writes: > > Mark> Apparently I should ping the thread after 2 weeks with no response... > > Yeah. > > Mark> Note that python unwinders are completely unusable in gdb-8.1 and > Mark> later without this fix... > > Thanks. > > I read the patch, and I think it looks reasonable. However, since we > were considering it for gdb 9 in another thread, I applied it and ran > the test suite. It caused a few regressions, like: > > gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: cannot a > ssign to register: PASS => FAIL > gdb.mi/mi-reg-undefined.exp: register values, format r, frame 1: PASS => FAIL > gdb.mi/mi-reg-undefined.exp: register values, format r, frame 2: PASS => FAIL > gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1: PASS => FAIL > gdb.mi/mi-reg-undefined.exp: register values, format x, frame 2: PASS => FAIL > > > Here's some stuff from gdb.log: > > me: print pseudo register > print $rbp = -1 > Left operand of assignment is not an lvalue. > (gdb) FAIL: gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: cannot assign to register > > > 221-data-list-register-values --thread 1 --frame 1 x 0 1 2 8 9 > ~"../../binutils-gdb/gdb/value.c:389: internal-error: int value_bits_any_optimiz > ed_out(const value*, int, int): Assertion `!value->lazy' failed.\nA problem inte > rnal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit th > is debugging session? (y or n) " > FAIL: gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1 (timeout) > > > I didn't investigate any further but there's still some issue to be > worked out. > > thanks, > Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assertion fi->level 2020-02-07 21:09 ` Mark Williams @ 2020-02-09 18:57 ` Mark Williams 0 siblings, 0 replies; 6+ messages in thread From: Mark Williams @ 2020-02-09 18:57 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 3274 bytes --] The problem seems to be that jit_frame_prev_register returns a value with lval == not_lval, so the value returned from get_frame_register_value ends up with lval == not_lval, rather than lval == lval_register. This is hidden by value_of_register_lazy/value_fetch_lazy, because value_of_register_lazy creates a value like this: reg_val = allocate_value_lazy (register_type (gdbarch, regnum)); VALUE_LVAL (reg_val) = lval_register; VALUE_REGNUM (reg_val) = regnum; and then value_fetch_lazy gets a new_val with lval==not_lval, but then copies its contents into the reg_val created by value_of_register_lazy without overwriting the lval field. This seems wrong... if an unwinder says its returning a value thats not an lval, it doesn't seem correct for value_fetch_lazy to turn it into one. On the other hand, it does seem to work as an lval, so the bug is probably in jit_frame_prev_register? I'm just not familiar enough with the code to know what the correct fix is... In any case, the simple fix for my patch is to call value_of_register_lazy/value_fetch_lazy when there *is* a valid frame_id, and get_frame_register_value when there isn't. That way it only changes the behavior when it was going to crash anyway. I'll go with that unless someone has a better suggestion... New patch attached. On Fri, Feb 7, 2020 at 1:08 PM Mark Williams <mark@myosotissp.com> wrote: > > Thanks, I'll take a look. > > On Fri, Feb 7, 2020 at 1:05 PM Tom Tromey <tom@tromey.com> wrote: > > > > >>>>> "Mark" == Mark Williams <mark@myosotissp.com> writes: > > > > Mark> Apparently I should ping the thread after 2 weeks with no response... > > > > Yeah. > > > > Mark> Note that python unwinders are completely unusable in gdb-8.1 and > > Mark> later without this fix... > > > > Thanks. > > > > I read the patch, and I think it looks reasonable. However, since we > > were considering it for gdb 9 in another thread, I applied it and ran > > the test suite. It caused a few regressions, like: > > > > gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: cannot a > > ssign to register: PASS => FAIL > > gdb.mi/mi-reg-undefined.exp: register values, format r, frame 1: PASS => FAIL > > gdb.mi/mi-reg-undefined.exp: register values, format r, frame 2: PASS => FAIL > > gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1: PASS => FAIL > > gdb.mi/mi-reg-undefined.exp: register values, format x, frame 2: PASS => FAIL > > > > > > Here's some stuff from gdb.log: > > > > me: print pseudo register > > print $rbp = -1 > > Left operand of assignment is not an lvalue. > > (gdb) FAIL: gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: cannot assign to register > > > > > > 221-data-list-register-values --thread 1 --frame 1 x 0 1 2 8 9 > > ~"../../binutils-gdb/gdb/value.c:389: internal-error: int value_bits_any_optimiz > > ed_out(const value*, int, int): Assertion `!value->lazy' failed.\nA problem inte > > rnal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit th > > is debugging session? (y or n) " > > FAIL: gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1 (timeout) > > > > > > I didn't investigate any further but there's still some issue to be > > worked out. > > > > thanks, > > Tom [-- Attachment #2: 0001-gdb-Fix-Assertion-fi-level-0-failed.patch --] [-- Type: application/octet-stream, Size: 5777 bytes --] From 2604c15697ecb25e39fa7b049193bfdc263c40cc Mon Sep 17 00:00:00 2001 From: mwilliams <mwilliams@fb.com> Date: Sun, 9 Feb 2020 10:12:24 -0800 Subject: [PATCH] gdb: Fix "Assertion 'fi->level == 0' failed." At the point that python unwinders are called to determine if they want to handle a frame, calling value_of_register_lazy can fail in the caller of an inlined function, because the inlined function's frame id has not yet been computed. However, there's no need to call value_of_register_lazy because the very next statement is value_fetch_lazy, and the two calls together are identical to a call to get_frame_register_value, except that the latter doesn't need a frame id. An existing test case already stopped in an inlined function and inspected various frames, so just add a dummy unwinder to it. The presence of the dummy unwinder should not affect the existing test, because all it does is tell gdb that it does not want to handle the current frame. But without this fix, gdb crashes, breaking the test. gdb/Changelog 2020-02-09 Mark Williams <mark@myosotissp.com> PR gdb/22748 * findvar.c call get_frame_register_value instead of value_of_register_lazy/value_fetch_lazy when there is no frame id. * frame.c add has_frame_id implementation * frame.h add has_frame_id declaration gdb/testsuite/Changelog 2020-02-09 Mark Williams <mark@myosotissp.com> PR gdb/22748 * gdb.python/py-frame-inline.exp modified to load the new dummy unwinder to expose the bug * gdb.python/py-frame-inline.py new file, implementing a dummy unwinder. --- gdb/findvar.c | 9 ++++++-- gdb/frame.c | 6 +++++ gdb/frame.h | 1 + gdb/testsuite/gdb.python/py-frame-inline.exp | 4 ++++ gdb/testsuite/gdb.python/py-frame-inline.py | 24 ++++++++++++++++++++ 5 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.python/py-frame-inline.py diff --git a/gdb/findvar.c b/gdb/findvar.c index a836c63dc5..12d4e0361c 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -264,14 +264,19 @@ value_of_register (int regnum, struct frame_info *frame) { struct gdbarch *gdbarch = get_frame_arch (frame); struct value *reg_val; + struct frame_info *next_frame; /* 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); + next_frame = get_next_frame_sentinel_okay (frame); + if (has_frame_id(next_frame)) { + reg_val = value_of_register_lazy(frame, regnum); + value_fetch_lazy(reg_val); + } else + reg_val = get_frame_register_value (frame, regnum); return reg_val; } diff --git a/gdb/frame.c b/gdb/frame.c index d74d1d5c7c..34789e56cc 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -558,6 +558,12 @@ compute_frame_id (struct frame_info *fi) } } +bool +has_frame_id (struct frame_info *fi) +{ + return fi->this_id.p; +} + /* Return a frame uniq ID that can be used to, later, re-find the frame. */ diff --git a/gdb/frame.h b/gdb/frame.h index cfc15022ed..0a4d748df7 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -478,6 +478,7 @@ extern CORE_ADDR get_frame_base (struct frame_info *); if (frame_id_eq (id, get_frame_id (r))) instead, since that avoids the bug. */ +extern bool has_frame_id (struct frame_info *fi); extern struct frame_id get_frame_id (struct frame_info *fi); extern struct frame_id get_stack_frame_id (struct frame_info *fi); extern struct frame_id frame_unwind_caller_id (struct frame_info *next_frame); diff --git a/gdb/testsuite/gdb.python/py-frame-inline.exp b/gdb/testsuite/gdb.python/py-frame-inline.exp index 71bffd375d..488346e651 100644 --- a/gdb/testsuite/gdb.python/py-frame-inline.exp +++ b/gdb/testsuite/gdb.python/py-frame-inline.exp @@ -24,6 +24,10 @@ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { # Skip all tests if Python scripting is not enabled. if { [skip_python_tests] } { continue } +set remote_python_file [gdb_remote_download host \ + ${srcdir}/${subdir}/${testfile}.py] +gdb_test_no_output "source ${remote_python_file}" "load python file" + if ![runto main] then { fail "can't run to function f" return 0 diff --git a/gdb/testsuite/gdb.python/py-frame-inline.py b/gdb/testsuite/gdb.python/py-frame-inline.py new file mode 100644 index 0000000000..56be6d3523 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-frame-inline.py @@ -0,0 +1,24 @@ +# 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 file is part of the GDB testsuite. +from gdb.unwinder import Unwinder, register_unwinder +class DummyUnwinder(Unwinder): + def __init__(self): + super(DummyUnwinder, self).__init__('dummy_unwinder') + def __call__(self, pending_frame): + fp = pending_frame.read_register('pc') + return None +register_unwinder(None, DummyUnwinder()) -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-09 18:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-18 23:04 [PATCH] Fix assertion fi->level Mark Williams 2020-01-19 17:27 ` Mark Williams 2020-02-03 3:05 ` Mark Williams 2020-02-07 21:04 ` Tom Tromey 2020-02-07 21:09 ` Mark Williams 2020-02-09 18:57 ` Mark Williams
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox