]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Fix host_file_normalize_mingw
authorPedro Alves <pedro@palves.net>
Fri, 29 Aug 2025 19:05:48 +0000 (20:05 +0100)
committerPedro Alves <pedro@palves.net>
Tue, 2 Sep 2025 12:01:53 +0000 (13:01 +0100)
Tom de Vries ran the testsuite on msys2-ucrt64 with mount point map:
...
  /bin C:/msys64/usr/bin
  /c   C:
  /    C:/msys64
...
and ran into the problem that host_file_normalize didn't translate:
...
  /home/user/gdb/build/gdb/testsuite/temp/n/x
...
into:
...
  C:/msys64/home/user/gdb/build/gdb/testsuite/temp/n/x
...

The problem is that host_file_normalize_mingw mishandles a
file/directory under the root mount point.  A simpler reproducer is
"/foo".  If we add that as a test to
gdb.testsuite/mount-point-map.exp, we see:

 input:    /foo
 expected: C:/msys64/foo/
 got:      /foo
 FAIL: gdb.testsuite/mount-point-map.exp: /foo

For a mount point that ends in /, this line in
host_file_normalize_mingw:

  } elseif {[string index $filename $mount_len] eq "/"} {

... is always false, because the character at $mount_len is the one
_after_ the slash.

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 fix this by special casing the "/" mount point.

And then... while playing with fixing 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 in a63213cd374d ('MSYS2+MinGW testing: Unix
<-> Windows path conversion').  The original version of that patch
when I posted it to the mailing list only supported drive mounts,
which turned out incorrect, and then I generalized it to work with all
mount points before it was merged.  In the process, I inadvertently
made the code append the slash whenever the input filename matches a
mount exactly, any mount.

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

Fix all this in host_file_normalize_mingw, and add corresponding tests
to gdb.testsuite/mount-point-map.exp.

I smoke tested this 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 pass.

Tom ran gdb.testsuite/mount-point-map.exp on both x86_64-linux and
msys2-ucrt64, and it passed in both cases.

Change-Id: I852a8662f0cb8b0ee4e683e9b157618cf6955477

gdb/testsuite/gdb.testsuite/mount-point-map.exp
gdb/testsuite/lib/gdb.exp

index e36f9f0893b9f828e21f2aa5e7dc49783bb9915b..9e462bb63e965ab448016c2b73ed5998b7ec1e07 100644 (file)
@@ -28,6 +28,22 @@ proc test {from to} {
     gdb_assert {$got == $to} $from
 }
 
-test "/" "C:/msys64/"
+# 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"
index 1f59284bfb19aec6407771d5332531c64dbe34c5..ab4506a44558201603b26b28e0139602aa28a93c 100644 (file)
@@ -2376,11 +2376,33 @@ 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"