gdb/testsuite Increasing timeout in names.exp test

[akondrat-amd]

Increasing the timeout in names.cpp to 90 seconds; the current 30‑second limit isn’t sufficient under emulation.

[lumachad]

Emulation timeouts should be controlled by the test invocation rather than in the testsuite itself. I don't think we need this change.

[akondrat-amd]

Emulation timeouts should be controlled by the test invocation rather than in the testsuite itself. I don't think we need this change.

This is hardcoded timeout in .cpp file, the application timesout before we hit all 9 breakpoints.

[lumachad]

Emulation timeouts should be controlled by the test invocation rather than in the testsuite itself. I don't think we need this change.

This is hardcoded timeout in .cpp file, the application timesout before we hit all 9 breakpoints.

Sorry, I've misinterpreted it. Could we not set these hardcoded values in case things go wrong and instead try to pass a value in or determine some reasonable number at runtime? In a normal system we can't afford to wait for 90 seconds before things time out.

I think it is unlikely we will run into issues, but still...

[lancesix]

Added one suggestion, feedbacks are welcome.

[lancesix]

Should that be dynamically set based on the testsuite's timeout so only one setting controls both?

diff --git a/gdb/testsuite/gdb.rocm/names.cpp b/gdb/testsuite/gdb.rocm/names.cpp
index f62dd295bf6..5ffa458c1da 100644
--- a/gdb/testsuite/gdb.rocm/names.cpp
+++ b/gdb/testsuite/gdb.rocm/names.cpp
@@ -209,7 +209,7 @@ int
 main (int argc, char **argv)
 {
   /* So that the GPU threads don't spin forever.  */
-  gdb_watchdog (30);
+  gdb_watchdog (TIMEOUT);
 
   if (argc != 2)
     {
diff --git a/gdb/testsuite/gdb.rocm/names.exp b/gdb/testsuite/gdb.rocm/names.exp
index ff734c32e97..d832a954b28 100644
--- a/gdb/testsuite/gdb.rocm/names.exp
+++ b/gdb/testsuite/gdb.rocm/names.exp
@@ -21,7 +21,8 @@ require allow_hipcc_tests
 
 standard_testfile .cpp
 
-if {[build_executable "failed to prepare" $testfile $srcfile {debug hip}]} {
+set options { "additional_flags=-DTIMEOUT=$timeout" debug hip }
+if {[build_executable "failed to prepare" $testfile $srcfile $options]} {
     return
 }

Do not have to be exactly TCL's timeout, but could be based on it (but I guess this value would be as good as any).

WDYT?

[lumachad]

Passer-by comment. I see these other tests also invoke the same:

gdb.rocm/mi-attach.cpp: gdb_watchdog (30);
gdb.rocm/detach-while-breakpoints-inserted.cpp: gdb_watchdog (30);
gdb.rocm/runtime-core.cpp: gdb_watchdog (30);
gdb.rocm/names.cpp: gdb_watchdog (30);
gdb.rocm/lane-info.cpp: gdb_watchdog (30);
gdb.rocm/device-interrupt.cpp: gdb_watchdog (10);

[akondrat-amd]

It will complicate manual compilation. I think, this timeout should be the worst possible case, it happens only if something goes wrong and the value doesn't affect the execution time. It's OK to wait an extra minute if something goes wrong.

[lancesix]

if that is a concern, the pattern could be to have

#ifndef TIMEOUT
#define TIMEOUT 30
#endif

[akondrat-amd]

The only problem is that we now have to figure out if we are running on the emulator in .exp script and adjust the TIMEOUT. It's getting too complicated for no reason, I guess.

[lancesix]

it is (as far as I know) already standard practice to bump the timeout using the site.exp file on environments where this is necessary. Such change would make it so that only local setting would also impact how long it takes before raising SIGALRM.

[lancesix]

and I guess, if one wants to run the test manually (outside of the test framework), it could make more sense to have

#ifdef TIMEOUT
  gdb_watchdog (TIMEOUT);
#endif

that way, manually building and running things would completely remove the timeout (useful when we are debugging GDB itself).

[akondrat-amd]

Good idea, I'll review the other tests and add necessary code.