[v2,11/13] gdb: make user-created frames reinflatable

Message ID 20221214033441.499512-12-simon.marchi@polymtl.ca
State Committed
Headers
Series Make frame_info_ptr automatic |

Commit Message

Simon Marchi Dec. 14, 2022, 3:34 a.m. UTC
  This patch teaches frame_info_ptr to reinflate user-created frames
(frames created through create_new_frame, with the "select-frame view"
command).

Before this patch, frame_info_ptr doesn't support reinflating
user-created frames, because it currently reinflates by getting the
current target frame (for frame 0) or frame_find_by_id (for other
frames).  To reinflate a user-created frame, we need to call
create_new_frame, to make it lookup an existing user-created frame, or
otherwise create one.

So, in prepare_reinflate, get the frame id even if the frame has level
0, if it is user-created.  In reinflate, if the saved frame id is user
create it, call create_new_frame.

In order to test this, I initially enhanced the gdb.base/frame-view.exp
test added by the previous patch by setting a pretty-printer for the
type of the function parameters, in which we do an inferior call.  This
causes print_frame_args to not reinflate its frame (which is a
user-created one) properly.  On one machine (my Arch Linux one), it
properly catches the bug, as the frame is not correctly restored after
printing the first parameter, so it messes up the second parameter:

    frame
    #0  baz (z1=hahaha, z2=<error reading variable: frame address is not available.>) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:40
    40        return z1.m + z2.n;
    (gdb) FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame
    frame
    #0  baz (z1=hahaha, z2=<error reading variable: frame address is not available.>) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:40
    40        return z1.m + z2.n;
    (gdb) FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again

However, on another machine (my Ubuntu 22.04 one), it just passes fine,
without the appropriate fix.  I then thought about writing a selftest
for that, it's more reliable.  I left the gdb.base/frame-view.exp pretty
printer test there, it's already written, and we never know, it might
catch some unrelated issue some day.

Change-Id: I5849baf77991fc67a15bfce4b5e865a97265b386
---
 gdb/Makefile.in                          |  1 +
 gdb/frame.c                              | 22 +++++--
 gdb/frame.h                              |  7 ++-
 gdb/testsuite/gdb.base/frame-view.c      |  6 ++
 gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
 gdb/testsuite/gdb.base/frame-view.py     | 41 ++++++++++++
 gdb/unittests/frame_info_ptr-selftests.c | 80 ++++++++++++++++++++++++
 7 files changed, 193 insertions(+), 11 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/frame-view.py
 create mode 100644 gdb/unittests/frame_info_ptr-selftests.c
  

Comments

Tom de Vries Jan. 23, 2023, 12:57 p.m. UTC | #1
On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>   gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--

Hi,

on aarch64-linux I get:
...
FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB 
internal error)
FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again 
(GDB internal error)
...

