Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: Tom Tromey <tom@tromey.com>
Cc: gdb-patches@sourceware.org
Subject: [PATCH v2] testsuite: Introduce gdb_watchdog (avoid unistd.h/alarm)
Date: Wed, 13 Aug 2025 01:51:43 +0100	[thread overview]
Message-ID: <f82ad86e-a769-47f0-b8ec-a37674eb68f1@palves.net> (raw)
In-Reply-To: <2e78248c-2838-4655-9420-600c4f3e1868@palves.net>

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


  reply	other threads:[~2025-08-13  0:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-08 22:50 [PATCH] " Pedro Alves
2025-08-10  3:28 ` Kevin Buettner
2025-08-11 11:48   ` Pedro Alves
2025-08-12 14:41 ` Tom Tromey
2025-08-12 18:57   ` Pedro Alves
2025-08-13  0:51     ` Pedro Alves [this message]
2025-08-22 19:08       ` [PATCH v2] " Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f82ad86e-a769-47f0-b8ec-a37674eb68f1@palves.net \
    --to=pedro@palves.net \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox