Win: handle \\?\UNC\ prefix in realPathFromHandle (PR43204)

[hans]

Summary

After r361885, realPathFromHandle() ends up getting called on the working directory on each Clang invocation. This unveiled that the code didn't work for paths on network shares.

For example, if one maps the local dir c:\src\tmp to x:

net use x: \\localhost\c$\tmp

and run e.g. "clang -c foo.cc" in x:\, realPathFromHandle will get \\?\UNC\localhost\c$\src\tmp\ back from GetFinalPathNameByHandleW, and would strip off the initial \\?\ prefix, ending up with a path that doesn't work.

This patch makes the prefix stripping a little smarter to handle this case.

[jeroen.dobbelaere]

Hi Hans, I am not too familiar with all windows path naming variants. Did you really observe a '\\?UNC\xxxx' path ?

llvm/lib/Support/Windows/Path.inc

379 ↗

(On Diff #218675)

For my testcase, I observe a:

\\?\UNC\server\path
   ^-- extra \

So, the check should be:

if (CountChars >= 8 && ::memcmp(Data, L"\\\\?\\UNC\\", 16) == 0) {

Later on, CountChars and Data should be reduced by '6'.

[rnk]

As discussed, maybe we should try harder to revert whatever made us more dependent on this.

[hans]

In D67166#1657796, @jeroen.dobbelaere wrote:

Hi Hans, I am not too familiar with all windows path naming variants. Did you really observe a '\\?UNC\xxxx' path ?

You're right, I fat fingered it. Let me upload a new patch...

[hans]

In D67166#1658031, @rnk wrote:

As discussed, maybe we should try harder to revert whatever made us more dependent on this.

I think we should do both. Even if we revert to become less dependent on this function, I think we should also try to fix the UNC case which is obviously broken here.

[rnk]

lgtm

llvm/lib/Support/Windows/Path.inc

378 ↗

(On Diff #218748)

The mix of byte counts and character counts here is making me feel like this is too clever. You could do something like this:

template <size_t N>
static size_t wideStartsWith(const wchar_t *Str, const wchar_t (&Prefix)[N]) {
  return wcsncmp(Str, Prefix, N) == 0;
}

Alternatively it could take an ArrayRef<wchar_t> and use this memcmp pattern here.

[amccarth]

The \\?\ prefix tells the Windows API layer not to parse the path strings and just pass it along to the file system. Ramifications:

1. You can no longer use . or .. as a path component. Those are treated as special in the API layer rather than as part of the file system.
2. Many Windows APIs that take a path will accept / and treat it as \. If you use \\?\, the entire path must use \ throughout.
3. The prefix also lets you use long path names (> MAX_PATH), but only if you use them with the W versions of the APIs (e.g., CreateFileW rather than CreateFileA). [On Windows 10, there are other ways to get long path names, but I don't think that's relevant here.]

I suspect the desire to ever strip the prefix stems from some of these ramifications.

The decision to include or strip this prefix can have all sorts of impact that we may not be able to anticipate.

But I don't see anything wrong with the code in this patch.

llvm/trunk/lib/Support/Windows/Path.inc

 
 static std::error_code realPathFromHandle(HANDLE H,
                                           SmallVectorImpl<char> &RealPath) {
   RealPath.clear();
   SmallVector<wchar_t, MAX_PATH> Buffer;
   if (std::error_code EC = realPathFromHandle(H, Buffer))
     return EC;
 
-  const wchar_t *Data = Buffer.data();
+  // Strip the \\?\ prefix. We don't want it ending up in output, and such
+  // paths don't get canonicalized by file APIs.
+  wchar_t *Data = Buffer.data();
   DWORD CountChars = Buffer.size();
-  if (CountChars >= 4) {
-    if (0 == ::memcmp(Data, L"\\\\?\\", 8)) {
-      CountChars -= 4;
-      Data += 4;
-    }
-  }
+  if (CountChars >= 8 && ::memcmp(Data, L"\\\\?\\UNC\\", 16) == 0) {
+    // Convert \\?\UNC\foo\bar to \\foo\bar
+    CountChars -= 6;
+    Data += 6;
+    Data[0] = '\\';
+  } else if (CountChars >= 4 && ::memcmp(Data, L"\\\\?\\", 8) == 0) {
+    // Convert \\?\c:\foo to c:\foo
 
   // Convert the result from UTF-16 to UTF-8.
   return UTF16ToUTF8(Data, CountChars, RealPath);
 }
 
 std::error_code is_local(int FD, bool &Result) {
   SmallVector<wchar_t, 128> FinalPath;
   HANDLE Handle = reinterpret_cast<HANDLE>(_get_osfhandle(FD));