In more detail:
...
(gdb) PASS: gdb.base/frame-view.exp: with_pretty_printer=true: set 
scheduler-locking on
frame^M
#0  baz (z1=hahaha, /home/tdevries/gdb/src/gdb/value.c:4056: 
internal-error: value_fetch_lazy_register: Assertion `next_frame != 
NULL' failed.^M
...


Back-trace:
...
----- Backtrace -----
Resyncing due to internal error.
0x563033 gdb_internal_backtrace_1
         /home/tdevries/gdb/src/gdb/bt-utils.c:122
0x5630f3 _Z22gdb_internal_backtracev
         /home/tdevries/gdb/src/gdb/bt-utils.c:168
0xb4461f internal_vproblem
         /home/tdevries/gdb/src/gdb/utils.c:396
0xb44a33 _Z15internal_verrorPKciS0_St9__va_list
         /home/tdevries/gdb/src/gdb/utils.c:476
0xdb8e97 _Z18internal_error_locPKciS0_z
         /home/tdevries/gdb/src/gdbsupport/errors.cc:58
0xb7a1d7 value_fetch_lazy_register
         /home/tdevries/gdb/src/gdb/value.c:4056
0xb7a88b _Z16value_fetch_lazyP5value
         /home/tdevries/gdb/src/gdb/value.c:4180
0xb74c93 _Z19value_optimized_outP5value
         /home/tdevries/gdb/src/gdb/value.c:1500
0x770b2f _Z30frame_unwind_register_unsigned14frame_info_ptri
         /home/tdevries/gdb/src/gdb/frame.c:1337
0x42bac3 aarch64_dwarf2_prev_register
         /home/tdevries/gdb/src/gdb/aarch64-tdep.c:1280
0x680ec3 dwarf2_frame_prev_register
         /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1217
0x770587 _Z27frame_unwind_register_value14frame_info_ptri
         /home/tdevries/gdb/src/gdb/frame.c:1244
0x770ad3 _Z30frame_unwind_register_unsigned14frame_info_ptri
         /home/tdevries/gdb/src/gdb/frame.c:1333
0x76c30f _Z17default_unwind_pcP7gdbarch14frame_info_ptr
         /home/tdevries/gdb/src/gdb/frame-unwind.c:244
0x4b652b _Z17gdbarch_unwind_pcP7gdbarch14frame_info_ptr
         /home/tdevries/gdb/src/gdb/gdbarch.c:2959
0x67d363 _Z29dwarf2_tailcall_sniffer_first14frame_info_ptrPPvPKl
         /home/tdevries/gdb/src/gdb/dwarf2/frame-tailcall.c:390
0x680593 dwarf2_frame_cache
         /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1089
0x68070f dwarf2_frame_unwind_stop_reason
         /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1101
0x773503 get_prev_frame_always_1
         /home/tdevries/gdb/src/gdb/frame.c:2281
0x773beb _Z21get_prev_frame_always14frame_info_ptr
         /home/tdevries/gdb/src/gdb/frame.c:2376
0x775d6b _Z28get_frame_unwind_stop_reason14frame_info_ptr
         /home/tdevries/gdb/src/gdb/frame.c:3051
0x68151f _Z16dwarf2_frame_cfa14frame_info_ptr
         /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1356
0x678b13 _ZN18dwarf_expr_context16execute_stack_opEPKhS1_
         /home/tdevries/gdb/src/gdb/dwarf2/expr.c:2110
0x676f4f _ZN18dwarf_expr_context4evalEPKhm
         /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1239
0x677faf _ZN18dwarf_expr_context16execute_stack_opEPKhS1_
         /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1811
0x676f4f _ZN18dwarf_expr_context4evalEPKhm
         /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1239
0x6769e3 
_ZN18dwarf_expr_context8evaluateEPKhmbP18dwarf2_per_cu_data14frame_info_ptrPK18property_addr_infoP4typeS9_l
         /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1078
0x6a924b dwarf2_evaluate_loc_desc_full
         /home/tdevries/gdb/src/gdb/dwarf2/loc.c:1513
0x6a941f 
_Z24dwarf2_evaluate_loc_descP4type14frame_info_ptrPKhmP18dwarf2_per_cu_dataP18dwarf2_per_objfileb
         /home/tdevries/gdb/src/gdb/dwarf2/loc.c:1558
0x6abb83 locexpr_read_variable
         /home/tdevries/gdb/src/gdb/dwarf2/loc.c:3052
0x769083 
_ZNK13language_defn14read_var_valueEP6symbolPK5block14frame_info_ptr
         /home/tdevries/gdb/src/gdb/findvar.c:578
0x769a17 _Z14read_var_valueP6symbolPK5block14frame_info_ptr
         /home/tdevries/gdb/src/gdb/findvar.c:794
0xa188ef 
_Z14read_frame_argRK19frame_print_optionsP6symbol14frame_info_ptrP9frame_argS6_
         /home/tdevries/gdb/src/gdb/stack.c:540
0xa19557 print_frame_args
         /home/tdevries/gdb/src/gdb/stack.c:888
0xa1ae4b print_frame
         /home/tdevries/gdb/src/gdb/stack.c:1390
0xa1a0e3 
_Z16print_frame_infoRK19frame_print_options14frame_info_ptri10print_whatii
         /home/tdevries/gdb/src/gdb/stack.c:1116
0xa180c7 _Z17print_stack_frame14frame_info_ptri10print_whati
         /home/tdevries/gdb/src/gdb/stack.c:367
0xa1801b 
_Z26print_stack_frame_to_uioutP6ui_out14frame_info_ptri10print_whati
         /home/tdevries/gdb/src/gdb/stack.c:346
0xa9d44f 
_Z27print_selected_thread_frameP6ui_out10enum_flagsI23user_selected_what_flagE
         /home/tdevries/gdb/src/gdb/thread.c:1994
0xa1c907 frame_command_core
         /home/tdevries/gdb/src/gdb/stack.c:1857
0xa20ec7 base_command
         /home/tdevries/gdb/src/gdb/stack.c:1962
0x5b8f9f do_simple_func
         /home/tdevries/gdb/src/gdb/cli/cli-decode.c:95
0x5be36b _Z8cmd_funcP16cmd_list_elementPKci
         /home/tdevries/gdb/src/gdb/cli/cli-decode.c:2737
0xaa41ef _Z15execute_commandPKci
         /home/tdevries/gdb/src/gdb/top.c:688
0x7421cf _Z15command_handlerPKc
         /home/tdevries/gdb/src/gdb/event-top.c:616
0x74272f _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE
         /home/tdevries/gdb/src/gdb/event-top.c:852
0xacc9c7 tui_command_line_handler
         /home/tdevries/gdb/src/gdb/tui/tui-interp.c:104
0x74184f gdb_rl_callback_handler
         /home/tdevries/gdb/src/gdb/event-top.c:246
0xffff957ba97f ???
0x74169b gdb_rl_callback_read_char_wrapper_noexcept
         /home/tdevries/gdb/src/gdb/event-top.c:188
0x741733 gdb_rl_callback_read_char_wrapper
         /home/tdevries/gdb/src/gdb/event-top.c:221
0x741fa3 stdin_event_handler
         /home/tdevries/gdb/src/gdb/event-top.c:541
0xdba083 handle_file_event
         /home/tdevries/gdb/src/gdbsupport/event-loop.cc:573
0xdba537 gdb_wait_for_event
         /home/tdevries/gdb/src/gdbsupport/event-loop.cc:694
0xdb91cb _Z16gdb_do_one_eventi
         /home/tdevries/gdb/src/gdbsupport/event-loop.cc:264
0x868d9b start_event_loop
         /home/tdevries/gdb/src/gdb/main.c:411
0x868ee7 captured_command_loop
         /home/tdevries/gdb/src/gdb/main.c:471
0x86a807 captured_main
         /home/tdevries/gdb/src/gdb/main.c:1310
0x86a86f _Z8gdb_mainP18captured_main_args
         /home/tdevries/gdb/src/gdb/main.c:1325
0x418597 main
         /home/tdevries/gdb/src/gdb/gdb.c:32
...

Thanks,
- Tom
  
Luis Machado Jan. 23, 2023, 2:34 p.m. UTC | #2
On 1/23/23 12:57, Tom de Vries wrote:
> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>   gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
> 
> Hi,
> 
> on aarch64-linux I get:
> ...
> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)


I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.

> ...
> 
> In more detail:
> ...
> (gdb) PASS: gdb.base/frame-view.exp: with_pretty_printer=true: set scheduler-locking on
> frame^M
> #0  baz (z1=hahaha, /home/tdevries/gdb/src/gdb/value.c:4056: internal-error: value_fetch_lazy_register: Assertion `next_frame != NULL' failed.^M
> ...
> 
> 
> Back-trace:
> ...
> ----- Backtrace -----
> Resyncing due to internal error.
> 0x563033 gdb_internal_backtrace_1
>          /home/tdevries/gdb/src/gdb/bt-utils.c:122
> 0x5630f3 _Z22gdb_internal_backtracev
>          /home/tdevries/gdb/src/gdb/bt-utils.c:168
> 0xb4461f internal_vproblem
>          /home/tdevries/gdb/src/gdb/utils.c:396
> 0xb44a33 _Z15internal_verrorPKciS0_St9__va_list
>          /home/tdevries/gdb/src/gdb/utils.c:476
> 0xdb8e97 _Z18internal_error_locPKciS0_z
>          /home/tdevries/gdb/src/gdbsupport/errors.cc:58
> 0xb7a1d7 value_fetch_lazy_register
>          /home/tdevries/gdb/src/gdb/value.c:4056
> 0xb7a88b _Z16value_fetch_lazyP5value
>          /home/tdevries/gdb/src/gdb/value.c:4180
> 0xb74c93 _Z19value_optimized_outP5value
>          /home/tdevries/gdb/src/gdb/value.c:1500
> 0x770b2f _Z30frame_unwind_register_unsigned14frame_info_ptri
>          /home/tdevries/gdb/src/gdb/frame.c:1337
> 0x42bac3 aarch64_dwarf2_prev_register
>          /home/tdevries/gdb/src/gdb/aarch64-tdep.c:1280
> 0x680ec3 dwarf2_frame_prev_register
>          /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1217
> 0x770587 _Z27frame_unwind_register_value14frame_info_ptri
>          /home/tdevries/gdb/src/gdb/frame.c:1244
> 0x770ad3 _Z30frame_unwind_register_unsigned14frame_info_ptri
>          /home/tdevries/gdb/src/gdb/frame.c:1333
> 0x76c30f _Z17default_unwind_pcP7gdbarch14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/frame-unwind.c:244
> 0x4b652b _Z17gdbarch_unwind_pcP7gdbarch14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/gdbarch.c:2959
> 0x67d363 _Z29dwarf2_tailcall_sniffer_first14frame_info_ptrPPvPKl
>          /home/tdevries/gdb/src/gdb/dwarf2/frame-tailcall.c:390
> 0x680593 dwarf2_frame_cache
>          /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1089
> 0x68070f dwarf2_frame_unwind_stop_reason
>          /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1101
> 0x773503 get_prev_frame_always_1
>          /home/tdevries/gdb/src/gdb/frame.c:2281
> 0x773beb _Z21get_prev_frame_always14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/frame.c:2376
> 0x775d6b _Z28get_frame_unwind_stop_reason14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/frame.c:3051
> 0x68151f _Z16dwarf2_frame_cfa14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/dwarf2/frame.c:1356
> 0x678b13 _ZN18dwarf_expr_context16execute_stack_opEPKhS1_
>          /home/tdevries/gdb/src/gdb/dwarf2/expr.c:2110
> 0x676f4f _ZN18dwarf_expr_context4evalEPKhm
>          /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1239
> 0x677faf _ZN18dwarf_expr_context16execute_stack_opEPKhS1_
>          /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1811
> 0x676f4f _ZN18dwarf_expr_context4evalEPKhm
>          /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1239
> 0x6769e3 _ZN18dwarf_expr_context8evaluateEPKhmbP18dwarf2_per_cu_data14frame_info_ptrPK18property_addr_infoP4typeS9_l
>          /home/tdevries/gdb/src/gdb/dwarf2/expr.c:1078
> 0x6a924b dwarf2_evaluate_loc_desc_full
>          /home/tdevries/gdb/src/gdb/dwarf2/loc.c:1513
> 0x6a941f _Z24dwarf2_evaluate_loc_descP4type14frame_info_ptrPKhmP18dwarf2_per_cu_dataP18dwarf2_per_objfileb
>          /home/tdevries/gdb/src/gdb/dwarf2/loc.c:1558
> 0x6abb83 locexpr_read_variable
>          /home/tdevries/gdb/src/gdb/dwarf2/loc.c:3052
> 0x769083 _ZNK13language_defn14read_var_valueEP6symbolPK5block14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/findvar.c:578
> 0x769a17 _Z14read_var_valueP6symbolPK5block14frame_info_ptr
>          /home/tdevries/gdb/src/gdb/findvar.c:794
> 0xa188ef _Z14read_frame_argRK19frame_print_optionsP6symbol14frame_info_ptrP9frame_argS6_
>          /home/tdevries/gdb/src/gdb/stack.c:540
> 0xa19557 print_frame_args
>          /home/tdevries/gdb/src/gdb/stack.c:888
> 0xa1ae4b print_frame
>          /home/tdevries/gdb/src/gdb/stack.c:1390
> 0xa1a0e3 _Z16print_frame_infoRK19frame_print_options14frame_info_ptri10print_whatii
>          /home/tdevries/gdb/src/gdb/stack.c:1116
> 0xa180c7 _Z17print_stack_frame14frame_info_ptri10print_whati
>          /home/tdevries/gdb/src/gdb/stack.c:367
> 0xa1801b _Z26print_stack_frame_to_uioutP6ui_out14frame_info_ptri10print_whati
>          /home/tdevries/gdb/src/gdb/stack.c:346
> 0xa9d44f _Z27print_selected_thread_frameP6ui_out10enum_flagsI23user_selected_what_flagE
>          /home/tdevries/gdb/src/gdb/thread.c:1994
> 0xa1c907 frame_command_core
>          /home/tdevries/gdb/src/gdb/stack.c:1857
> 0xa20ec7 base_command
>          /home/tdevries/gdb/src/gdb/stack.c:1962
> 0x5b8f9f do_simple_func
>          /home/tdevries/gdb/src/gdb/cli/cli-decode.c:95
> 0x5be36b _Z8cmd_funcP16cmd_list_elementPKci
>          /home/tdevries/gdb/src/gdb/cli/cli-decode.c:2737
> 0xaa41ef _Z15execute_commandPKci
>          /home/tdevries/gdb/src/gdb/top.c:688
> 0x7421cf _Z15command_handlerPKc
>          /home/tdevries/gdb/src/gdb/event-top.c:616
> 0x74272f _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE
>          /home/tdevries/gdb/src/gdb/event-top.c:852
> 0xacc9c7 tui_command_line_handler
>          /home/tdevries/gdb/src/gdb/tui/tui-interp.c:104
> 0x74184f gdb_rl_callback_handler
>          /home/tdevries/gdb/src/gdb/event-top.c:246
> 0xffff957ba97f ???
> 0x74169b gdb_rl_callback_read_char_wrapper_noexcept
>          /home/tdevries/gdb/src/gdb/event-top.c:188
> 0x741733 gdb_rl_callback_read_char_wrapper
>          /home/tdevries/gdb/src/gdb/event-top.c:221
> 0x741fa3 stdin_event_handler
>          /home/tdevries/gdb/src/gdb/event-top.c:541
> 0xdba083 handle_file_event
>          /home/tdevries/gdb/src/gdbsupport/event-loop.cc:573
> 0xdba537 gdb_wait_for_event
>          /home/tdevries/gdb/src/gdbsupport/event-loop.cc:694
> 0xdb91cb _Z16gdb_do_one_eventi
>          /home/tdevries/gdb/src/gdbsupport/event-loop.cc:264
> 0x868d9b start_event_loop
>          /home/tdevries/gdb/src/gdb/main.c:411
> 0x868ee7 captured_command_loop
>          /home/tdevries/gdb/src/gdb/main.c:471
> 0x86a807 captured_main
>          /home/tdevries/gdb/src/gdb/main.c:1310
> 0x86a86f _Z8gdb_mainP18captured_main_args
>          /home/tdevries/gdb/src/gdb/main.c:1325
> 0x418597 main
>          /home/tdevries/gdb/src/gdb/gdb.c:32
> ...
> 
> Thanks,
> - Tom
  
