[PATCH] gdb/testsuite: Always close '-x hip' with '-x none' (was: Re: [PATCH v2] gdb/testsuite: replace hipcc with amdclang++ as the HIP compiler)

[Pedro Alves <pedro@palves.net>]

Hi!

On 2026-08-06 12:29, Sarang Patrange wrote:
> +    # amdclang++ infers the input language from the file extension and
> +    # compiles .cpp as C++, not HIP.  Since the HIP testcases are named
> +    # .cpp, explicitly tag each input: "-x hip" before C/C++/HIP source
> +    # files, and "-x none" before everything else (object files,
> +    # archives, shared libraries), so the compiler does not try to
> +    # parse those as HIP source.  This lets a single amdclang++
> +    # invocation correctly handle a mix of sources and non-source
> +    # inputs (e.g. when set_unbuffered_mode.o is appended via ldflags
> +    # later in the command line).

Turns out this part about set_unbuffered_mode.o isn't working as intended.  
Testing on Windows still revealed breakage.  See patch below for proposed fix.

> +    if {[lsearch -exact $options hip] != -1 && !$getting_compiler_info} {
> +	set new_source {}
> +	foreach src $source {
> +	    set ext [string tolower [file extension $src]]
> +	    if {$ext in {".c" ".cc" ".cp" ".cxx" ".cpp" ".c++" ".hip"}} {
> +		lappend new_source "-x" "hip" $src
> +	    } else {
> +		lappend new_source "-x" "none" $src
> +	    }
> +	}
> +	set source $new_source
> +    }
> +

From 1b251b00f8dc2bfeff83871739bb2a2d14f8470c Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 31 Jul 2026 14:54:15 +0100
Subject: [PATCH] gdb/testsuite: Always close '-x hip' with '-x none'

When compiling a HIP testcase on Windows, we see:

...
  Executing on host: clang++  --hip-link -mllvm=-amdgpu-spill-cfi-saved-regs -Wno-unused-command-line-argument -x hip /c/msys2/home/alves/gdb/build-testsuite-windows-msvc/temp/4415/device_enumerator.cpp  -fdiagnostics-color=never -Wno-unknown-warning-option -Wno-deprecated-declarations -w -g  /c/msys2/home/alves/gdb/build-testsuite-windows-msvc/set_unbuffered_mode_saved.o /c/msys2/home/alves/gdb/build-testsuite-windows-msvc/windows-manifest.o    -o /c/msys2/home/alves/gdb/build-testsuite-windows-msvc/temp/4415/device_enumerator.x    (timeout = 300)
  ...
  C:/msys2/home/alves/gdb/build-testsuite-windows-msvc/set_unbuffered_mode_saved.o:1:2: error: source file is not valid UTF-8
      1 | d<86><U+001A><U+0000><95><F0><9A>jx<U+0008><U+0000><U+0000>D<U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000>.text<U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000>N<U+0000><U+0000><U+0000>$<U+0004><U+0000><U+0000>r<U+0004><U+0000><U+0000><U+0000><U+0000><U+0000><U+0000><U+0004><U+0000><U
...

When compiling HIP programs, gdb_compile prepends "-x hip" before .cpp
sources, and "-x none" before .o sources.  That logic however does not
consider what happens when you also include a .o file on the
compilation/link line via ldflags, which is what we do with
set_unbuffered_mode_saved.o and windows-manifest.o.  Since the last
"-x" option passed on the clang command line wins, if that last -x
option for the last "source" file was "-x hip", then that makes clang
treat set_unbuffered_mode_saved.o as a HIP source file, which fails in
the way you see above.  I.e., what we end up with is, simplified:

  clang++ \
    -x hip \
    .../device_enumerator.cpp \               << affected by -x hip
    .../set_unbuffered_mode_saved.o \         << affected by -x hip
    .../windows-manifest.o \                  << affected by -x hip
    -o .../temp/4415

... and this command tells clang to treat all of device_enumerator.cpp
set_unbuffered_mode_saved.o and windows-manifest.o as HIP source
files.

I think prepending "-x none" in the set_unbuffered_mode_saved.o path
(like ldflags="-x none set_unbuffered_mode_saved.o") is the wrong
thing to do, as that really means we'd be leaking gdb_compile's "-x
hip/none" concern to other places.

I think instead the correct thing is to make sure that gdb_compile
always "closes" a previous "-x hip" with "-x none".  "-x none" resets
the language detection back to the default.  That's what this commit
does.

So we get instead:

  clang++ \
    -x hip \
    .../device_enumerator.cpp \               << affected by -x hip
    -x none \                                 << new
    .../set_unbuffered_mode_saved.o \         << under -x none
    .../windows-manifest.o \                  << under -x none
    -o .../temp/4415

... which compiles & links fine.

Change-Id: I49ab369ebba1d638f61a73673b25ffc35305875b
---
 gdb/testsuite/lib/gdb.exp | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index b66e501d08d..1c02b0fa918 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7129,17 +7129,24 @@ proc gdb_compile {source dest type options} {
     # archives, shared libraries), so the compiler does not try to
     # parse those as HIP source.  This lets a single amdclang++
     # invocation correctly handle a mix of sources and non-source
-    # inputs (e.g. when set_unbuffered_mode.o is appended via ldflags
-    # later in the command line).
+    # inputs.
     if {[lsearch -exact $options hip] != -1 && !$getting_compiler_info} {
 	set new_source {}
+	set x_lang ""
 	foreach src $source {
 	    set ext [string tolower [file extension $src]]
 	    if {$ext in {".c" ".cc" ".cp" ".cxx" ".cpp" ".c++" ".hip"}} {
-		lappend new_source "-x" "hip" $src
+		set x_lang "hip"
 	    } else {
-		lappend new_source "-x" "none" $src
+		set x_lang "none"
 	    }
+	    lappend new_source "-x" $x_lang $src
+	}
+	# Make sure that if we last opened "-x hip", we close it off
+	# with "-x none", in case something adds some input file via
+	# ldflags (e.g. set_unbuffered_mode.o).
+	if {$x_lang != "" && $x_lang != "none"} {
+	    lappend new_source "-x" "none"
 	}
 	set source $new_source
     }

base-commit: 97feaffb829a5fcf404d2d093062758123e8795f
-- 
2.54.0