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

[Kevin Buettner <kevinb@redhat.com>]

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>