Simon Marchi Jan. 24, 2023, 3:55 a.m. UTC | #3
On 1/23/23 09:34, Luis Machado wrote:
> On 1/23/23 12:57, Tom de Vries wrote:
>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>>   gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
>>
>> Hi,
>>
>> on aarch64-linux I get:
>> ...
>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)
> 
> 
> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.

I can reproduce too, I have a potential fix.  I'll try to send it
tomorrow.

Simon
  
Luis Machado Jan. 24, 2023, 8:22 a.m. UTC | #4
On 1/24/23 03:55, Simon Marchi wrote:
> 
> 
> On 1/23/23 09:34, Luis Machado wrote:
>> On 1/23/23 12:57, Tom de Vries wrote:
>>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>>>    gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
>>>
>>> Hi,
>>>
>>> on aarch64-linux I get:
>>> ...
>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)
>>
>>
>> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.
> 
> I can reproduce too, I have a potential fix.  I'll try to send it
> tomorrow.

Let me know if I can help with testing it on one of the targets.

> 
> Simon
  
Simon Marchi Jan. 25, 2023, 3:45 a.m. UTC | #5
On 1/24/23 03:22, Luis Machado wrote:
> On 1/24/23 03:55, Simon Marchi wrote:
>>
>>
>> On 1/23/23 09:34, Luis Machado wrote:
>>> On 1/23/23 12:57, Tom de Vries wrote:
>>>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>>>>    gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
>>>>
>>>> Hi,
>>>>
>>>> on aarch64-linux I get:
>>>> ...
>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)
>>>
>>>
>>> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.
>>
>> I can reproduce too, I have a potential fix.  I'll try to send it
>> tomorrow.
> 
> Let me know if I can help with testing it on one of the targets.

