[PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Pedro Alves]

There are a good number of testcases in the testsuite that use alarm()
as a watchdog that aborts the test if something goes wrong.

alarm()/SIG_ALRM do not exist on (native) Windows, so those tests fail
to compile there.

For example, testing with x86_64-w64-mingw32-gcc, we see:

 Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
 gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c: In function 'main':
 C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:17:3: error: implicit declaration of function 'alarm' [-Wimplicit-function-declaration]
    17 |   alarm (60);
       |   ^~~~~

While testing with a clang configured to default to
x86_64-pc-windows-msvc, which uses the C/C++ runtime headers from
Visual Studio and has no unistd.h, we get:

 Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
 gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:8:10: fatal error: 'unistd.h' file not found
     8 | #include <unistd.h>
       |          ^~~~~~~~~~

Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
defines a new gdb_watchdog function, which wraps alarm on Unix-like
systems, and uses a timer on Windows.

This patch adjusts gdb.base/attach.c as example of usage.  Testing
gdb.base/attach.exp with clang/x86_64-pc-windows-msvc required a
related portability tweak to can_spawn_for_attach, to not rely on
unistd.h on Windows.

gdb.rocm/mi-attach.cpp is another example adjusted, one which always
runs with clang configured as x86_64-pc-windows-msvc on Windows (via
hipcc).

Change-Id: I3b07bcb60de039d34888ef3494a5000de4471951
---
 gdb/testsuite/gdb.base/attach.c      |  4 +-
 gdb/testsuite/gdb.rocm/mi-attach.cpp |  4 +-
 gdb/testsuite/lib/gdb.exp            | 15 ++++-
 gdb/testsuite/lib/gdb_watchdog.h     | 83 ++++++++++++++++++++++++++++
 4 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/lib/gdb_watchdog.h

diff --git a/gdb/testsuite/gdb.base/attach.c b/gdb/testsuite/gdb.base/attach.c
index b3c54984012..41b304ef594 100644
--- a/gdb/testsuite/gdb.base/attach.c
+++ b/gdb/testsuite/gdb.base/attach.c
@@ -5,7 +5,7 @@
    exit unless/until gdb sets the variable to non-zero.)
    */
 #include <stdio.h>
-#include <unistd.h>
+#include "../lib/gdb_watchdog.h"
 
 int  bidule = 0;
 volatile int  should_exit = 0;
