New testcase gdb.threads/leader-exit-attach.exp (PR threads/8153)

  On 2024-03-25 19:36, Pedro Alves wrote:
> On 2024-03-23 06:39, Eli Zaretskii wrote:
>>> From: Pedro Alves <pedro@palves.net>
>>> Date: Fri, 22 Mar 2024 19:30:30 +0000
>>>
>>> While working on Windows non-stop mode, I managed to introduce a bug
>>> that led to fake_create_process being called.  That then resulted in
>>> GDB crashes later on, because fake_create_process added a thread with
>>> an incorrect ptid for this target.  It is putting dwThreadId in the
>>> tid field of the ptid instead of on the lwp field.  This is fixed by
>>> this patch.
>>>
>>> I do however wonder why nobody has seen it this long.
>>
>> AFAIU, to actually see the bug, one would need to attach GDB to a
>> process whose main thread has exited, is that true?  If so, I'm not
>> surprised this bug was not reported: it's unusual for the main thread
>> to exit without shutting down the process, and the need to attach to
>> such a process (as opposed to having it run from GDB to begin with)
>> makes that even more rare.  And finally, not every bug is reported by
>> the first person who sees it the first time, right?
> 
> Yes, that could be the reason.  But it could also be because the brokenness with the
> Windows debug API that Chris was seeing only happens on Windows versions we no
> longer claim support for (i.e., earlier than Windows XP).
> 
> Anyhow, the patch is pretty obvious on its own, so I went ahead and merged it
> without that blurb in the commit log, like below.
> 
> I also wrote a testcase that exercises the scenario in question.  I'll post
> that next.


Here's said testcase.  Only two decades between original fix and testcase,
not too bad.  :-)

While writing this, I stumbled on server/31554, and I filed it in bugzilla,
and added a kfail here, to avoid falling deeper down the rabbit hole.

I also filed server/31555 for the can't-attach-to-zombie-task on Linux, and marked
it as kfail instead of xfail as there may be a workaround for that.  (Attach to all
the process's threads anyhow. We'd not get an exit status for the final process exit,
but I think we could live without it).

From ff9c3b19e5c876ed8e5cb5f45f1e3a9873010991 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Mon, 25 Mar 2024 15:17:02 +0000
Subject: [PATCH] New testcase gdb.threads/leader-exit-attach.exp (PR
 threads/8153)

Add a new testcase for exercising attaching to a process after its
main thread has exited.

This is not possible on Linux, the kernel does not allow attaching to
a zombie task, so the test is kfailed there.  It is possible however
on Windows at least, and was the scenario addressed by the Windows
backend fix in
https://sourceware.org/legacy-ml/gdb-patches/2003-12/msg00479.html,
nowadays PR threads/8153, back in 2003.

Passes cleanly on Cygwin.
KFAILed on GNU/Linux native and gdbserver.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=8153
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31554
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31555
Change-Id: Ib554f92f68c965bb4603cdf2aadb55ca45ded53b
---
 .../gdb.threads/leader-exit-attach.exp        | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 gdb/testsuite/gdb.threads/leader-exit-attach.exp


base-commit: ccf3148e3133f016a8e1484e85e5e4d8c271c4f0
  

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Here's said testcase.  Only two decades between original fix and testcase,
Pedro> not too bad.  :-)

Does this mean that the phony process stuff is needed and so it should
be ported to gdbserver as well?

Maybe we need another new bug.

Tom
  

On 2024-03-26 15:26, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> Here's said testcase.  Only two decades between original fix and testcase,
> Pedro> not too bad.  :-)
> 
> Does this mean that the phony process stuff is needed and so it should
> be ported to gdbserver as well?

At least on Windows 10, it isn't needed.  The jury is still out on whether it is needed on any Windows version we claim to support (Windows XP and up).
I was hoping we could drop all that hacky stuff from gdb instead of adding it to gdbserver.

I can't run Cygwin tests against gdbserver atm for the some reason I can't explain, but if I try it manually, GDBserver attaches to the process just fine.

If I run the new test on native Cygwin with this:

 --- c/gdb/windows-nat.c
 +++ w/gdb/windows-nat.c
 @@ -1360,6 +1360,8 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
  DWORD
  windows_nat_target::fake_create_process ()
  {
 +  gdb_assert (0);
 +

... it still passes.

I only tripped on the bad ptid in the phony process code path because I had a bug in the non-stop series that happened to result in that path being incorrectly taken.

Pedro Alves

> 
> Maybe we need another new bug.
> 
> Tom
  

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> At least on Windows 10, it isn't needed.  The jury is still out
Pedro> on whether it is needed on any Windows version we claim to
Pedro> support (Windows XP and up).

Ok, thanks, I see.

Pedro> I was hoping we could drop all that hacky stuff from gdb instead
Pedro> of adding it to gdbserver.

Yeah, me too.

Tom
  

On 2024-03-25 19:57, Pedro Alves wrote:

> Here's said testcase.  Only two decades between original fix and testcase,
> not too bad.  :-)

I've merged this one now.