From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1879) id F2EAD3898C6F; Fri, 29 Apr 2022 13:28:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F2EAD3898C6F Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Simon Marchi To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/infrun: make fetch_inferior_event restore thread if exited or signalled X-Act-Checkin: binutils-gdb X-Git-Author: Simon Marchi X-Git-Refname: refs/heads/master X-Git-Oldrev: d51926f06a7f4854bebdd71dcb0a78dbaa2f4168 X-Git-Newrev: 225170409b47bc96b62b2ecfcc0d9d5ae1ef8949 Message-Id: <20220429132829.F2EAD3898C6F@sourceware.org> Date: Fri, 29 Apr 2022 13:28:29 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Apr 2022 13:28:30 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D225170409b47= bc96b62b2ecfcc0d9d5ae1ef8949 commit 225170409b47bc96b62b2ecfcc0d9d5ae1ef8949 Author: Simon Marchi Date: Sat Apr 23 23:20:48 2022 -0400 gdb/infrun: make fetch_inferior_event restore thread if exited or signa= lled =20 Commit 152a1749566 ("gdb: prune inferiors at end of fetch_inferior_event, fix intermittent failure of gdb.threads/fork-plus-threads.exp") introduced some follow-fork-related test failures, such as: =20 info inferiors^M Num Description Connection Executable ^M * 1 process 634972 1 (native) /home/simark/build/bi= nutils-gdb-one-target/gdb/testsuite/outputs/gdb.base/foll-fork/foll-fork ^M 2 process 634975 1 (native) /home/simark/build/bi= nutils-gdb-one-target/gdb/testsuite/outputs/gdb.base/foll-fork/foll-fork ^M (gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=3Dparent: deta= ch-on-fork=3Doff: cmd=3Dnext 2: test_follow_fork: info inferiors inferior 2^M [Switching to inferior 2 [process 634975] (/home/simark/build/binut= ils-gdb-one-target/gdb/testsuite/outputs/gdb.base/foll-fork/foll-fork)]^M [Switching to thread 2.1 (Thread 0x7ffff7c9a740 (LWP 634975))]^M #0 0x00007ffff7d7abf7 in _Fork () from /usr/lib/libc.so.6^M (gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=3Dparent: deta= ch-on-fork=3Doff: cmd=3Dnext 2: test_follow_fork: inferior 2 continue^M Continuing.^M [Inferior 2 (process 634975) exited normally]^M [Switching to Thread 0x7ffff7c9a740 (LWP 634972)]^M (gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=3Dparent: deta= ch-on-fork=3Doff: cmd=3Dnext 2: test_follow_fork: continue until exit at co= ntinue unfollowed inferior to end break callee^M Breakpoint 2 at 0x555555555160: file /home/simark/src/binutils-gdb/= gdb/testsuite/gdb.base/foll-fork.c, line 9.^M (gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=3Dparent: deta= ch-on-fork=3Doff: cmd=3Dnext 2: test_follow_fork: break callee =20 What happens here is: =20 - inferior 2 is selected - we continue, leading to inferior 2's exit - we set breakpoint, expect 2 locations, but only one location is resolved =20 Reading between the lines, we understand that inferior 2 got pruned, when it shouldn't have been. =20 The issue can be reproduced by hand with: =20 $ ./gdb -q --data-directory=3Ddata-directory testsuite/outputs/gdb.= base/foll-fork/foll-fork -ex "set detach-on-fork off" -ex start -ex "next 2= " -ex "inferior 2" -ex "set debug infrun" ... Temporary breakpoint 1, main () at /home/simark/src/binutils-gdb/gd= b/testsuite/gdb.base/foll-fork.c:14 14 int v =3D 5; [New inferior 2 (process 637627)] [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1". 17 if (pid =3D=3D 0) /* set breakpoint here */ [Switching to inferior 2 [process 637627] (/home/simark/build/binut= ils-gdb-one-target/gdb/testsuite/outputs/gdb.base/foll-fork/foll-fork)] [Switching to thread 2.1 (Thread 0x7ffff7c9a740 (LWP 637627))] #0 0x00007ffff7d7abf7 in _Fork () from /usr/lib/libc.so.6 (gdb) continue Continuing. [infrun] clear_proceed_status_thread: 637627.637627.0 [infrun] proceed: enter [infrun] proceed: addr=3D0xffffffffffffffff, signal=3DGDB_SIGNAL_= DEFAULT [infrun] scoped_disable_commit_resumed: reason=3Dproceeding [infrun] start_step_over: enter [infrun] start_step_over: stealing global queue of threads to s= tep, length =3D 0 [infrun] operator(): step-over queue now empty [infrun] start_step_over: exit [infrun] proceed: start: resuming threads, all-stop-on-top-of-non= -stop [infrun] proceed: resuming 637627.637627.0 [infrun] resume_1: step=3D0, signal=3DGDB_SIGNAL_0, trap_expect= ed=3D0, current thread [637627.637627.0] at 0x7ffff7d7abf7 [infrun] do_target_resume: resume_ptid=3D637627.637627.0, step= =3D0, sig=3DGDB_SIGNAL_0 [infrun] infrun_async: enable=3D1 [infrun] prepare_to_wait: prepare_to_wait [infrun] proceed: end: resuming threads, all-stop-on-top-of-non-s= top [infrun] reset: reason=3Dproceeding [infrun] maybe_set_commit_resumed_all_targets: enabling commit-re= sumed for target native [infrun] maybe_call_commit_resumed_all_targets: calling commit_re= sumed for target native [infrun] maybe_call_commit_resumed_all_targets: calling commit_re= sumed for target native [infrun] proceed: exit [infrun] fetch_inferior_event: enter [infrun] scoped_disable_commit_resumed: reason=3Dhandling event [infrun] do_target_wait: Found 2 inferiors, starting at #1 [infrun] random_pending_event_thread: None found. [infrun] print_target_wait_results: target_wait (-1.0.0 [process = -1], status) =3D [infrun] print_target_wait_results: 637627.637627.0 [process 63= 7627], [infrun] print_target_wait_results: status->kind =3D EXITED, ex= it_status =3D 0 [infrun] handle_inferior_event: status->kind =3D EXITED, exit_sta= tus =3D 0 [Inferior 2 (process 637627) exited normally] [infrun] stop_waiting: stop_waiting [infrun] stop_all_threads: start: reason=3Dpresenting stop to use= r in all-stop, inf=3D-1 [infrun] stop_all_threads: pass=3D0, iterations=3D0 [infrun] stop_all_threads: 637624.637624.0 not executing [infrun] stop_all_threads: pass=3D1, iterations=3D1 [infrun] stop_all_threads: 637624.637624.0 not executing [infrun] stop_all_threads: done [infrun] stop_all_threads: end: reason=3Dpresenting stop to user = in all-stop, inf=3D-1 [Switching to Thread 0x7ffff7c9a740 (LWP 637624)] [infrun] infrun_async: enable=3D0 [infrun] reset: reason=3Dhandling event [infrun] maybe_set_commit_resumed_all_targets: not requesting com= mit-resumed for target native, no resumed threads (gdb) [infrun] fetch_inferior_event: exit (gdb) info inferiors Num Description Connection Executable * 1 process 637624 1 (native) /home/simark/build/bi= nutils-gdb-one-target/gdb/testsuite/outputs/gdb.base/foll-fork/foll-fork (gdb) i th Id Target Id Frame * 1 Thread 0x7ffff7c9a740 (LWP 637624) "foll-fork" main () at /h= ome/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:17 =20 After handling the EXITED event for inferior 2, inferior 2 should have stayed the current inferior, which should have prevented it from getting pruned. When debugging, we find that when getting at the prune_inferiors call, the current inferior is inferior 1. Further debugging shows that prior to the call to clean_up_just_stopped_threads_fsms, the current inferior is inferior 2, and after, it's inferior 1. Then, back in fetch_inferior_event, the restore_thread object is disabled, due to: =20 /* 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 has terminated." instead of "No thread selected.". */ if (!non_stop && cmd_done && ecs->ws.kind () !=3D TARGET_WAITKIND_NO_RESUMED) restore_thread.dont_restore (); =20 So in the end, inferior 1 stays current, and inferior 2 gets wrongfully pruned. =20 I'd say clean_up_just_stopped_threads_fsms is the culprit here. It actually attempts to restore the event_thread to be current at the end, after the loop (I presume the current thread on entry is always supposed to be the event thread). But in this case, the event is of kind EXITED, and ecs->event_thread is not set, so the current inferior isn't restored. =20 Fix that by using scoped_restore_current_thread. If there is no current thread, scoped_restore_current_thread will still restore the current inferior, and that's what we want. =20 Random note: the thread_info object for inferior 2's thread is never freed. It is held (by refcount) by the restore_thread object in fetch_inferior_event, while the inferior's thread list gets cleared, in the exit event processing. When the refcount reaches 0 (when the restore_thread object is destroyed), there's nothing that actually deletes the thread_info object. And I think that nothing in GDB points to it anymore, so it leaks. I don't want to fix that in this patch, but thought it would be good to mention it, in case somebody has an idea for how to fix that. =20 Change-Id: Ibc7df543e2c46aad5f3b9250b28c3fb5912be4e8 Diff: --- gdb/infrun.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index acefd4f0bec..531d398d7b8 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4056,12 +4056,19 @@ reinstall_readline_callback_handler_cleanup () static void clean_up_just_stopped_threads_fsms (struct execution_control_state *ecs) { + /* The first clean_up call below assumes the event thread is the current + one. */ + if (ecs->event_thread !=3D nullptr) + gdb_assert (ecs->event_thread =3D=3D inferior_thread ()); + if (ecs->event_thread !=3D nullptr && ecs->event_thread->thread_fsm () !=3D nullptr) ecs->event_thread->thread_fsm ()->clean_up (ecs->event_thread); =20 if (!non_stop) { + scoped_restore_current_thread restore_thread; + for (thread_info *thr : all_non_exited_threads ()) { if (thr->thread_fsm () =3D=3D nullptr) @@ -4072,9 +4079,6 @@ clean_up_just_stopped_threads_fsms (struct execution_= control_state *ecs) switch_to_thread (thr); thr->thread_fsm ()->clean_up (thr); } - - if (ecs->event_thread !=3D nullptr) - switch_to_thread (ecs->event_thread); } }