[PATCH 01/23] Preserve selected thread in all-stop w/ background execution
Aktemur, Tankut Baris
tankut.baris.aktemur@intel.com
Wed Oct 9 09:36:00 GMT 2019
* On September 7, 2019 1:28 AM, Pedro Alves wrote:
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index a9588f896a..9c888aa72f 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3048,6 +3048,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
>
> finish_state.release ();
>
> + /* If we've switched threads above, switch back to the previously
> + current thread. We don't want the user to see a different
> + selected thread. */
> + switch_to_thread (cur_thr);
> +
> /* Tell the event loop to wait for it to stop. If the target
> supports asynchronous execution, it'll do this from within
> target_resume. */
> @@ -3702,14 +3707,11 @@ fetch_inferior_event (void *client_data)
> set_current_traceframe (-1);
> }
>
> - gdb::optional<scoped_restore_current_thread> maybe_restore_thread;
> -
> - if (non_stop)
> - /* In non-stop mode, the user/frontend should not notice a thread
> - switch due to internal events. Make sure we reverse to the
> - user selected thread and frame after handling the event and
> - running any breakpoint commands. */
> - maybe_restore_thread.emplace ();
> + /* The user/frontend should not notice a thread switch due to
> + internal events. Make sure we revert to the user selected
> + thread and frame after handling the event and running any
> + breakpoint commands. */
> + scoped_restore_current_thread restore_thread;
>
Because this increases the refcount of the current thread, in case the
fetched inferior event denotes a thread exit, the thread will not be deleted
right away. A non-deleted but exited thread stays in the inferior's thread
list. This, in turn, causes the "init_thread_list" call in inferior.c to
be skipped. As a side effect, a regression is observed in
gdb.arch/i386-mpx-simple_segv.exp
IMHO, the 'any_thread_p' predicate should be updated. This predicate is used
in two places (one in 'inferior.c' and the other in 'mi/mi-main.c'). Both
uses, I believe, are in fact interested in whether there are any non-exited
threads. I'd suggest updating 'any_thread_p' to 'any_non_exited_thread_p'.
I'm attaching a patch that can be used to test.
> overlay_cache_invalid = 1;
> /* Flush target cache before starting to handle each event. Target
> @@ -3786,6 +3788,19 @@ fetch_inferior_event (void *client_data)
> inferior_event_handler (INF_EXEC_COMPLETE, NULL);
> cmd_done = 1;
> }
> +
> + /* If we got a TARGET_WAITKIND_NO_RESUMED event, then the
> + previously selected thread is gone. We have two
> + choices - switch to no thread selected, or restore the
> + previously selected thread (now exited). We chose the
> + later, just because that's what GDB used to do. After
> + this, "info threads" says "The current thread <Thread
> + ID 2> has terminated." instead of "No thread
> + selected.". */
> + if (!non_stop
> + && cmd_done
> + && ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED)
> + restore_thread.dont_restore ();
> }
> }
>
The comment and the code seem to contradict each other. The comment says
"if we got a TARGET_WAITKIND_NO_RESUMED" whereas the condition is
ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED
Should TARGET_WAITKIND_THREAD_EXITED, TARGET_WAITKIND_EXITED, and
TARGET_WAITKIND_SIGNALLED be also included in the condition? They also mean
that the thread is gone, right?
Regards,
-Baris
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
-------------- next part --------------
A non-text attachment was scrubbed...
Name: any_thread_p.patch
Type: application/octet-stream
Size: 1974 bytes
Desc: any_thread_p.patch
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20191009/0ab98c44/attachment.obj>
More information about the Gdb-patches
mailing list