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

Tue Mar 23 16:03:08 PDT 2021

aganea added a comment.

In D99212#2645957 <https://reviews.llvm.org/D99212#2645957>, @amccarth wrote:

> I like the solution.  Has anyone actually tried it on Windows 7?  I ask because some of the observations noted in second hand descriptions of the problem didn't fully add up.  I think we understand the root problem, but I'm not 100% certain.

Yes, I've managed to repro the initial issue <https://github.com/rust-lang/rust/issues/81051> on a Windows 7 VM.
The problem is easily reproducible with this Python snippet: https://pastebin.com/v51m3uBU
Note the last two commands. `GetFinalPathNameByHandleW` fails on Windows 7 but not on Windows 10. If setting `SetFileInformationByHandle` with `fdi.DeleteFile = False` just before calling `GetFinalPathNameByHandleW`, things work as expected (even if we made several calls with `fdi.DeleteFile = True` before).

After patch, as suggedted by @rdwampler, I simply forced to always call `Disposition.DeleteFile = false` before doing anything else.

In the screenshot below, you can see what happens on Windows 7. The first three calls to `SetDispositionInformationFile` API are for a write to a local drive (corresponds to `llvm-ar r empty.a d:\bug.py`). The last two calls to `SetDispositionInformationFile` are for a write to a network drive (corresponds to the same command, but ran on Y: which is the mounted network drive)

F15971831: llvm-ar-win7.PNG <https://reviews.llvm.org/F15971831>

Please take another look to the updated patch.

================
Comment at: llvm/lib/Support/Windows/Path.inc:408
+  // 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.
----------------
rdwampler wrote:
> IIUC, realPathFromHandle() fails because FILE_DISPOSITION_INFO is already set to true, if so can you update the comment here to clarify. I think this would still fail if setDeleteDisposition() is called twice with Delete being true.
Changed as suggested. Please let me know if the wording isn't correct of it doesn't fully capture the issue.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99212/new/

https://reviews.llvm.org/D99212