Ok, so I have a patch that fixes the test, but I'm not sure it's really
good.

First, here's my analysis of what I think is happening.

 - When we create the user frame (the "select-frame view" command), we
   create a sentinel frame just for our user-created frame, in
   create_new_frame.  This sentinel frame has the same id as the regular
   sentinel frame.
 - When printing the frame, after doing the "select-frame view"
   command, the argument's pretty printer is invoked, which does an
   inferior function call (this is the point of the test).  This clears
   the frame cache, including the "real" sentinel frame, which sets the
   sentinel_frame global to nullptr.
 - Later in the frame-printing process (when printing the second
   argument), the auto-reinflation mechanism (which would have been the
   manual reinflate call before, doesn't matter here) re-creates the
   user frame by calling create_new_frame again, creating its own
   special sentinel frame again.  However, note that the "real" sentinel
   frame, the sentinel_frame global, is still nullptr.  If the selected
   frame had been a regular frame, we would have called
   get_current_frame at some point during the reinflation, which would
   have re-created the "real" sentinel frame.  But it's not the case
   here.
 - Deep down the stack, something wants to fill in the unwind stop
   reason for frame 0, which requires trying to unwind frame 1.  This
   leads us to trying to unwind the PC of frame 1:

     #0  gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955
     #1  0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450)
         at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390
     #2  0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089
     #3  0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101
     #4  0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281
     #5  0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376
     #6  0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051
     #7  0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356
     #8  0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002")
         at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110
     #9  0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
     #10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811
     #11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
     #12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0, 
         type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078
     #13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, 
         subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513
     #14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true)
         at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557
     #15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052
 - AArch64 defines a special "prev register" function,
   aarch64_dwarf2_prev_register, to handle unwinding the PC.  This
   function does

     frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);

   (I must admit that I don't really understand this part, it seems to
    me like you would want to unwind LR's value from this_frame->next...
    but anyway.)
 - frame_unwind_register_unsigned ultimately creates a lazy register
   value, saving the frame id of this_frame->next.  this_frame is the
   user-created frame, to this_frame->next is the special sentinel frame
   we created for it.
 - When time comes to un-lazify the value, value_fetch_lazy_register
   calls frame_find_by_id, to find the frame with the id we saved.
 - frame_find_by_id sees it's the sentinel frame id, so returns the
   sentinel_frame global, which is, if you remember, nullptr.
 - We hit the `gdb_assert (next_frame != NULL)` assertion.

