From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sonic311-30.consmr.mail.ir2.yahoo.com (sonic311-30.consmr.mail.ir2.yahoo.com [77.238.176.162]) by sourceware.org (Postfix) with ESMTPS id 19D913858D38 for ; Fri, 12 Apr 2024 10:56:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 19D913858D38 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=yahoo.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=yahoo.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 19D913858D38 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=77.238.176.162 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712919413; cv=none; b=YJMTz527Bas+oKrWvBZyRrsM6XMWAUPiU2s++qQKzgeGxUqIOGJ/cqeb18zMUo30thHd0BH4uF+x4JI5/0y4pCkFEUxN2SQAIaKCXum3/09ZBQRwld5oXKreY3vvztrTFJfuC9MxUWW9bW1Fq8PPEXOmbJGBFKVdA9nGLk7b7Lc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712919413; c=relaxed/simple; bh=NHFZRgmIuE5uFMHid8ptEbPijY1xXedfStTVEJkdZmc=; h=DKIM-Signature:Date:From:To:Message-ID:Subject:MIME-Version; b=tvUPCpYL9aR6JMMbgvnllFe5Tklh3tjF21OQXga03J09GbjuHCcco0ygs9a5tIGMwx6ia0j3OMFmI+mrJ5RSNjb8wrRzUG4nin/IjXnYEWGdtR/Rlaup3jD5RPC4Fq4MZ8e36s8FoysrmE3QErKdXce3k7YhDosf0MdLRGAxHIo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.de; s=s2048; t=1712919409; bh=NHFZRgmIuE5uFMHid8ptEbPijY1xXedfStTVEJkdZmc=; h=Date:From:To:In-Reply-To:References:Subject:From:Subject:Reply-To; b=iGqngEyTugZ+WDAyeq9GuJqzIcI+bWbrSVAfwl7OJ9qyIlxmEL8KS+yFCKu73qgCrpxTo+M3Oi+wVZ30/Rgo0cerLQVFi6WwYoJIS2fNmLe0cfO7v9voKoRJKulAqvJlqezizW6vgXY/QM2UyevFp6LpfSzifge7vJ+CdyBHufercmsd34Uog04YfCXH5b97ypJr/cf1/wnSz3htkW2BOnzd2BK3bmG6Qr1WYi1FA+SjlhDjans6lJ3y3xvm8BWt/rkvEqBcHdfbfshnTeKzxLujzQ8Ot0DiwyAU5mDP2soEKPcg7vuzujMknu5lHmn2rltcNdk9KtzDdMefja8FnQ== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1712919409; bh=iBxNPTgSp+q/2xo6rNzWaYVUypJhzbBv0napi/FP5z3=; h=X-Sonic-MF:Date:From:To:Subject:From:Subject; b=Xj1yw8V676L/dpEs/OuPPwgaqlyeRXYr0cT38g16dM0HuAbK4wtoVdehu0MzraoUR769Yxyiw3bv/8e5N6pB22kzK7zeiKrZD2kDCAGIq+VouOQQ6J0ui64eo4ixZATgTv1TCbR6WiIQZzgJf/VcsJnqCOiw8NlLhOpBR9LvWylPGDZJgeNu0Vie4q1vJNa4FVsz7/gmkEerMMsNg1T1ux7VyXmFQjoGFOHaq0gyO12QhvHZ1/Cl+zaHtESz+ZaeRJzyD6bleZmVH3sOF+bAsoGKig+9HR7ZBhxGFxmDfi7/DYjfQbLo4eZr7X7KraBQb/nJv6G4DwzPnI/iAdzuYg== X-YMail-OSG: bGqSkzUVM1l8azH4GpsED6Mdg6vEEmgw3U6EiqXMZULxmNEOn1CIvwQuXN4TG6y 2oZumnojNPBSdCFr9oIUW2.djDbIv20xnFg90H1TG.WDi4ScS78Yod7xvbM6q8Ogh_94lXE2NQSH 1mTC1kgP4F1N7FkgM7w01d0hyTpn._xbzZfaX_ETtX0VCe0vNbcXA1fgJCxs2Fnz1jFv0hrpvh.f kiabCGTeGKfvRkfOK6Ab2rRUqi7az7NvySZPj75dttR6bPzRMwJPaI9u3eY7XomSWnBHCC6qV7.O BNNvM3rk835IropodOVDiYfi98y7gTj563ih7De482e4fYKUYukTBAmIzIHLZEuIPBaM766l2Ltt 8LDo3.HaMUppsk4Sj2PfwgZN6VjTtVPY_pmoqeCwekn9vZBFkIy7XZaRr_tihpAyYim2n18WS93M iC7IQciA4lzQKEggQGX5sWFUrtIw.tzFynzfyauYEa6hLDHyoDF9rD5KpMBwvd0wH.RIxp6Wi5Fe or2IWr.r_ghOtRBM5UJwawhllAcQ_oFKfdpPE2DskNJF6UQbidZ5necA7tkPQ0o4pFxmu.7JibD7 7AwGJoZqD8lO.mnNEYDac3lVEx5FXf2mT4oN1_wIyyLGI1uKLBbHH9geWOAYaxy9qlEtkMgW0KDm 2xrdNVSB1CKRLJ4ywVCiR0URt3lwaUHnQojAvB2erzINTeqqBWAJ4rqzV7m6xHLkIDglHSbiopoN kmHL.FoXwkHFJnccLOkS68_SpF3xnk_DagXgcsbB6qWcZelwMTjCE2B9P946mD8705_lj0W4uZGV S6XgVrzBW0WszAo3qSpSWzLSzw5EVY_ak.EawDqR0cE97JcOlHXyRjUWiJcuNq0.ezdEqUJWOwWB R4705uC895_js1wiPC1dAR29sXF44ogBzUE7m05MeWV0iGEBRsFwPS1kMM2.l3yaJCegydi.mDzg f6m4mt1GBVVcfurUZYRwX_YdQrsl3WblIYAkWwyGPg62_Zad_dhu7yZsbxtXqyIkCcA4SBOVid1X z9AEO52DjwrqGhUYXxeFJEIRBSTUTSlxreQIf2xMzN3xEYtHSxkCZZjGlOzbUVClKR44wb17q0Ex 6gAhCN2YbchwzKNBhbKnfPjxUHCowsW6EroTFPAFyL6ET5_x6X4MY3BzTKdPmj.VE7XFrq3lARoJ srBY_31P6SLnSYw2UWQrnsH7xwYMaBltziYOauGmPs20SmdcHUqBSiJpfHfOPVNIvh4f9FYG_sAp qvNBUNhvTKsTtXrsR0UjoERbknMIy52gDdz9_vNtLnFyl_PX3JccdWQZIkelC.4Z8wjK0sfrxcxZ XTDpBGGHMmkpwSrfSRdh2536bqj3Aov2jlTT8ad89S7MUv4MaWdrqLRsBqIs7n6V7Bggb_wDyFHo fhQxMyt2WgqfY7oU010QaXFs9JdUKOuNFTSzN930j3mBwojebPi.McimcozFbvbwHpa9YHNWHIPz _i47EJzzWVnbrRcVDmTtkgWu8Un8.pLR02oWPKYZ2p5wczSbUWTw_RkLIkuRMfPxB5TAzd26KE0s 9EMW8lEX7GBs4KYitzCy9ZiSqsgtHfl2SaKETb2T_p3f5TvzqnkgGPKj2zhstmKmWDgHsdHBKS_g cPUlbXITQy0YiojZ4vOcspU65bVwOBSBG1p.J5MGipGGE5DhP4hXnXAd6t.jGl0G8FoC2macDPcD KcnIg09Ps4P6JJWTP993ZJqq53dGLJlYZ8JB_wpoANLiQNMddl07boI0Vok6DZGl6NWGIQbF6bhc JgiOL1ebeMdywARPNCFiaVwYphKrDTEQiy_BpWIdxsbMx4vdsOV6uAkReWV9brUeMV6yzpgtEyLI _wFxBE4vZrGaguiCKAWByxdm1IrmeESOEcVsko4YVuEumxc1fbUeMO3HlZ4yqW98yHGt5y6DC70q EuD_S7mTeVTsQ7BNQs1Wunm5vdjOcNoBNBfgqEGlkUXhB4F2dW99vybWbvaIqoLhuJKhY5qp4SoH PPAbPsrlfKFCHT5Q93vd0H2zPiL8mZSwe2mgc3f2emcGNtQtmlY.rAAGclCtBa9RpnCMeZm.afwi h3s9iqT.ndvW8wiTZG5uq36vfSv.jaNpaiQchWdq8Fh7_xvfmHkr.nDnjpecDDnINubjIdPU_0NU h4l6D_Alpx526zJuGep1nCinNM2y9j_6qh7BBTg7c6fSvZmyh4sIwcU1PPEwjJk6KYWUTC41R1KY Xvo__HzgSF5BrjkVPhGisx9oaQc4t3x0NO71fyD0CasOrRPDKXi3PH9G5JqUGV3ERoYEC X-Sonic-MF: X-Sonic-ID: 0d5d90be-a25e-47fc-8c4f-da2235ba52d9 Received: from sonic.gate.mail.ne1.yahoo.com by sonic311.consmr.mail.ir2.yahoo.com with HTTP; Fri, 12 Apr 2024 10:56:49 +0000 Date: Fri, 12 Apr 2024 10:56:45 +0000 (UTC) From: Hannes Domani To: "gdb-patches@sourceware.org" , Pedro Alves Message-ID: <178084166.332205.1712919405648@mail.yahoo.com> In-Reply-To: <20240411200356.270360-1-pedro@palves.net> References: <20240411200356.270360-1-pedro@palves.net> Subject: Re: [PATCH] gdb/Windows: Fix detach while running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Mailer: WebService/1.1.22205 YMailNorrin X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Am Donnerstag, 11. April 2024 um 22:04:34 MESZ hat Pedro Alves Folgendes geschrieben: > While testing a WIP Cygwin GDB that supports non-stop, I noticed that > gdb.threads/attach-non-stop.exp exposes that this: > > (gdb) attach PID& > ... > (gdb) detach > > ... hangs. > > And it turns out that it hangs in all-stop as well.=C2=A0 This commits > fixes that. > > After "attach &", the target is set running, we've called > ContinueDebugEvent and the process_thread thread is waiting for > WaitForDebugEvent events.=C2=A0 It is the equivalent of "attach; c&". > > In windows_nat_target::detach, the first thing we do is > unconditionally call windows_continue (for ContinueDebugEvent), which > blocks in do_synchronously, until the process_thread sees an event out > of WaitForDebugEvent.=C2=A0 Unless the inferior happens to run into a > breakpoint, etc., then this hangs indefinitely. > > If we've already called ContinueDebugEvent earlier, then we shouldn't > be calling it again in ::detach. > > Still in windows_nat_target::detach, we have an interesting issue that > ends up being the bulk of the patch -- only the process_thread thread > can call DebugActiveProcessStop, but if it is blocked in > WaitForDebugEvent, we need to somehow force it to break out of it. > The only way to do that, is to force the inferior to do something that > causes WaitForDebugEvent to return some event. > > This patch uses CreateRemoteThread to do it, which results in > WaitForDebugEvent reporting CREATE_THREAD_DEBUG_EVENT.=C2=A0 We then > terminate the injected thread before it has a chance to run any > userspace code. > > Note that Win32 functions like DebugBreakProcess and > GenerateConsoleCtrlEvent would also inject a new thread in the > inferior.=C2=A0 I first used DebugBreakProcess, but that is actually more > complicated to use, because we'd have to be sure to consume the > breakpoint event before detaching, otherwise the inferior would likely > die due a breakpoint exception being raised with no debugger around to > intercept it. > > See the new break_out_process_thread method. > > So the fix has two parts: > > - Keep track of whether we've called ContinueDebugEvent and the >=C2=A0=C2=A0 process_thread thread is waiting for events, or whether >=C2=A0=C2=A0 WaitForDebugEvent already returned an event. > > - In windows_nat_target::detach, if the process_thread thread is >=C2=A0=C2=A0 waiting for events, unblock out of its WaitForDebugEvent, bef= ore >=C2=A0=C2=A0 proceeding with the actual detach. > > New test included.=C2=A0 Passes cleanly on GNU/Linux native and gdbserver= , > and also passes cleanly on Cygwin, with the fix.=C2=A0 Before the fix, it > would hang and fail with a timeout. I tried this, and it works for me too for Windows native. Tested-By: Hannes Domani I have 2 comments below. > Change-Id: Ifb91c58c08af1a9bcbafecedc93dfce001040905 > --- > .../gdb.threads/detach-while-running.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 |=C2=A0 30 ++++ > .../gdb.threads/detach-while-running.exp=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |= =C2=A0 77 ++++++++++ > gdb/windows-nat.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 131 ++++++++++++++++-- > 3 files changed, 230 insertions(+), 8 deletions(-) > create mode 100644 gdb/testsuite/gdb.threads/detach-while-running.c > create mode 100644 gdb/testsuite/gdb.threads/detach-while-running.exp > > diff --git a/gdb/testsuite/gdb.threads/detach-while-running.c b/gdb/tests= uite/gdb.threads/detach-while-running.c > new file mode 100644 > index 00000000000..19cc3b5761c > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/detach-while-running.c > @@ -0,0 +1,30 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > +=C2=A0 Copyright 2024 Free Software Foundation, Inc. > + > +=C2=A0 This program is free software; you can redistribute it and/or mod= ify > +=C2=A0 it under the terms of the GNU General Public License as published= by > +=C2=A0 the Free Software Foundation; either version 3 of the License, or > +=C2=A0 (at your option) any later version. > + > +=C2=A0 This program is distributed in the hope that it will be useful, > +=C2=A0 but WITHOUT ANY WARRANTY; without even the implied warranty of > +=C2=A0 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.=C2=A0 See th= e > +=C2=A0 GNU General Public License for more details. > + > +=C2=A0 You should have received a copy of the GNU General Public License > +=C2=A0 along with this program.=C2=A0 If not, see .=C2=A0 */ > + > +#include > +#include > + > +int > +main (int argc, char **argv) > +{ > +=C2=A0 alarm (30); > + > +=C2=A0 while (1) > +=C2=A0=C2=A0=C2=A0 sleep (1); > + > +=C2=A0 return 0; > +} The test passes for me, but I had to #ifdef the alarm() function out, since it doesn't exist on Windows. > diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c > index b123a66ef0f..7e571f281cb 100644 > --- a/gdb/windows-nat.c > +++ b/gdb/windows-nat.c > @@ -2072,20 +2089,118 @@ windows_nat_target::attach (const char *args, in= t from_tty) >=C2=A0=C2=A0 target_terminal::ours (); > } > > +void > +windows_nat_target::break_out_process_thread (bool &process_alive) > +{ > +=C2=A0 /* This is called when the process_thread thread is blocked in > +=C2=A0=C2=A0=C2=A0 WaitForDebugEvent (unless it already returned some ev= ent we > +=C2=A0=C2=A0=C2=A0 haven't consumed yet), and we need to unblock it so t= hat we can > +=C2=A0=C2=A0=C2=A0 have it call DebugActiveProcessStop. > + > +=C2=A0=C2=A0=C2=A0 To make WaitForDebugEvent return, we need to force so= me event in > +=C2=A0=C2=A0=C2=A0 the inferior.=C2=A0 Any method that lets us do that (= without > +=C2=A0=C2=A0=C2=A0 disturbing the other threads), injects a new thread i= n the > +=C2=A0=C2=A0=C2=A0 inferior. > + > +=C2=A0=C2=A0=C2=A0 We don't use DebugBreakProcess for this, because that= injects a > +=C2=A0=C2=A0=C2=A0 thread that ends up executing a breakpoint instructio= n.=C2=A0 We can't > +=C2=A0=C2=A0=C2=A0 let the injected thread hit that breakpoint _after_ w= e've > +=C2=A0=C2=A0=C2=A0 detached.=C2=A0 Consuming events until we see a break= point trap isn't > +=C2=A0=C2=A0=C2=A0 100% reliable, because we can't distinguish it from s= ome other > +=C2=A0=C2=A0=C2=A0 thread itself deciding to call int3 while we're detac= hing, unless > +=C2=A0=C2=A0=C2=A0 we temporarily suspend all threads.=C2=A0 It's just a= lot of > +=C2=A0=C2=A0=C2=A0 complication, and there's an easier way. > + > +=C2=A0=C2=A0=C2=A0 Important observation: the thread creation event for = the newly > +=C2=A0=C2=A0=C2=A0 injected thread is sufficient to unblock WaitForDebug= Event. > + > +=C2=A0=C2=A0=C2=A0 Instead of DebugBreakProcess, we can instead use > +=C2=A0=C2=A0=C2=A0 CreateRemoteThread to control the code that the injec= ted thread > +=C2=A0=C2=A0=C2=A0 runs ourselves.=C2=A0 We could consider pointing the = injected thread > +=C2=A0=C2=A0=C2=A0 at some side-effect-free Win32 function as entry poin= t.=C2=A0 However, > +=C2=A0=C2=A0=C2=A0 finding the address of such a function requires havin= g at least > +=C2=A0=C2=A0=C2=A0 minimal symbols loaded for ntdll.dll.=C2=A0 Having a = way that avoids > +=C2=A0=C2=A0=C2=A0 that is better, so that detach always works correctly= even when > +=C2=A0=C2=A0=C2=A0 we don't have any symbols loaded. > + > +=C2=A0=C2=A0=C2=A0 So what we do is inject a thread that doesn't actuall= y run ANY > +=C2=A0=C2=A0=C2=A0 userspace code, because we force-terminate it as soon= as we see > +=C2=A0=C2=A0=C2=A0 its corresponding thread creation event.=C2=A0 Create= RemoteThread > +=C2=A0=C2=A0=C2=A0 gives us the new thread's ID, which we can match with= the thread > +=C2=A0=C2=A0=C2=A0 associated with the CREATE_THREAD_DEBUG_EVENT event.= =C2=A0 */ > + > +=C2=A0 DWORD injected_thread_id =3D 0; > +=C2=A0 HANDLE injected_thread_handle > +=C2=A0=C2=A0=C2=A0 =3D CreateRemoteThread (windows_process.handle, NULL, > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 0, (LPTHREAD_START_ROUTINE) 0, > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 NULL, 0, &injected_thread_id); > + > +=C2=A0 process_alive =3D true; > + > +=C2=A0 /* At this point, the user has declared that they want to detach,= so > +=C2=A0=C2=A0=C2=A0 any event that happens from this point on should be f= orwarded to > +=C2=A0=C2=A0=C2=A0 the inferior.=C2=A0 */ > + > +=C2=A0 for (;;) > +=C2=A0=C2=A0=C2=A0 { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 DEBUG_EVENT current_event; > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wait_for_debug_event_main_thread (¤= t_event); > + > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (current_event.dwDebugEventCode =3D=3D= EXIT_PROCESS_DEBUG_EVENT) > +=C2=A0=C2=A0=C2=A0 { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 DEBUG_EVENTS ("got EXIT_PROCESS_DEBUG_EVE= NT"); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 process_alive =3D false; > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 break; Not sure how likely it is to get here, but injected_thread_handle leaks in this case. > +=C2=A0=C2=A0=C2=A0 } > + > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (current_event.dwDebugEventCode =3D=3D= CREATE_THREAD_DEBUG_EVENT > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 && current_event.dwThreadId =3D=3D inject= ed_thread_id) > +=C2=A0=C2=A0=C2=A0 { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 DEBUG_EVENTS ("got CREATE_THREAD_DEBUG_EV= ENT for injected thread"); > + > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Terminate the injected thread, so it d= oesn't run any code > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 at all.=C2=A0 All we wanted w= as some event, and > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CREATE_THREAD_DEBUG_EVENT is = sufficient.=C2=A0 */ > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CHECK (TerminateThread (injected_thread_h= andle, 0)); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CHECK (CloseHandle (injected_thread_handl= e)); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 break; > +=C2=A0=C2=A0=C2=A0 } > + > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 DEBUG_EVENTS ("got unrelated event, code = %u", > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 curre= nt_event.dwDebugEventCode); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 windows_continue (DBG_CONTINUE, -1, 0); > +=C2=A0=C2=A0=C2=A0 } > +} Hannes