Re: [PATCH 2/2] [gdb/testsuite] Fix host_file_normalize_mingw

[Pedro Alves <pedro@palves.net>]

Hi Tom,

On 2025-08-27 14:14, Tom de Vries wrote:

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index b7961eb30dc..4b0249db70b 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2381,9 +2381,9 @@ proc host_file_normalize_mingw {filename unix_to_win} {
>  	if {[string equal -length $mount_len $unix_filename $filename]} {
>  	    if {[string length $filename] == $mount_len} {
>  		return "$win_filename/"
> -	    } elseif {[string index $filename $mount_len] eq "/"} {
> +	    } elseif {[string index $filename [expr $mount_len - 1]] eq "/"} {
>  		set rest [string range $filename $mount_len end]
> -		return "$win_filename$rest"
> +		return "$win_filename/$rest"
>  	    }
>  	}
>      }


Unfortunately, this breaks things for me.
I test on a directory under c:/, and the testsuite broke very visibly.  :-)

For example, the patch makes us mishandle this:

 set f3 "/c/foo/bar"
 set f4 "C:/foo/bar"
 gdb_assert { [host_file_normalize_mingw $f3 $unix_to_win] == $f4 }

Instead of turning $f3 into:
   C:/foo/bar
it turns it into:
   C:/msys64/c/foo/bar

Notice that the "/" mount point is the only one that ends in "/".  This
is even if you try to create one explicitly with a trailing /.  On MSYS2:

 $ mount c:/foo /foo/
 mount: warning - /foo/ does not exist.
 $ mount
 C:/foo on /foo type ntfs (binary,user)
 ...

So I think we need to instead special case the "/" mount point.

And then... while playing with this, I noticed I had done something strange with this case:

  	    if {[string length $filename] == $mount_len} {
  		return "$win_filename/"

The intent was to append the slash when the mount is a drive letter, like 'cygpath -ma' does:

 $ cygpath -ma /c
 C:/

Other cases do not get a trailing slash:

 $ cygpath -ma /c/foo
 C:/foo

I think this is because on Windows, every drive letter has a current directory, and really "C:" means
"current directory of drive letter C:", not "root of C:".  Resolving it to "C:/" makes it unambiguous.

However, I mishandled that, and made the code append the slash whenever the input filename matches
a mount exactly, any mount.

And then I noticed that TCL's "file normalize" on Linux always removes the trailing slash, and since
host_file_normalize is an abstraction for it, I thought host_file_normalize_mingw should do the same.
Likewise for duplicate slashes, "file normalize" gets rid of them.

I was going to give you a suggestion for the root mount fix, and handle the slashes things in a follow
up patch to yours, but fixing the slashes details changes how to address the issue with handling the
root dir, so I ended up doing it all in one patch.  See below.

I changed how one adds tests to your new testcase to a way that I think is much easier to write, read,
and makes gdb.sum results also have some sense.  It also prints some info to gdb.log that helped me
understand what went wrong when a test FAILs.

The "/foo" test I added has the same effect as yours, just with a shorter and simpler filename, that
fits better with the other new tests I added.  What matters is that this case is a file/directory under
a mount point, and I made the testcase test that for every mount point (root, drive letter, other).

I absolutely love the idea of a testcase for this.  It helps so much!  Thanks a lot for starting that.

WDYT?  Does this version work for you?  I smoke tested it here with a few of the testcases that required
tweaking in the patch that added host_file_normalize like gdb.base/source-dir.exp and gdb.base/fullname.exp
and they still passed.

From de4e47323d9e7df1b09cd635bfce902b00b3371b Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 29 Aug 2025 20:05:48 +0100
Subject: [PATCH] Fix host_file_normalize_mingw

Change-Id: I852a8662f0cb8b0ee4e683e9b157618cf6955477
---
 .../gdb.testsuite/mount-point-map.exp         | 33 ++++++++++++++++---
 gdb/testsuite/lib/gdb.exp                     | 26 +++++++++++++--
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.testsuite/mount-point-map.exp b/gdb/testsuite/gdb.testsuite/mount-point-map.exp
index d4c8bfdeff6..9e462bb63e9 100644
--- a/gdb/testsuite/gdb.testsuite/mount-point-map.exp
+++ b/gdb/testsuite/gdb.testsuite/mount-point-map.exp
@@ -18,9 +18,32 @@ set unix_to_win {
     / C:/msys64
 }
 
-set d1 "/"
-set d2 "C:/msys64/"
-gdb_assert { [host_file_normalize_mingw $d1 $unix_to_win] == $d2 }
+# Test that FROM is normalized to TO.
 
-set d3 "C:/msys64"
-gdb_assert { [host_file_normalize_mingw $d3 $unix_to_win] == $d3 }
+proc test {from to} {
+    set got [host_file_normalize_mingw $from $::unix_to_win]
+    verbose -log "input:    $from"
+    verbose -log "expected: $to"
+    verbose -log "got:      $got"
+    gdb_assert {$got == $to} $from
+}
+
+# Drive letters always get a '/' suffix, other Windows file names do
+# not.
+test "/" "C:/msys64"
+test "/c" "C:/"
+test "/bin" "C:/msys64/usr/bin"
+
+# A file name that already starts with a drive letter.
+test "C:/msys64" "C:/msys64"
+
+# A subdir/subfile under each mount.
+test "/foo" "C:/msys64/foo"
+test "/c/foo" "C:/foo"
+test "/bin/foo" "C:/msys64/usr/bin/foo"
+
+# Test slash normalization.
+test "//" "C:/msys64"
+test "/c///foo//bar//" "C:/foo/bar"
+# We don't currently handle UNC paths.
+test "//server///" "C:/msys64/server"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 18e449ad8a0..17eb00a10f3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2374,12 +2374,34 @@ proc host_file_normalize_mingw {filename unix_to_win} {
 	return $filename
     }
 
+    # Collapse all repeated forward slashes.
+    set filename [regsub -all {//+} $filename {/}]
+
+    # Strip trailing slash, except for root.
+    if {$filename ne "/" && [string match */ $filename]} {
+	set filename [string range $filename 0 end-1]
+    }
+
     foreach {unix_filename win_filename} $unix_to_win {
 	set mount_len [string length $unix_filename]
 	if {[string equal -length $mount_len $unix_filename $filename]} {
-	    if {[string length $filename] == $mount_len} {
-		return "$win_filename/"
+	    if {$unix_filename eq "/"} {
+		if {$filename eq "/"} {
+		    return "$win_filename"
+		} else {
+		    return "$win_filename$filename"
+		}
+	    } elseif {[string length $filename] == $mount_len} {
+		# Like "cygpath -ma" if the file name resolves to a
+		# drive letter, append a slash, to make it unambiguous
+		# that we resolved to the root of the drive and not
+		# the drive's current directory.
+		if {[string match {[A-Za-z]:} $win_filename]} {
+		    return "$win_filename/"
+		} else {
+		    return "$win_filename"
+		}
 	    } elseif {[string index $filename $mount_len] eq "/"} {
 		set rest [string range $filename $mount_len end]
 		return "$win_filename$rest"

-- 
2.50.1