I have a hard time pinpointing a single thing that is wrong.  The fact
that the user-created frames have their own sentinel frame, which share
the same id as the real sentinel frame, that sounds like it can't work
with frame_find_by_id.  In the case without the pretty printer, where
things appear to work, it means that frame_find_by_id returns the "real"
sentinel frame, part of the "real" stack frame chain, and not the
sentinel that is attached to the user-created frame.  Things still kinda
work in the end because those two sentinel frames behave the same, as
far as "prev register" is concerned.

Ultimately, I don't really understand how this "frame view" feature is
useful.  Sure, we pretend that the frame's sp and pc is what the user
gave, but what about the other registers?  It seems to me like you are
very likely to get bogus values.

The patch I had in mind was to give user-created frames a special
unwinder, rather than letting other unwinders (the arch-specific ones,
the dwarf2 one, etc) kick in.  That unwinder would always return the
unwind stop reason UNWIND_OUTERMOST, which effectively makes the
user-created frame an outer frame, and we would never try to unwind any
register value from it.  It makes the test pass, because it breaks the
chain of event explained above.  But it's probably just papering over
the actual problem.

Simon
  
Luis Machado Jan. 30, 2023, 8:49 a.m. UTC | #6
On 1/25/23 03:45, Simon Marchi wrote:
> 
> 
> On 1/24/23 03:22, Luis Machado wrote:
>> On 1/24/23 03:55, Simon Marchi wrote:
>>>
>>>
>>> On 1/23/23 09:34, Luis Machado wrote:
>>>> On 1/23/23 12:57, Tom de Vries wrote:
>>>>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>>>>>     gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
>>>>>
>>>>> Hi,
>>>>>
>>>>> on aarch64-linux I get:
>>>>> ...
>>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
>>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)
>>>>
>>>>
>>>> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.
>>>
>>> I can reproduce too, I have a potential fix.  I'll try to send it
>>> tomorrow.
>>
>> Let me know if I can help with testing it on one of the targets.
> 
> Ok, so I have a patch that fixes the test, but I'm not sure it's really
> good.
> 
> First, here's my analysis of what I think is happening.
> 
>   - When we create the user frame (the "select-frame view" command), we
>     create a sentinel frame just for our user-created frame, in
>     create_new_frame.  This sentinel frame has the same id as the regular
>     sentinel frame.
>   - When printing the frame, after doing the "select-frame view"
>     command, the argument's pretty printer is invoked, which does an
>     inferior function call (this is the point of the test).  This clears
>     the frame cache, including the "real" sentinel frame, which sets the
>     sentinel_frame global to nullptr.
>   - Later in the frame-printing process (when printing the second
>     argument), the auto-reinflation mechanism (which would have been the
>     manual reinflate call before, doesn't matter here) re-creates the
>     user frame by calling create_new_frame again, creating its own
>     special sentinel frame again.  However, note that the "real" sentinel
>     frame, the sentinel_frame global, is still nullptr.  If the selected
>     frame had been a regular frame, we would have called
>     get_current_frame at some point during the reinflation, which would
>     have re-created the "real" sentinel frame.  But it's not the case
>     here.
>   - Deep down the stack, something wants to fill in the unwind stop
>     reason for frame 0, which requires trying to unwind frame 1.  This
>     leads us to trying to unwind the PC of frame 1:
> 
>       #0  gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955
>       #1  0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450)
>           at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390
>       #2  0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089
>       #3  0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101
>       #4  0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281
>       #5  0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376
>       #6  0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051
>       #7  0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356
>       #8  0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002")
>           at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110
>       #9  0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
>       #10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811
>       #11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
>       #12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0,
>           type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078
>       #13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980,
>           subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513
>       #14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true)
>           at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557
>       #15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052
>   - AArch64 defines a special "prev register" function,
>     aarch64_dwarf2_prev_register, to handle unwinding the PC.  This
>     function does
> 
>       frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);
> 
>     (I must admit that I don't really understand this part, it seems to
>      me like you would want to unwind LR's value from this_frame->next...
>      but anyway.)

