[PATCH] D99212: [Support] Fix Windows 7 path handling

[Alexandre Ganea via Phabricator via llvm-commits]

aganea created this revision.
aganea added reviewers: rdwampler, rnk, jhenderson, amccarth.
Herald added subscribers: dexonsmith, JDevlieghere, hiraditya.
aganea requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

As reported here: https://bugs.llvm.org/show_bug.cgi?id=48378#c0 and here: https://github.com/rust-lang/rust/issues/81051 since rG79657e2339b58bc01fe1b85a448bb073d57d90bb <https://reviews.llvm.org/rG79657e2339b58bc01fe1b85a448bb073d57d90bb>, some LLVM programs such as llvm-ar don't work properly on Windows 7.

The issue is shown in the snippet by Oleksandr Prodan: https://pastebin.com/v51m3uBU

In essence, once the 'Delete' flag has been set on a file, the file path can't be queried anymore with `GetFinalPathNameByHandleW`. This however works on Windows 10, `GetFinalPathNameByHandleW` returns sucessfully.

With this current patch, we simply prevent setting the flag on network files, but do not prevent from clearning it. All tests pass fine on Windows 7 & Windows 10. At the moment, we cannot specifically add a test coverage for this, since it requres mounting a network drive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99212

Files:
  llvm/lib/Support/Windows/Path.inc


Index: llvm/lib/Support/Windows/Path.inc
===================================================================
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -402,18 +402,23 @@
 }
 
 static std::error_code setDeleteDisposition(HANDLE Handle, bool Delete) {
-  // First, check if the file is on a network (non-local) drive. If so, don't
-  // set DeleteFile to true, since it prevents opening the file for writes.
-  SmallVector<wchar_t, 128> FinalPath;
-  if (std::error_code EC = realPathFromHandle(Handle, FinalPath))
-    return EC;
+  // First check if the file is on network (non-local) drive; don't ask for
+  // deletion in that case, since it prevents opening the file for writes.
+  // When Delete is false skip the check, since on Windows 7 the function
+  // realPathFromHandle() below would fail. In that case, network files would
+  // also get the 'false' flag, but that is fine.
+  if (Delete) {
+    SmallVector<wchar_t, 128> FinalPath;
+    if (std::error_code EC = realPathFromHandle(Handle, FinalPath))
+      return EC;
 
-  bool IsLocal;
-  if (std::error_code EC = is_local_internal(FinalPath, IsLocal))
-    return EC;
+    bool IsLocal;
+    if (std::error_code EC = is_local_internal(FinalPath, IsLocal))
+      return EC;
 
-  if (!IsLocal)
-    return std::error_code();
+    if (!IsLocal)
+      return std::error_code();
+  }
 
   // The file is on a local drive, set the DeleteFile to true.
   FILE_DISPOSITION_INFO Disposition;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99212.332769.patch
Type: text/x-patch
Size: 1489 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210323/d9277dfe/attachment.bin>