[Bug gdb/27938] New: If "set debug frame" is stet GDB fails

[Bug gdb/27938] New: If "set debug frame" is stet GDB fails

[stanislav.simek at siemens dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=27938

            Bug ID: 27938
           Summary: If "set debug frame" is stet GDB fails
           Product: gdb
           Version: 10.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: stanislav.simek at siemens dot com
  Target Milestone: ---

Hi,
If I enable "set debug frame" and do a step, then GDB crash.
Without enabling it is working withou problem.
Am I doing something wrong?

==================================================================
See gdb console:

set debug frame 1
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=NORMAL_FRAME,unwind=0xc66ee0,pc=0x4103d9b0,id={stack=0x41920000,code=0x4103d9b0,!special},func=0x4103d9b0}
// cached 
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=NORMAL_FRAME,unwind=0xc66ee0,pc=0x4103d9b0,id={stack=0x41920000,code=0x4103d9b0,!special},func=0x4103d9b0}
// cached 
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=NORMAL_FRAME,unwind=0xc66ee0,pc=0x4103d9b0,id={stack=0x41920000,code=0x4103d9b0,!special},func=0x4103d9b0}
// cached 
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ reinit_frame_cache () }
{ create_sentinel_frame (...) ->
{level=-1,type=SENTINEL_FRAME,unwind=0xcd47e0,pc=<unknown>,id={stack=<sentinel>,!code,special=0x0},func=<unknown>}
}
{ frame_unwind_arch (next_frame=-1) -> aarch64 }
{ get_prev_frame_always (this_frame=-1) { frame_id_eq
(l={stack=<sentinel>,!code,special=0x0},r={stack=<outer>,!code,special=0x0}) ->
0 }
-> {level=0,type=<unknown>,unwind=<unknown>,pc=<unknown>,id=<not
computed>,func=<unknown>} }
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=<unknown>,unwind=<unknown>,pc=<unknown>,id=<not
computed>,func=<unknown>} // cached 
{ frame_unwind_register_value (frame=-1,regnum=32(pc),...) -> register=32
bytes=[b4d9034100000000] }
{ frame_unwind_pc (this_frame=-1) -> 0x4103d9b4 }
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=<unknown>,unwind=<unknown>,pc=0x4103d9b4,id=<not
computed>,func=<unknown>} // cached 
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=<unknown>,unwind=<unknown>,pc=0x4103d9b4,id=<not
computed>,func=<unknown>} // cached 
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=<unknown>,unwind=<unknown>,pc=0x4103d9b4,id=<not
computed>,func=<unknown>} // cached 
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=<unknown>,unwind=<unknown>,pc=0x4103d9b4,id=<not
computed>,func=<unknown>} // cached 
{ compute_frame_id (fi=0) { get_frame_func (this_frame=0) -> 0x4103d9b0 }
{ frame_id_p (l={!stack,!code,!special}) -> 0 }
{ frame_unwind_register_value (frame=-1,regnum=31(sp),...) -> register=31
bytes=[10ff914100000000] }
{ frame_unwind_arch (next_frame=0) -> aarch64 }
{ frame_unwind_register_value (frame=0,regnum=32(pc),...) {
frame_unwind_register_value (frame=0,regnum=30(x30),...) { frame_id_p
(l={stack=<sentinel>,!code,special=0x0}) -> 1 }
->{ frame_id_p (l={stack=<sentinel>,!code,special=0x0}) -> 1 }
{ frame_id_eq
(l={stack=<sentinel>,!code,special=0x0},r={stack=<sentinel>,!code,special=0x0})
-> 1 }
{ frame_unwind_register_value (frame=-1,regnum=30(x30),...) -> register=30
bytes=[f881004100000000] }
{ frame_id_p (l={stack=<sentinel>,!code,special=0x0}) -> 1 }
{ frame_id_eq
(l={stack=<sentinel>,!code,special=0x0},r={stack=<sentinel>,!code,special=0x0})
-> 1 }
{ get_prev_frame_always (this_frame=-1) ->
{level=0,type=NORMAL_FRAME,unwind=0xc66ee0,pc=0x4103d9b4,id=<computing>,func=0x4103d9b0}
// cached 
../../../gdb-10.1/gdb/frame.c:626: internal-error: frame_id
get_frame_id(frame_info*): Assertion `fi->this_id.p !=
frame_id_status::COMPUTING' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) [answered Y; input not from terminal]
../../../gdb-10.1/gdb/frame.c:626: internal-error: frame_id
get_frame_id(frame_info*): Assertion `fi->this_id.p !=
frame_id_status::COMPUTING' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Create a core file of GDB? (y or n) [answered Y; input not from terminal]

This is a bug, please report it.  For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27938] If "set debug frame" is set GDB fails

[stanislav.simek at siemens dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=27938

Stanislav Šimek <stanislav.simek at siemens dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|If "set debug frame" is     |If "set debug frame" is set
                   |stet GDB fails              |GDB fails

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27938] If "set debug frame" is set GDB fails

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27938

--- Comment #1 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ca89bdf8b2b013484f252d9688cb01a3def2fa2e

commit ca89bdf8b2b013484f252d9688cb01a3def2fa2e
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed May 26 15:50:05 2021 +0100

    gdb: remove VALUE_FRAME_ID and fix another frame debug issue

    This commit was originally part of this patch series:

      (v1): https://sourceware.org/pipermail/gdb-patches/2021-May/179357.html
      (v2): https://sourceware.org/pipermail/gdb-patches/2021-June/180208.html
      (v3): https://sourceware.org/pipermail/gdb-patches/2021-July/181028.html

    However, that series is being held up in review, so I wanted to break
    out some of the non-related fixes in order to get these merged.

    This commit addresses two semi-related issues, both of which are
    problems exposed by using 'set debug frame on'.

    The first issue is in frame.c in get_prev_frame_always_1, and was
    introduced by this commit:

      commit a05a883fbaba69d0f80806e46a9457727fcbe74c
      Date:   Tue Jun 29 12:03:50 2021 -0400

          gdb: introduce frame_debug_printf

    This commit replaced fprint_frame with frame_info::to_string.
    However, the former could handle taking a nullptr while the later, a
    member function, obviously requires a non-nullptr in order to make the
    function call.  In one place we are not-guaranteed to have a
    non-nullptr, and so, there is the possibility of triggering undefined
    behaviour.

    The second issue addressed in this commit has existed for a while in
    GDB, and would cause this assertion:

      gdb/frame.c:622: internal-error: frame_id get_frame_id(frame_info*):
Assertion `fi->this_id.p != frame_id_status::COMPUTING' failed.

    We attempt to get the frame_id for a frame while we are computing the
    frame_id for that same frame.

    What happens is that when GDB stops we create a frame_info object for
    the sentinel frame (frame #-1) and then we attempt to unwind this
    frame to create a frame_info object for frame #0.

    In the test case used here to expose the issue we have created a
    Python frame unwinder.  In the Python unwinder we attemt to read the
    program counter register.

    Reading this register will initially create a lazy register value.
    The frame-id stored in the lazy register value will be for the
    sentinel frame (lazy register values hold the frame-id for the frame
    from which the register will be unwound).

    However, the Python unwinder does actually want to examine the value
    of the program counter, and so the lazy register value is resolved
    into a non-lazy value.  This sends GDB into value_fetch_lazy_register
    in value.c.

    Now, inside this function, if 'set debug frame on' is in effect, then
    we want to print something like:

      frame=%d, regnum=%d(%s), ....

    Where 'frame=%d' will be the relative frame level of the frame for
    which the register is being fetched, so, in this case we would expect
    to see 'frame=0', i.e. we are reading a register as it would be in
    frame #0.  But, remember, the lazy register value actually holds the
    frame-id for frame #-1 (the sentinel frame).

    So, to get the frame_info for frame #0 we used to call:

      frame = frame_find_by_id (VALUE_FRAME_ID (val));

    Where VALUE_FRAME_ID is:

      #define VALUE_FRAME_ID(val) (get_prev_frame_id_by_id (VALUE_NEXT_FRAME_ID
(val)))

    That is, we start with the frame-id for the next frame as obtained by
    VALUE_NEXT_FRAME_ID, then call get_prev_frame_id_by_id to get the
    frame-id of the previous frame.

    The get_prev_frame_id_by_id function finds the frame_info for the
    given frame-id (in this case frame #-1), calls get_prev_frame to get
    the previous frame, and then calls get_frame_id.

    The problem here is that calling get_frame_id requires that we know
    the frame unwinder, so then have to try each frame unwinder in turn,
    which would include the Python unwinder.... which is where we started,
    and thus we have a loop!

    To prevent this loop GDB has an assertion in place, which is what
    actually triggers.

    Solving the assertion failure is pretty easy, if we consider the code
    in value_fetch_lazy_register and get_prev_frame_id_by_id then what we
    do is:

      1. Start with a frame_id taken from a value,
      2. Lookup the corresponding frame,
      3. Find the previous frame,
      4. Get the frame_id for that frame, and
      5. Lookup the corresponding frame
      6. Print the frame's level

    Notice that steps 3 and 5 give us the exact same result, step 4 is
    just wasted effort.  We could shorten this process such that we drop
    steps 4 and 5, thus:

      1. Start with a frame_id taken from a value,
      2. Lookup the corresponding frame,
      3. Find the previous frame,
      6. Print the frame's level

    This will give the exact same frame as a result, and this is what I
    have done in this patch by removing the use of VALUE_FRAME_ID from
    value_fetch_lazy_register.

    Out of curiosity I looked to see how widely VALUE_FRAME_ID was used,
    and saw it was only used in one other place in valops.c:value_assign,
    where, once again, we take the result of VALUE_FRAME_ID and pass it to
    frame_find_by_id, thus introducing a redundant frame_id lookup.

    I don't think the value_assign case risks triggering the assertion
    though, as we are unlikely to call value_assign while computing the
    frame_id for a frame, however, we could make value_assign slightly
    more efficient, with no real additional complexity, by removing the
    use of VALUE_FRAME_ID.

    So, in this commit, I completely remove VALUE_FRAME_ID, and replace it
    with a use of VALUE_NEXT_FRAME_ID, followed by a direct call to
    get_prev_frame_always, this should make no difference in either case,
    and resolves the assertion issue from value.c.

    As I said, this patch was originally part of another series, the
    original test relied on the fixes in that original series.  However, I
    was able to create an alternative test for this issue by enabling
    frame debug within an existing test script.

    This commit probably fixes bug PR gdb/27938, though the bug doesn't
    have a reproducer attached so it is not possible to know for sure.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27938

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27938] If "set debug frame" is set GDB fails

[andrew.burgess at embecosm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=27938

Andrew Burgess <andrew.burgess at embecosm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrew.burgess at embecosm dot com

--- Comment #2 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
I suspect that this issue might be fixed in current HEAD on the master branch. 
Would it be possible to build and test that version?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/27938] If "set debug frame" is set GDB fails

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27938

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at sourceware dot org
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
Nearly a year without a response, I'm going to close this
since it seems to have been fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.