It is somewhat historical, and it is something I'm planning to address eventually. Right now, though, we hardwire the PC to
obtaining LR.

>   - frame_unwind_register_unsigned ultimately creates a lazy register
>     value, saving the frame id of this_frame->next.  this_frame is the
>     user-created frame, to this_frame->next is the special sentinel frame
>     we created for it.
>   - When time comes to un-lazify the value, value_fetch_lazy_register
>     calls frame_find_by_id, to find the frame with the id we saved.
>   - frame_find_by_id sees it's the sentinel frame id, so returns the
>     sentinel_frame global, which is, if you remember, nullptr.
>   - We hit the `gdb_assert (next_frame != NULL)` assertion.
> 
> I have a hard time pinpointing a single thing that is wrong.  The fact
> that the user-created frames have their own sentinel frame, which share
> the same id as the real sentinel frame, that sounds like it can't work

 From your (very detailed) explanation, it does sound a bit iffy to abuse the frame id mechanism and use
the same id as the existing real frame.

> with frame_find_by_id.  In the case without the pretty printer, where
> things appear to work, it means that frame_find_by_id returns the "real"
> sentinel frame, part of the "real" stack frame chain, and not the
> sentinel that is attached to the user-created frame.  Things still kinda
> work in the end because those two sentinel frames behave the same, as
> far as "prev register" is concerned.
> 
> Ultimately, I don't really understand how this "frame view" feature is
> useful.  Sure, we pretend that the frame's sp and pc is what the user
> gave, but what about the other registers?  It seems to me like you are
> very likely to get bogus values.
> 
> The patch I had in mind was to give user-created frames a special
> unwinder, rather than letting other unwinders (the arch-specific ones,
> the dwarf2 one, etc) kick in.  That unwinder would always return the
> unwind stop reason UNWIND_OUTERMOST, which effectively makes the
> user-created frame an outer frame, and we would never try to unwind any
> register value from it.  It makes the test pass, because it breaks the
> chain of event explained above.  But it's probably just papering over
> the actual problem.
> 
> Simon
  
Simon Marchi Jan. 30, 2023, 4:20 p.m. UTC | #7
On 1/30/23 03:49, Luis Machado wrote:
> On 1/25/23 03:45, Simon Marchi wrote:
>>
>>
>> On 1/24/23 03:22, Luis Machado wrote:
>>> On 1/24/23 03:55, Simon Marchi wrote:
>>>>
>>>>
>>>> On 1/23/23 09:34, Luis Machado wrote:
>>>>> On 1/23/23 12:57, Tom de Vries wrote:
>>>>>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>>>>>>     gdb/testsuite/gdb.base/frame-view.exp    | 47 ++++++++++++--
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> on aarch64-linux I get:
>>>>>> ...
>>>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
>>>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)
>>>>>
>>>>>
>>>>> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.
>>>>
>>>> I can reproduce too, I have a potential fix.  I'll try to send it
>>>> tomorrow.
>>>
>>> Let me know if I can help with testing it on one of the targets.
>>
>> Ok, so I have a patch that fixes the test, but I'm not sure it's really
>> good.
>>
>> First, here's my analysis of what I think is happening.
>>
>>   - When we create the user frame (the "select-frame view" command), we
>>     create a sentinel frame just for our user-created frame, in
>>     create_new_frame.  This sentinel frame has the same id as the regular
>>     sentinel frame.
>>   - When printing the frame, after doing the "select-frame view"
>>     command, the argument's pretty printer is invoked, which does an
>>     inferior function call (this is the point of the test).  This clears
>>     the frame cache, including the "real" sentinel frame, which sets the
>>     sentinel_frame global to nullptr.
>>   - Later in the frame-printing process (when printing the second
>>     argument), the auto-reinflation mechanism (which would have been the
>>     manual reinflate call before, doesn't matter here) re-creates the
>>     user frame by calling create_new_frame again, creating its own
>>     special sentinel frame again.  However, note that the "real" sentinel
>>     frame, the sentinel_frame global, is still nullptr.  If the selected
>>     frame had been a regular frame, we would have called
>>     get_current_frame at some point during the reinflation, which would
>>     have re-created the "real" sentinel frame.  But it's not the case
>>     here.
>>   - Deep down the stack, something wants to fill in the unwind stop
>>     reason for frame 0, which requires trying to unwind frame 1.  This
>>     leads us to trying to unwind the PC of frame 1:
>>
>>       #0  gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955
>>       #1  0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450)
>>           at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390
>>       #2  0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089
>>       #3  0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101
>>       #4  0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281
>>       #5  0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376
>>       #6  0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051
>>       #7  0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356
>>       #8  0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002")
>>           at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110
>>       #9  0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
>>       #10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811
>>       #11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
>>       #12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0,
>>           type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078
>>       #13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980,
>>           subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513
>>       #14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true)
>>           at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557
>>       #15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052
>>   - AArch64 defines a special "prev register" function,
>>     aarch64_dwarf2_prev_register, to handle unwinding the PC.  This
>>     function does
>>
>>       frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);
>>
>>     (I must admit that I don't really understand this part, it seems to
>>      me like you would want to unwind LR's value from this_frame->next...
>>      but anyway.)
> 
> It is somewhat historical, and it is something I'm planning to address eventually. Right now, though, we hardwire the PC to
> obtaining LR.

