Re: [PATCH] Guard against frame.c destructors running before frame-info.c's

[Simon Marchi <simon.marchi@polymtl.ca>]

On 11/15/22 15:48, Kévin Le Gouguec wrote:
> On x86_64-windows, since 04e2ac7b2a7, we observe this internal error:
> 
>   [...]/gdbsupport/intrusive_list.h:458: internal-error: erase_element:
>   Assertion `elem_node->prev != INTRUSIVE_LIST_UNLINKED_VALUE' failed.
> 
> Breaking in the destructors for intrusive_list and frame_info_ptr shows that
> in this configuration, the destructors for frame.c's statically-stored
> objects are run before frame-info.c's:
> 
>   Thread 1 hit Breakpoint 7, intrusive_list<frame_info_ptr,
>   intrusive_base_node<frame_info_ptr> >::~intrusive_list (this=0x7ff69c418c90
>   <frame_info_ptr::frame_list>, __in_chrg=<optimized out>)
>   [...]/../gdbsupport/intrusive_list.h:250
>   250	    clear ();
>   (gdb) bt
>   #0  intrusive_list<frame_info_ptr, intrusive_base_node<frame_info_ptr> >
>       ::~intrusive_list (this=0x7ff69c418c90 <frame_info_ptr::frame_list>,
>       __in_chrg=<optimized out>) [...]/../gdbsupport/intrusive_list.h:250
>   #1  0x00007ff69b78edba in __tcf_1 () [...]/frame-info.c:27
>   #2  0x00007ff9c457aa9f in msvcrt!_initterm_e ()
>       from C:\Windows\System32\msvcrt.dll
>   #3  0x00007ff69b8246a6 in captured_main_1 (context=0x5ffe00)
>       [...]/main.c:1111
>   #4  0x00007ff69b825149 in captured_main (data=0x5ffe00) [...]/main.c:1320
>   #5  0x00007ff69b8251b1 in gdb_main (args=0x5ffe00) [...]/main.c:1345
>   #6  0x00007ff69b5d1730 in main (argc=2, argv=0x751730) [...]/gdb.c:32
>   (gdb) c
>   Continuing.
> 
>   Thread 1 hit Breakpoint 8, frame_info_ptr::~frame_info_ptr
>   (this=0x7ff69c418e20 <selected_frame>, __in_chrg=<optimized out>)
>   [...]/frame-info.h:74
>   74	    if (is_linked ())
>   (gdb) bt
>   #0  frame_info_ptr::~frame_info_ptr (this=0x7ff69c418e20 <selected_frame>,
>       __in_chrg=<optimized out>) [...]/frame-info.h:74
>   #1  0x00007ff69b79a643 in __tcf_1 () [...]/frame.c:1675
>   #2  0x00007ff9c457aa9f in msvcrt!_initterm_e () from
>       C:\Windows\System32\msvcrt.dll
>   #3  0x00007ff69b8246a6 in captured_main_1 (context=0x5ffe00)
>       [...]/main.c:1111
>   #4  0x00007ff69b825149 in captured_main (data=0x5ffe00) [...]/main.c:1320
>   #5  0x00007ff69b8251b1 in gdb_main (args=0x5ffe00) [...]/main.c:1345
>   #6  0x00007ff69b5d1730 in main (argc=2, argv=0x751730) [...]/gdb.c:32
> ---
>  gdb/frame-info.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/frame-info.h b/gdb/frame-info.h
> index 3369b114326..893b6632363 100644
> --- a/gdb/frame-info.h
> +++ b/gdb/frame-info.h
> @@ -76,7 +76,11 @@ class frame_info_ptr : public intrusive_list_node<frame_info_ptr>
>  
>    ~frame_info_ptr ()
>    {
> -    frame_list.erase (frame_list.iterator_to (*this));
> +    /* If this node has static storage, it may be deleted after
> +       frame_list.  Attempting to erase ourselves would then trigger
> +       internal errors, so make sure we are still linked first.  */
> +    if (is_linked ())
> +      frame_list.erase (frame_list.iterator_to (*this));
>    }
>  
>    frame_info_ptr &operator= (const frame_info_ptr &other)
> -- 
> 2.25.1
> 

Do you have a way to reproduce, so we can experiment with it?

Simon