@@ -14,7 +14,7 @@ int main ()
 {
   int  local_i = 0;
 
-  alarm (60);
+  gdb_watchdog (60);
 
   while (! should_exit)
     {
diff --git a/gdb/testsuite/gdb.rocm/mi-attach.cpp b/gdb/testsuite/gdb.rocm/mi-attach.cpp
index da7659dc566..5752cdd10bb 100644
--- a/gdb/testsuite/gdb.rocm/mi-attach.cpp
+++ b/gdb/testsuite/gdb.rocm/mi-attach.cpp
@@ -15,8 +15,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include <unistd.h>
 #include <hip/hip_runtime.h>
+#include "../lib/gdb_watchdog.h"
 
 __global__ void
 kern ()
@@ -30,7 +30,7 @@ main ()
 {
   /* This program will run outside of GDB, make sure that if anything goes
      wrong it eventually gets killed.  */
-  alarm (30);
+  gdb_watchdog (30);
 
   kern<<<1, 1>>> ();
   return hipDeviceSynchronize () != hipSuccess;
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 98691dfce25..857d1d634c2 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6846,7 +6846,20 @@ gdb_caching_proc can_spawn_for_attach {} {
 
     set me "can_spawn_for_attach"
     set src {
-	#include <unistd.h>
+	#ifdef _WIN32
+	# include <windows.h>
+	#else
+	# include <unistd.h>
+	#endif
+
+	#ifdef _WIN32
+	unsigned
+	sleep (unsigned seconds)
+	{
+	    Sleep (seconds * 1000);
+	    return 0;
+	}
+	#endif
 
 	int
 	main (void)
diff --git a/gdb/testsuite/lib/gdb_watchdog.h b/gdb/testsuite/lib/gdb_watchdog.h
new file mode 100644
index 00000000000..e16f6b06f38
--- /dev/null
+++ b/gdb/testsuite/lib/gdb_watchdog.h
@@ -0,0 +1,83 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Set a watchdog that aborts the testcase after a timeout.  */
+
+#ifndef GDB_WATCHDOG_H
+#define GDB_WATCHDOG_H
+
+/* Forward define to make sure the definitions have the right
+   prototype, at least in C.  */
+static void gdb_watchdog (unsigned int seconds);
+
+static const char _gdb_watchdog_msg[] = "gdb_watchdog: timeout expired - aborting test\n";
+
+#ifdef _WIN32
+#include <windows.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+static VOID CALLBACK
+_gdb_watchdog_timer_routine (PVOID lpParam, BOOLEAN TimerOrWaitFired)
+{
+  fputs (_gdb_watchdog_msg, stderr);
+  fflush (stderr);
+  abort ();
+}
+
+static void
+gdb_watchdog (unsigned int seconds)
+{
+  HANDLE timer;
+  HANDLE timer_queue;
+
+  timer_queue = CreateTimerQueue ();
+  if (timer_queue == NULL)
+    return;
+
+  if (!CreateTimerQueueTimer (&timer, timer_queue,
+			      _gdb_watchdog_timer_routine, NULL,
+			      seconds * 1000, 0, 0))
+    {
+      /* Failed to create timer.  */
+      DeleteTimerQueue (timer_queue);
+    }
+}
+
+#else /* POSIX systems */
+
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+static void
+_gdb_sigalrm_handler (int signo)
+{
+  write (2, _gdb_watchdog_msg, sizeof (_gdb_watchdog_msg) - 1);
+  abort ();
+}
+
+static void
+gdb_watchdog (unsigned int seconds)
+{
+  signal (SIGALRM, _gdb_sigalrm_handler);
+  alarm (seconds);
+}
+
+#endif
+
+#endif /* GDB_WATCHDOG_H */

base-commit: f5b1b0288a97965bd71668b046fa35a85c4cea04
-- 
2.50.1

Re: [PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Kevin Buettner]

Hi Pedro,

On Fri,  8 Aug 2025 23:50:50 +0100
Pedro Alves <pedro@palves.net> wrote:

> There are a good number of testcases in the testsuite that use alarm()
> as a watchdog that aborts the test if something goes wrong.
> 
> alarm()/SIG_ALRM do not exist on (native) Windows, so those tests fail
> to compile there.
> 
> For example, testing with x86_64-w64-mingw32-gcc, we see:
> 
>  Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
>  gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c: In
> function 'main': C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:17:3:
> error: implicit declaration of function 'alarm'
> [-Wimplicit-function-declaration] 17 |   alarm (60); |   ^~~~~
> 
> While testing with a clang configured to default to
> x86_64-pc-windows-msvc, which uses the C/C++ runtime headers from
> Visual Studio and has no unistd.h, we get:
> 
>  Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
>  gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:8:10:
> fatal error: 'unistd.h' file not found 8 | #include <unistd.h>
>        |          ^~~~~~~~~~
> 
> Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
> defines a new gdb_watchdog function, which wraps alarm on Unix-like
> systems, and uses a timer on Windows.
> 
> This patch adjusts gdb.base/attach.c as example of usage.  Testing
> gdb.base/attach.exp with clang/x86_64-pc-windows-msvc required a
> related portability tweak to can_spawn_for_attach, to not rely on
> unistd.h on Windows.
> 
> gdb.rocm/mi-attach.cpp is another example adjusted, one which always
> runs with clang configured as x86_64-pc-windows-msvc on Windows (via
> hipcc).

This all sounds reasonble to me.

One possible nit, and it's *way* outside my wheelhouse, so feel free to
ignore it...

[...]
> +static VOID CALLBACK
> +_gdb_watchdog_timer_routine (PVOID lpParam, BOOLEAN TimerOrWaitFired)
> +{
> +  fputs (_gdb_watchdog_msg, stderr);
> +  fflush (stderr);

On POSIX, fputs and fflush are not async-signal-safe.  It's my
understanding that they aren't on WIN32 either, but I don't know
whether this is actually a signal handler.  In any case, my AI
assistant (Qwen3 Coder) says that something like this might
work:

    DWORD bytes_written;
    WriteFile(GetStdHandle(STD_ERROR_HANDLE), _gdb_watchdog_msg, 
	      sizeof(_gdb_watchdog_msg)-1, &bytes_written, NULL);

Regardless, the rest LGTM.  And even if fputs and fflush aren't
async-signal-safe in this context, I doubt that it matters much.

So...

Approved-by: Kevin Buettner <kevinb@redhat.com>

Re: [PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Pedro Alves]

On 2025-08-10 04:28, Kevin Buettner wrote:
> Hi Pedro,

Hi!

> 
> On Fri,  8 Aug 2025 23:50:50 +0100
> This all sounds reasonble to me.
> 
> One possible nit, and it's *way* outside my wheelhouse, so feel free to
> ignore it...
> 
> [...]
>> +static VOID CALLBACK
>> +_gdb_watchdog_timer_routine (PVOID lpParam, BOOLEAN TimerOrWaitFired)
>> +{
>> +  fputs (_gdb_watchdog_msg, stderr);
>> +  fflush (stderr);
> 
> On POSIX, fputs and fflush are not async-signal-safe.  It's my
> understanding that they aren't on WIN32 either, but I don't know
> whether this is actually a signal handler.

There isn't actually a concept of async-signal-safety on Windows.  Signal
handlers don't run on the mainline code's preempted stack, like happen on POSIX.
Instead, Windows runs the signal handler on a system-created thread, concurrently
with mainline code.

In this case, we don't have a signal handler, but it's basically the same thing.
The timer runs on a separate thread (Windows thread-pool thread in this case).

So the concern here would be the same concern with any two threads calling
into fputs/fflush at the same time.  The stdio routines are thread safe, in
that they serialize access to FILE* streams with a per-stream internal lock,
so simultaneous fputs calls to the same stream from different threads won’t
corrupt the stream's buffer.

I think that the only case where a call to fputs/fflush here could block
indefinitely, is if they end up blocking inside the internal WriteFile
they do, say, because they are writing to a full pipe and the reading
end isn't consuming input.  But for that scenario, going directly to
WriteFile won't help either, it will block the same way.

This is exactly the same for the write call on the POSIX side, BTW.

I think the only way to avoid this would be if we tried to open the
handle with FILE_FLAG_OVERLAPPED and do async writes, but that adds
complexity and the watchdog thread must pump the overlapped completion
or timeout logic.  Similarly, on the POSIX side we'd have to make sure to
do an async write.

Or we could just not print anything.

abort() is a CRT function too, and it flushes streams (hmm, maybe I don't need
the fflush after all), and if we were to avoid fputs/fflush to avoid locks, then
I'd think that we would want to avoid abort too.  Maybe call
ExitProcess or TerminateProcess directly?

However, I was thinking that using abort() could be nice for leaving it open the possibility
making Windows generate a minidump or for hooking Windows's crash reporting mechanism with
cygwin's dumper to generate an ELF core dump if we wanted, or something of the sort.
I suppose we could still have that if we terminate the process with something at
the Win32 level that triggers the same mechanisms, like maybe raise an uncaught exception?

Dunno.  fputs + abort just seems appealing to me for being simple, and abort() being the
same that we do on the POSIX side.

And in the end I think it's like you say -- I doubt it matters much.

> 
> Regardless, the rest LGTM.  And even if fputs and fflush aren't
> async-signal-safe in this context, I doubt that it matters much.
> 
> So...
> 
> Approved-by: Kevin Buettner <kevinb@redhat.com>
> 

Re: [PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Tom Tromey]

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

Pedro> Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
Pedro> defines a new gdb_watchdog function, which wraps alarm on Unix-like
Pedro> systems, and uses a timer on Windows.

It seems like this would need some special work to handle the remote
host testing case.

Pedro> +#include "../lib/gdb_watchdog.h"
 
... at least, I assume this isn't automatically copied to the remote.

Tom

Re: [PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Pedro Alves]

On 2025-08-12 15:41, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
> Pedro> defines a new gdb_watchdog function, which wraps alarm on Unix-like
> Pedro> systems, and uses a timer on Windows.
> 
> It seems like this would need some special work to handle the remote
> host testing case.
> 
> Pedro> +#include "../lib/gdb_watchdog.h"
>  
> ... at least, I assume this isn't automatically copied to the remote.
> 

Hmm.  Yeah, I forget it's also possible to compile on the host side.

I see that for example lib/attributes.h is handled with lappend_include_file
and include_file.  I'll take a better look.

[PATCH v2] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Pedro Alves]

On 2025-08-12 19:57, Pedro Alves wrote:
> On 2025-08-12 15:41, Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>>
>> Pedro> Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
>> Pedro> defines a new gdb_watchdog function, which wraps alarm on Unix-like
>> Pedro> systems, and uses a timer on Windows.
>>
>> It seems like this would need some special work to handle the remote
>> host testing case.
>>
>> Pedro> +#include "../lib/gdb_watchdog.h"
>>  
>> ... at least, I assume this isn't automatically copied to the remote.
>>
> 
> Hmm.  Yeah, I forget it's also possible to compile on the host side.
> 
> I see that for example lib/attributes.h is handled with lappend_include_file
> and include_file.  I'll take a better look.


So locally I have patches touching a lot more testcases, converting them to use
gdb_watchdog.  I adjusted them all locally to use lappend_include_file, but
I didn't like the result all that much.  It's annoying to have to list the
includes twice, once in the source files, and once in the .exp file.

Even worst is that in my local series, I have a patch that makes
gdb_watchdog.h include another header in testsuite/lib/, and the lappend_include_file
approach would mean that every .exp file that includes gdb_watchdog.h would need
to be updated to also lappend_include_file the header that gdb_watchdog.h includes.

So I thought about making this automatic, and came up with this:

 [PATCH] Automatically handle includes in testsuite/lib/
 https://inbox.sourceware.org/gdb-patches/20250813004100.2525141-1-pedro@palves.net/T/#u

Let me know what you think.

With that in place, the updated gdb_watchdog patch is below.  The only difference
is that we now include gdb_watchdog.h like so:

 #include "gdb_watchdog.h"

instead of:

 #include "../lib/gdb_watchdog.h"

From 338f5c87ccea20cfe65b122d227ee37499b04c6f Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 8 Aug 2025 23:50:04 +0100
Subject: [PATCH v2] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

There are a good number of testcases in the testsuite that use alarm()
as a watchdog that aborts the test if something goes wrong.

alarm()/SIG_ALRM do not exist on (native) Windows, so those tests fail
to compile there.

For example, testing with x86_64-w64-mingw32-gcc, we see:

 Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
 gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c: In function 'main':
 C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:17:3: error: implicit declaration of function 'alarm' [-Wimplicit-function-declaration]
    17 |   alarm (60);
       |   ^~~~~

While testing with a clang configured to default to
x86_64-pc-windows-msvc, which uses the C/C++ runtime headers from
Visual Studio and has no unistd.h, we get:

 Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
 gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:8:10: fatal error: 'unistd.h' file not found
     8 | #include <unistd.h>
       |          ^~~~~~~~~~

Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
defines a new gdb_watchdog function, which wraps alarm on Unix-like
systems, and uses a timer on Windows.

This patch adjusts gdb.base/attach.c as example of usage.  Testing
gdb.base/attach.exp with clang/x86_64-pc-windows-msvc required a
related portability tweak to can_spawn_for_attach, to not rely on
unistd.h on Windows.

gdb.rocm/mi-attach.cpp is another example adjusted, one which always
runs with clang configured as x86_64-pc-windows-msvc on Windows (via
hipcc).

Change-Id: I3b07bcb60de039d34888ef3494a5000de4471951
---
 gdb/testsuite/gdb.base/attach.c      |  4 +-
 gdb/testsuite/gdb.rocm/mi-attach.cpp |  4 +-
 gdb/testsuite/lib/gdb.exp            | 15 +++++-
 gdb/testsuite/lib/gdb_watchdog.h     | 78 ++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/lib/gdb_watchdog.h

diff --git a/gdb/testsuite/gdb.base/attach.c b/gdb/testsuite/gdb.base/attach.c
index b3c54984012..5133dd07e55 100644
--- a/gdb/testsuite/gdb.base/attach.c
+++ b/gdb/testsuite/gdb.base/attach.c
@@ -5,7 +5,7 @@
    exit unless/until gdb sets the variable to non-zero.)
    */
 #include <stdio.h>
-#include <unistd.h>
+#include "gdb_watchdog.h"
 
 int  bidule = 0;
 volatile int  should_exit = 0;
@@ -14,7 +14,7 @@ int main ()
 {
   int  local_i = 0;
 
-  alarm (60);
+  gdb_watchdog (60);
 
   while (! should_exit)
     {
diff --git a/gdb/testsuite/gdb.rocm/mi-attach.cpp b/gdb/testsuite/gdb.rocm/mi-attach.cpp
index da7659dc566..441d460146a 100644
--- a/gdb/testsuite/gdb.rocm/mi-attach.cpp
+++ b/gdb/testsuite/gdb.rocm/mi-attach.cpp
@@ -15,8 +15,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include <unistd.h>
 #include <hip/hip_runtime.h>
+#include "gdb_watchdog.h"
 
 __global__ void
 kern ()
@@ -30,7 +30,7 @@ main ()
 {
   /* This program will run outside of GDB, make sure that if anything goes
      wrong it eventually gets killed.  */
-  alarm (30);
+  gdb_watchdog (30);
 
   kern<<<1, 1>>> ();
   return hipDeviceSynchronize () != hipSuccess;
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0361f10b9a6..d989314c28d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6849,7 +6849,20 @@ gdb_caching_proc can_spawn_for_attach {} {
 
     set me "can_spawn_for_attach"
     set src {
-	#include <unistd.h>
+	#ifdef _WIN32
+	# include <windows.h>
+	#else
+	# include <unistd.h>
+	#endif
+
+	#ifdef _WIN32
+	unsigned
+	sleep (unsigned seconds)
+	{
+	    Sleep (seconds * 1000);
+	    return 0;
+	}
+	#endif
 
 	int
 	main (void)
diff --git a/gdb/testsuite/lib/gdb_watchdog.h b/gdb/testsuite/lib/gdb_watchdog.h
new file mode 100644
index 00000000000..459b958836e
--- /dev/null
+++ b/gdb/testsuite/lib/gdb_watchdog.h
@@ -0,0 +1,78 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Set a watchdog that aborts the testcase after a timeout.  */
+
+#ifndef GDB_WATCHDOG_H
+#define GDB_WATCHDOG_H
+
+/* Forward define to make sure the definitions have the right
+   prototype, at least in C.  */
+static void gdb_watchdog (unsigned int seconds);
+
+static const char _gdb_watchdog_msg[]
+  = "gdb_watchdog: timeout expired - aborting test\n";
+
+#ifdef _WIN32
+#include <windows.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+static VOID CALLBACK
+_gdb_watchdog_timer_routine (PVOID lpParam, BOOLEAN TimerOrWaitFired)
+{
+  fputs (_gdb_watchdog_msg, stderr);
+  abort ();
+}
+
+static void
+gdb_watchdog (unsigned int seconds)
+{
+  HANDLE timer;
+
+  if (!CreateTimerQueueTimer (&timer, NULL,
+			      _gdb_watchdog_timer_routine, NULL,
+			      seconds * 1000, 0, 0))
+    {
+      /* Failed to create timer.  */
+      DeleteTimerQueue (timer_queue);
+    }
+}
+
+#else /* POSIX systems */
+
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+static void
+_gdb_sigalrm_handler (int signo)
+{
+  write (2, _gdb_watchdog_msg, sizeof (_gdb_watchdog_msg) - 1);
+  abort ();
+}
+
+static void
+gdb_watchdog (unsigned int seconds)
+{
+  signal (SIGALRM, _gdb_sigalrm_handler);
+  alarm (seconds);
+}
+
+#endif
+
+#endif /* GDB_WATCHDOG_H */

base-commit: f5b1b0288a97965bd71668b046fa35a85c4cea04
prerequisite-patch-id: 9b579e19acdb2121d8232cafc1bb63ef881684eb
-- 
2.50.1

Re: [PATCH v2] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

[Pedro Alves]

On 2025-08-13 01:51, Pedro Alves wrote:
> On 2025-08-12 19:57, Pedro Alves wrote:
>> On 2025-08-12 15:41, Tom Tromey wrote:
>>>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>>>
>>> Pedro> Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
>>> Pedro> defines a new gdb_watchdog function, which wraps alarm on Unix-like
>>> Pedro> systems, and uses a timer on Windows.
>>>
>>> It seems like this would need some special work to handle the remote
>>> host testing case.
>>>
>>> Pedro> +#include "../lib/gdb_watchdog.h"
>>>  
>>> ... at least, I assume this isn't automatically copied to the remote.
>>>
>>
>> Hmm.  Yeah, I forget it's also possible to compile on the host side.
>>
>> I see that for example lib/attributes.h is handled with lappend_include_file
>> and include_file.  I'll take a better look.
> 
> 
> So locally I have patches touching a lot more testcases, converting them to use
> gdb_watchdog.  I adjusted them all locally to use lappend_include_file, but
> I didn't like the result all that much.  It's annoying to have to list the
> includes twice, once in the source files, and once in the .exp file.
> 
> Even worst is that in my local series, I have a patch that makes
> gdb_watchdog.h include another header in testsuite/lib/, and the lappend_include_file
> approach would mean that every .exp file that includes gdb_watchdog.h would need
> to be updated to also lappend_include_file the header that gdb_watchdog.h includes.
> 
> So I thought about making this automatic, and came up with this:
> 
>  [PATCH] Automatically handle includes in testsuite/lib/
>  https://inbox.sourceware.org/gdb-patches/20250813004100.2525141-1-pedro@palves.net/T/#u
> 
> Let me know what you think.
> 
> With that in place, the updated gdb_watchdog patch is below.  The only difference
> is that we now include gdb_watchdog.h like so:
> 
>  #include "gdb_watchdog.h"
> 
> instead of:
> 
>  #include "../lib/gdb_watchdog.h"
> 

FYI, I've now pushed this.  But after pushing I noticed that this v2 patch email doesn't appear
to have reached the mailing list?  I can't find it in the archives, and have gotten a copy
via the list.  Odd.

Here's what I pushed.  The v2 version that I sent before had an incomplete change in the
Windows version of gdb_watchdog.  I had noticed that it's not necessary to create
a timer queue, there's a default one, and switched to doing that in v2 (passing NULL as
timer queue to CreateTimerQueueTimer), but had forgotten to delete the DeleteTimerQueue
line.

In this version, I'm simply aborting immediately if creating the timer fails.  I don't
expect that that will ever be reached, and the idea of aborting is that if it ever is
reached, it's better to see the problem immediately so we can better understand how it
can happen, instead of silently failing to have a watchdog in place.

I have some follow up patches making use of gdb_watchdog in other tests.

Pedro Alves

From e771abf8ff88a5d98b8960a7ea49708b9ec42521 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 8 Aug 2025 23:50:04 +0100
Subject: [PATCH] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)

There are a good number of testcases in the testsuite that use alarm()
as a watchdog that aborts the test if something goes wrong.

alarm()/SIG_ALRM do not exist on (native) Windows, so those tests fail
to compile there.

For example, testing with x86_64-w64-mingw32-gcc, we see:

 Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
 gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c: In function 'main':
 C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:17:3: error: implicit declaration of function 'alarm' [-Wimplicit-function-declaration]
    17 |   alarm (60);
       |   ^~~~~

While testing with a clang configured to default to
x86_64-pc-windows-msvc, which uses the C/C++ runtime headers from
Visual Studio and has no unistd.h, we get:

 Running /c/rocgdb/src/gdb/testsuite/gdb.base/attach.exp ...
 gdb compile failed, C:/rocgdb/src/gdb/testsuite/gdb.base/attach.c:8:10: fatal error: 'unistd.h' file not found
     8 | #include <unistd.h>
       |          ^~~~~~~~~~

Handle this by adding a new testsuite/lib/gdb_watchdog.h header that
defines a new gdb_watchdog function, which wraps alarm on Unix-like
systems, and uses a timer on Windows.

This patch adjusts gdb.base/attach.c as example of usage.  Testing
gdb.base/attach.exp with clang/x86_64-pc-windows-msvc required a
related portability tweak to can_spawn_for_attach, to not rely on
unistd.h on Windows.

gdb.rocm/mi-attach.cpp is another example adjusted, one which always
runs with clang configured as x86_64-pc-windows-msvc on Windows (via
hipcc).

Approved-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I3b07bcb60de039d34888ef3494a5000de4471951
---
 gdb/testsuite/gdb.base/attach.c      |  4 +-
 gdb/testsuite/gdb.rocm/mi-attach.cpp |  4 +-
 gdb/testsuite/lib/gdb.exp            | 15 +++++-
 gdb/testsuite/lib/gdb_watchdog.h     | 75 ++++++++++++++++++++++++++++
 4 files changed, 93 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/lib/gdb_watchdog.h

diff --git a/gdb/testsuite/gdb.base/attach.c b/gdb/testsuite/gdb.base/attach.c
index b3c54984012..5133dd07e55 100644
--- a/gdb/testsuite/gdb.base/attach.c
+++ b/gdb/testsuite/gdb.base/attach.c
@@ -5,7 +5,7 @@
    exit unless/until gdb sets the variable to non-zero.)
    */
 #include <stdio.h>
-#include <unistd.h>
+#include "gdb_watchdog.h"
 
 int  bidule = 0;
 volatile int  should_exit = 0;
@@ -14,7 +14,7 @@ int main ()
 {
   int  local_i = 0;
 
-  alarm (60);
+  gdb_watchdog (60);
 
   while (! should_exit)
     {
diff --git a/gdb/testsuite/gdb.rocm/mi-attach.cpp b/gdb/testsuite/gdb.rocm/mi-attach.cpp
index da7659dc566..441d460146a 100644
--- a/gdb/testsuite/gdb.rocm/mi-attach.cpp
+++ b/gdb/testsuite/gdb.rocm/mi-attach.cpp
@@ -15,8 +15,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include <unistd.h>
 #include <hip/hip_runtime.h>
+#include "gdb_watchdog.h"
 
 __global__ void
 kern ()
@@ -30,7 +30,7 @@ main ()
 {
   /* This program will run outside of GDB, make sure that if anything goes
      wrong it eventually gets killed.  */
-  alarm (30);
+  gdb_watchdog (30);
 
   kern<<<1, 1>>> ();
   return hipDeviceSynchronize () != hipSuccess;
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0361f10b9a6..d989314c28d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6849,7 +6849,20 @@ gdb_caching_proc can_spawn_for_attach {} {
 
     set me "can_spawn_for_attach"
     set src {
-	#include <unistd.h>
+	#ifdef _WIN32
+	# include <windows.h>
+	#else
+	# include <unistd.h>
+	#endif
+
+	#ifdef _WIN32
+	unsigned
+	sleep (unsigned seconds)
+	{
+	    Sleep (seconds * 1000);
+	    return 0;
+	}
+	#endif
 
 	int
 	main (void)
diff --git a/gdb/testsuite/lib/gdb_watchdog.h b/gdb/testsuite/lib/gdb_watchdog.h
new file mode 100644
index 00000000000..15d63e76198
--- /dev/null
+++ b/gdb/testsuite/lib/gdb_watchdog.h
@@ -0,0 +1,75 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Set a watchdog that aborts the testcase after a timeout.  */
+
+#ifndef GDB_WATCHDOG_H
+#define GDB_WATCHDOG_H
+
+/* Forward declaration to make sure the definitions have the right
+   prototype, at least in C.  */
+static void gdb_watchdog (unsigned int seconds);
+
+static const char _gdb_watchdog_msg[]
+  = "gdb_watchdog: timeout expired - aborting test\n";
+
+#ifdef _WIN32
+#include <windows.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+static VOID CALLBACK
+_gdb_watchdog_timer_routine (PVOID lpParam, BOOLEAN TimerOrWaitFired)
+{
+  fputs (_gdb_watchdog_msg, stderr);
+  abort ();
+}
+
+static void
+gdb_watchdog (unsigned int seconds)
+{
+  HANDLE timer;
+
+  if (!CreateTimerQueueTimer (&timer, NULL,
+			      _gdb_watchdog_timer_routine, NULL,
+			      seconds * 1000, 0, 0))
+    abort ();
+}
+
+#else /* POSIX systems */
+
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+static void
+_gdb_sigalrm_handler (int signo)
+{
+  write (2, _gdb_watchdog_msg, sizeof (_gdb_watchdog_msg) - 1);
+  abort ();
+}
+
+static void
+gdb_watchdog (unsigned int seconds)
+{
+  signal (SIGALRM, _gdb_sigalrm_handler);
+  alarm (seconds);
+}
+
+#endif
+
+#endif /* GDB_WATCHDOG_H */

base-commit: 3214cb0ce58245516d35cd93887f0d3bc14b5ebf
-- 
2.50.1