Ok, I'd be interesting in knowing more about the situation, for my own
general knowledge.  I might be understanding the GDB unwinding wrong of
the ARM-specific unwinding wrong.

>>   - frame_unwind_register_unsigned ultimately creates a lazy register
>>     value, saving the frame id of this_frame->next.  this_frame is the
>>     user-created frame, to this_frame->next is the special sentinel frame
>>     we created for it.
>>   - When time comes to un-lazify the value, value_fetch_lazy_register
>>     calls frame_find_by_id, to find the frame with the id we saved.
>>   - frame_find_by_id sees it's the sentinel frame id, so returns the
>>     sentinel_frame global, which is, if you remember, nullptr.
>>   - We hit the `gdb_assert (next_frame != NULL)` assertion.
>>
>> I have a hard time pinpointing a single thing that is wrong.  The fact
>> that the user-created frames have their own sentinel frame, which share
>> the same id as the real sentinel frame, that sounds like it can't work
> 
> From your (very detailed) explanation, it does sound a bit iffy to abuse the frame id mechanism and use
> the same id as the existing real frame.

Yeah, my latest attempt at fixing it revolves around giving the
user-created frame's sentinels their own specific ID.

Simon
  

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0f5df2ccb7b0..7859315cf366 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -459,6 +459,7 @@  SELFTESTS_SRCS = \
 	unittests/environ-selftests.c \
 	unittests/filtered_iterator-selftests.c \
 	unittests/format_pieces-selftests.c \
+	unittests/frame_info_ptr-selftests.c \
 	unittests/function-view-selftests.c \
 	unittests/gdb_tilde_expand-selftests.c \
 	unittests/gmp-utils-selftests.c \
diff --git a/gdb/frame.c b/gdb/frame.c
index 9fba29ec13eb..873c3912e076 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -3213,7 +3213,8 @@  frame_info_ptr::prepare_reinflate ()
 {
   m_cached_level = frame_relative_level (*this);
 
-  if (m_cached_level != 0)
+  if (m_cached_level != 0
+      || (m_ptr != nullptr && m_ptr->this_id.value.user_created_p))
     m_cached_id = get_frame_id (*this);
 }
 
@@ -3232,13 +3233,22 @@  frame_info_ptr::reinflate ()
       return;
     }
 
-  /* Frame #0 needs special handling, see comment in select_frame.  */
-  if (m_cached_level == 0)
-    m_ptr = get_current_frame ().get ();
+  if (m_cached_id.user_created_p)
+    m_ptr = create_new_frame (m_cached_id).get ();
   else
     {
-      gdb_assert (frame_id_p (m_cached_id));
-      m_ptr = frame_find_by_id (m_cached_id).get ();
+      /* Frame #0 needs special handling, see comment in select_frame.  */
+      if (m_cached_level == 0)
+	m_ptr = get_current_frame ().get ();
+      else
+	{
+	  /* If we reach here without a valid frame id, it means we are trying
+	     to reinflate a frame whose id was not know at construction time.
+	     We're probably trying to reinflate a frame while computing its id
+	     which is not possible, and would indicate a problem with GDB.  */
+	  gdb_assert (frame_id_p (m_cached_id));
+	  m_ptr = frame_find_by_id (m_cached_id).get ();
+	}
     }
 
   gdb_assert (m_ptr != nullptr);
diff --git a/gdb/frame.h b/gdb/frame.h
index ebbd9e08868e..88af96f0c619 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -325,7 +325,12 @@  class frame_info_ptr : public intrusive_list_node<frame_info_ptr>
   /* The underlying pointer.  */
   frame_info *m_ptr = nullptr;
 
-  /* The frame_id of the underlying pointer.  */
+  /* The frame_id of the underlying pointer.
+
+     For the current target frames (frames with level 0, obtained through
+     get_current_frame), we don't save the frame id, we leave it at
+     null_frame_id.  For user-created frames (also with level 0, but created
+     with create_new_frame), we do save the id.  */
   frame_id m_cached_id = null_frame_id;
 
   /* The frame level of the underlying pointer.  */
diff --git a/gdb/testsuite/gdb.base/frame-view.c b/gdb/testsuite/gdb.base/frame-view.c
index 7b1cbd6abd42..e330da0b1afe 100644
--- a/gdb/testsuite/gdb.base/frame-view.c
+++ b/gdb/testsuite/gdb.base/frame-view.c
@@ -28,6 +28,12 @@  struct type_2
   int n;
 };
 
+__attribute__((used)) static int
+called_from_pretty_printer (void)
+{
+  return 23;
+}
+
 static int
 baz (struct type_1 z1, struct type_2 z2)
 {
diff --git a/gdb/testsuite/gdb.base/frame-view.exp b/gdb/testsuite/gdb.base/frame-view.exp
index d2dba143448d..6c6e4e3fcc37 100644
--- a/gdb/testsuite/gdb.base/frame-view.exp
+++ b/gdb/testsuite/gdb.base/frame-view.exp
@@ -22,13 +22,29 @@  if { [build_executable "failed to prepare" \
     return
 }
 
-proc test_select_frame_view {} {
+# If WITH_PRETTY_PRINTER is true, load pretty printers for the function
+# parameter types, in which we do an inferior call.  This is meant to test
+# that the frame_info_ptr correctly reinflates frames created using
+# "select-frame view".
+
+proc test_select_frame_view { with_pretty_printer } {
     clean_restart $::binfile
 
+    if { $with_pretty_printer && [skip_python_tests] } {
+	untested "skipping Python test"
+	return
+    }
+
     if { ![runto_main] } {
 	return
     }
 
+    if { $with_pretty_printer } {
+	set remote_python_file \
+	    [gdb_remote_download host "${::srcdir}/${::subdir}/${::testfile}.py"]
+	gdb_test_no_output "source ${remote_python_file}" "load python file"
+    }
+
     # Stop thread 2 at a baz.
     gdb_test "break baz"
     gdb_test "continue" "Thread 2.*hit Breakpoint $::decimal, baz .*"
@@ -60,11 +76,34 @@  proc test_select_frame_view {} {
     gdb_test "thread 1" "Switching to thread 1 .*"
     gdb_test_no_output "select-frame view $frame_sp $frame_pc"
 
+    if { $with_pretty_printer } {
+	# When the pretty printer does its infcall, it is done on the currently
+	# selected thread, thread 1 here.  However, other threads are resumed
+	# at the same time.  This causes thread 2 to exit during that infcall,
+	# leading to this weirdness:
+	#
+	#     frame^M
+	#     #0  baz (z=[Thread 0x7ffff7cc26c0 (LWP 417519) exited]^M
+	#     hohoho) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:35^M
+	#     35        return z.n + 2;^M
+	#
+	# Avoid that by setting scheduler-locking on.
+	gdb_test_no_output "set scheduler-locking on"
+
+	set z1_pattern "hahaha"
+	set z2_pattern "hohoho"
+    } else {
+	set z1_pattern "\\.\\.\\."
+	set z2_pattern "\\.\\.\\."
+    }
+
     # Verify that the "frame" command does not change the selected frame.
     # There used to be a bug where the "frame" command would lose the
     # selection of user-created frames.
-    gdb_test "frame" "#0  baz \\(z1=.*, z2=.*\\).*" "frame"
-    gdb_test "frame" "#0  baz \\(z1=.*, z2=.*\\).*" "frame again"
+    gdb_test "frame" "#0  baz \\(z1=$z1_pattern, z2=$z2_pattern\\).*" "frame"
+    gdb_test "frame" "#0  baz \\(z1=$z1_pattern, z2=$z2_pattern\\).*" "frame again"
 }
 
-test_select_frame_view
+foreach_with_prefix with_pretty_printer {false true} {
+    test_select_frame_view $with_pretty_printer
+}
diff --git a/gdb/testsuite/gdb.base/frame-view.py b/gdb/testsuite/gdb.base/frame-view.py
new file mode 100644
index 000000000000..6c1b8bcf82be
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frame-view.py
@@ -0,0 +1,41 @@ 
+# Copyright (C) 2022 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/>.
+
+
+class Printer1:
+    def to_string(self):
+        n = gdb.parse_and_eval("called_from_pretty_printer ()")
+        assert n == 23
+        return "hahaha"
+
+
+class Printer2:
+    def to_string(self):
+        n = gdb.parse_and_eval("called_from_pretty_printer ()")
+        assert n == 23
+        return "hohoho"
+
+
+def lookup_function(val):
+    if str(val.type) == "struct type_1":
+        return Printer1()
+
+    if str(val.type) == "struct type_2":
+        return Printer2()
+
+    return None
+
+
+gdb.pretty_printers.append(lookup_function)
diff --git a/gdb/unittests/frame_info_ptr-selftests.c b/gdb/unittests/frame_info_ptr-selftests.c
new file mode 100644
index 000000000000..edf1b9fa0bb7
--- /dev/null
+++ b/gdb/unittests/frame_info_ptr-selftests.c
@@ -0,0 +1,80 @@ 
+/* Self tests for frame_info_ptr.
+
+   Copyright (C) 2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "defs.h"
+
+#include "frame.h"
+#include "gdbsupport/selftest.h"
+#include "scoped-mock-context.h"
+#include "test-target.h"
+
+namespace selftests {
+
+static void
+validate_user_created_frame (frame_id id)
+{
+  SELF_CHECK (id.stack_status == FID_STACK_VALID);
+  SELF_CHECK (id.stack_addr == 0x1234);
+  SELF_CHECK (id.code_addr_p);
+  SELF_CHECK (id.code_addr == 0x5678);
+}
+
+static frame_info_ptr
+user_created_frame_callee (frame_info_ptr frame)
+{
+  validate_user_created_frame (get_frame_id (frame));
+
+  frame.prepare_reinflate ();
+  reinit_frame_cache ();
+  frame.reinflate ();
+
+  validate_user_created_frame (get_frame_id (frame));
+
+  return frame;
+}
+
+static void
+test_user_created_frame ()
+{
+  scoped_mock_context<test_target_ops> mock_context
+    (current_inferior ()->gdbarch);
+  frame_info_ptr frame = create_new_frame (0x1234, 0x5678);
+
+  validate_user_created_frame (get_frame_id (frame));
+
+  /* Pass the frame to a callee, which calls reinit_frame_cache.  This lets us
+     validate that the reinflation in both the callee and caller restore the
+     same frame_info object.  */
+  frame.prepare_reinflate ();
+  frame_info_ptr callees_frame_info = user_created_frame_callee (frame);
+  frame.reinflate ();
+
+  validate_user_created_frame (get_frame_id (frame));
+  SELF_CHECK (frame.get () == callees_frame_info.get ());
+}
+
+} /* namespace selftests */
+
+void _initialize_frame_info_ptr_selftests ();
+void
+_initialize_frame_info_ptr_selftests ()
+{
+  selftests::register_test ("frame_info_ptr_user",
+			    selftests::test_user_created_frame);
+}