[lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

[MrTrillian]

Summary

Running lit tests on Windows can fail because its use of os.path.realpath expands substitute drives, which are used to keep paths short and avoid hitting MAX_PATH limitations.

Changes lit logic to:

• Use os.path.abspath on Windows, where MAX_PATH is a concern that we can work around using substitute drives, which os.path.realpath would resolve.
• Use os.path.realpath on Unix, where the current directory always has symlinks resolved, so it is impossible to preserve symlinks in the presence of relative paths, and so we must make sure that all code paths use real paths.

Also updates clang's FileManager::getCanonicalName and ExtractAPI code to avoid resolving substitute drives (ie resolving to a path under a different root).

How tested: built with -DLLVM_ENABLE_PROJECTS=clang and built check-all on both Windows

[MrTrillian]

How do I take action on the test failure? It runs without issues on my machine:

tristan@nuc-on-wood:~/project-llvm$ python3 build/bin/llvm-lit -sv --filter test.toy build/tools/mlir
/test/

Testing Time: 0.55s
  Excluded: 1636
  Passed  :    1

And I see that the failure is a timeout so I wonder if it's reliable?

Exit Code: -9
Timeout: Reached timeout of 60 seconds

[MrTrillian]

All premerge build failures seem like flukes.

• x64 windows failed 1/3 times
• x64 debian failed 2/3 times with a timeout (passes locally)
• libcxx seems to be failing for everyone: https://buildkite.com/llvm-project/libcxx-ci

[MrTrillian]

@rnk , I would appreciate your review on this since you helped with the previous iteration.

[aaron.ballman]

Adding a few more folks who are interested in lit changes to try to get the review unstuck.

FWIW, I worry about the subtlety of the > change because it's not entirely clear to me when I'd need to use %>t in a test. I worry code reviewers will miss this sort of thing and we'll only find out there's an issue when the test fails for someone with a problematic path. Is there a rule of thumb we should be following for its use?

In D154130#4480946, @MrTrillian wrote:

All premerge build failures seem like flukes.

• x64 windows failed 1/3 times
• x64 debian failed 2/3 times with a timeout (passes locally)
• libcxx seems to be failing for everyone: https://buildkite.com/llvm-project/libcxx-ci

FWIW, I agree.

[MrTrillian]

In D154130#4481673, @aaron.ballman wrote:

Adding a few more folks who are interested in lit changes to try to get the review unstuck.

FWIW, I worry about the subtlety of the > change because it's not entirely clear to me when I'd need to use %>t in a test. I worry code reviewers will miss this sort of thing and we'll only find out there's an issue when the test fails for someone with a problematic path. Is there a rule of thumb we should be following for its use?

Thanks for the extra reviewers!

95% of the %>t are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it. In fact I should rename PREFIX_EXPANDED to MODULEMAP_PREFIX so it would be much clearer.

There are three cases where I didn't expect to need the expanded paths: relative_include.m, case-insensitive-include-win.c and module-header-mismatches.m. There may be a way to change the clang implementation so it does not need to have expanded paths, but that felt like a different investigation.

I'm happy to consider alternative syntaxes to %>t too.

[aaron.ballman]

In D154130#4481763, @MrTrillian wrote:

In D154130#4481673, @aaron.ballman wrote:

Adding a few more folks who are interested in lit changes to try to get the review unstuck.

FWIW, I worry about the subtlety of the > change because it's not entirely clear to me when I'd need to use %>t in a test. I worry code reviewers will miss this sort of thing and we'll only find out there's an issue when the test fails for someone with a problematic path. Is there a rule of thumb we should be following for its use?

Thanks for the extra reviewers!

95% of the %>t are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it. In fact I should rename PREFIX_EXPANDED to MODULEMAP_PREFIX so it would be much clearer.

Okay, if this is mostly specific to module maps, that may resolve most of my concern (we don't add a lot of new tests there, so it's less of a burden for reviewers). CC @ChuanqiXu @Bigcheese @tahonermann to see if there are modules concerns with this.

There are three cases where I didn't expect to need the expanded paths: relative_include.m, case-insensitive-include-win.c and module-header-mismatches.m. There may be a way to change the clang implementation so it does not need to have expanded paths, but that felt like a different investigation.

I'm happy to consider alternative syntaxes to %>t too.

I think the syntax is reasonable enough (at least, I don't have arguably better suggestions), it was more just the "how will I know when to use it?" concerns.

[ChuanqiXu]

I don't write module map a lot neither. But I am curious where is the definition for the term %>/t? It is indeed a little bit odd at the first look.

[tahonermann]

95% of the %>t are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it.

Can you link to evidence that this behavior is by design? It isn't obvious to me why modulemap files would demand different behavior; especially since that would exacerbate MAX_PATH problems.

I'm not fond of the safe_abs_path name; "safe" doesn't communicate anything and the implementation is no more safe than os.path.abspath or os.path.realpath. Suggested alternatives:

• short_abs_path
• shortest_abs_path
• abs_path_no_subst_drive
• abs_path_preserve_drive

[jdenny]

If the %> feature is going to remain (depending on, for example, the answer to @tahonermann's question about modulemap), please:

1. Add it to the lit documentation at https://llvm.org/docs/TestingGuide.html#substitutions and https://llvm.org/docs/CommandGuide/lit.html#substitutions. (Aside: I wish someone would reconcile those lists.)
2. Consider whether there's a clearer syntax. For example, we already have %{/t:regex_replacement}. What about %{t:real} and %{/t:real}?

[jdenny]

3. Extend lit's own test suite to cover it.

[MrTrillian]

In D154130#4486898, @tahonermann wrote:

95% of the %>t are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it.

Can you link to evidence that this behavior is by design? It isn't obvious to me why modulemap files would demand different behavior; especially since that would exacerbate MAX_PATH problems.

I agree with you. The original rationale seems to be here:
https://github.com/llvm/llvm-project/blob/926f3759ec62a8f170e76a60316cc0bdd9dd2ec9/clang/lib/Lex/HeaderSearch.cpp#L257

cpp
    // To avoid false-negatives, we form as canonical a path as we can, and map
    // to lower-case in case we're on a case-insensitive file system.

And this change has more context: https://reviews.llvm.org/D134923 by @benlangmuir

I'm not fond of the safe_abs_path name; "safe" doesn't communicate anything and the implementation is no more safe than os.path.abspath or os.path.realpath. Suggested alternatives:

• short_abs_path
• shortest_abs_path
• abs_path_no_subst_drive
• abs_path_preserve_drive

Thanks for the name suggestions, those are better indeed! I will update with the last one.

[MrTrillian]

Add reviewers from https://reviews.llvm.org/D134923

[MrTrillian]

• Renamed safe_abs_path to abs_path_preserve_drive @tahonermann
• Renamed %>t and %>/t to %{t:real} and %{/t:real} @tahonermann
• Renamed PREFIX_EXPANDED to SUBMODULE_PREFIX @aaron.ballman
• Documented %{t:real} and %{/t:real} syntaxes @jdenny

[MrTrillian]

In D154130#4487292, @jdenny wrote:

3. Extend lit's own test suite to cover it.

I submitted an update with your suggestions #1 and #2 but this #3 is much more difficult because of the nature of %{t:real}. I'm not sure I can have a source/target path that includes symlinks in a way that allows me to test this, and even more so with substitute drives since we can't know which drive letters are unallocated.

[jdenny]

In D154130#4502036, @MrTrillian wrote:

In D154130#4487292, @jdenny wrote:

3. Extend lit's own test suite to cover it.

I submitted an update with your suggestions #1 and #2 but this #3 is much more difficult because of the nature of %{t:real}.

Thanks for doing that. I'll try to review more carefully soon, hopefully early next week.

I'm not sure I can have a source/target path that includes symlinks in a way that allows me to test this, and even more so with substitute drives since we can't know which drive letters are unallocated.

Not being a windows person, it's hard for me to answer about substitute drives, but I understand your concern.

One possible approach is to verify the relationships among the various flavors of a substitution (e.g., %t, %{t:real}, etc.), and hope (or ensure) that some CI configs that run check-lit will use substitute drives for the source and/or build directory. The tests should pass regardless.

A less desirable approach is just to do minimal sanity checks, such as checking that the base file name is the same across all flavors of a substitution.

Of course, the least desirable approach is to depend on other subprojects' test suites to catch lit bugs. It's easier for lit developers to identify lit bugs when check-lit itself shows them, preferably locally but possibly in CI.

Thanks for taking a look at this.

[MrTrillian]

This comment was removed by MrTrillian.

[tahonermann]

In D154130#4486898, @tahonermann wrote:

95% of the %>t are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it.

Can you link to evidence that this behavior is by design? It isn't obvious to me why modulemap files would demand different behavior; especially since that would exacerbate MAX_PATH problems.

I agree with you. The original rationale seems to be here:
https://github.com/llvm/llvm-project/blob/926f3759ec62a8f170e76a60316cc0bdd9dd2ec9/clang/lib/Lex/HeaderSearch.cpp#L257

Thank you for finding that. I took a look at the code and it looks to me like it would be safe to change ModuleMap::canonicalizeModuleMapPath() to use a drive preserving canonicalization. I think the worst case impact would be that a module cache lookup would fail thereby leading to a cache miss and rebuild of the target module. That could impose a significant performance and drive space penalty in the event that substitute drive paths are changed, but I would expect such changes to be rare.

[benlangmuir]

I took a look at the code and it looks to me like it would be safe to change ModuleMap::canonicalizeModuleMapPath() to use a drive preserving canonicalization

This sounds reasonable to me (the person who added canonicalizeModuleMapPath), though I am not at all a Windows path expert.

[MrTrillian]

I took a look at the code and it looks to me like it would be safe to change ModuleMap::canonicalizeModuleMapPath() to use a drive preserving canonicalization

Symbolic links can point across drives so I think a drive preserving canonicalization cannot be much more than a conversion to an absolute path -- similar to the now renamed abs_path_preserve_drive I'm adding to lit. I think the logic would have to be split across Windows (only abs) and non-Windows (realpath) again. I also think a cache miss in this case is ok, so I will follow up with a clang change implementing this. Thanks!

[MrTrillian]

That could impose a significant performance and drive space penalty in the event that substitute drive paths are changed, but I would expect such changes to be rare.

I think the more likely case is that one would build their repo under the substitute drive and later build it under the expanded path on the original drive. Though I feel like a lot of caching might then break.

[MrTrillian]

Update clang path canonicalization to avoid resolving substitute drives (ie resolving to a path under a different root). This requires much less change to tests.

[MrTrillian]

Undo whitespace changes.

[tahonermann]

It is unclear to me why/when we would ever want the substitute drive expansion; the modified tests aren't very elucidating. My naive expectation is that we effectively want to restore the Python 3.7 behavior.

clang/test/ExtractAPI/relative_include.m

24 ↗

(On Diff #542097)

I'm curious why this test requires the %{/t:real} normalization. Why would it be desirable to expand substitute drives for this test?

clang/test/Lexer/case-insensitive-include-win.c

14

I'm curious why this test requires the %{/t:real} normalization. Why would it be desirable to expand substitute drives for this test?

[MrTrillian]

In D154130#4516226, @tahonermann wrote:

It is unclear to me why/when we would ever want the substitute drive expansion; the modified tests aren't very elucidating. My naive expectation is that we effectively want to restore the Python 3.7 behavior.

It took some more investigation to figure out why I still needed realpaths there. In the end:

• case-insensitive-include.win.c: still needs it, added a comment.
• relative_module.m: required a fix in ExtractAPIConsumer.cpp to avoid calling getRealPath.

[tahonermann]

clang/test/Lexer/case-insensitive-include-win.c

5–9

Hmm, is it really desirable or necessary that the case mismatch logic resolve substituted drives? I wouldn't think so. This seems like another case where our common canonicalization would be desired.

[MrTrillian]

clang/test/Lexer/case-insensitive-include-win.c

5–9

I think it is necessary as I don't see how else we can retrieve the original casing of the file path. Merely making the path absolute would not do that. Searching for solutions on the internet finds ideas that are worse like converting to a short path then back to a long path or rebuilding the path one component at a time with a series directory listing requests.

[MrTrillian]

Marking comments as resolved per my reply. I'm not sure if that's best practice! @tahonermann

Looking forward to reviews

[MrTrillian]

Fixed clang-format issues and improved test comment.

[tahonermann]

Marking comments as resolved per my reply. I'm not sure if that's best practice!

Yes, that is fine.

clang/test/Lexer/case-insensitive-include-win.c

5–9

The warning this test checks for is diagnosed in Preprocessor::HandleHeaderIncludeOrImport(); search for pp_nonportable_path and/or pp_nonportable_system_path. I believe this warning will be issued if any component of the path does not match the case of the include directive. Since the file name differs in case, unless there is a bug in handling of the file name, this test isn't sensitive to case mismatches in %t.

From a user point of view, resolving a substitute drive doesn't seem desirableto me with respect to determining whether a non-portable path is being used; I don't think the behavior should be dependent on whether a substitute drive is being used or what its target is.

[MrTrillian]

Responded to comment.

clang/test/Lexer/case-insensitive-include-win.c

5–9

@tahonermann I think the code is working by design and it would be an unrelated change to modify its logic. See trySimplifyPath in PPDirectives.cpp:

// Below is a best-effort to handle ".." in paths. It is admittedly
// not 100% correct in the presence of symlinks.

      // If these path components differ by more than just case, then we
      // may be looking at symlinked paths. Bail on this diagnostic to avoid
      // noisy false positives.

The test was previously implicitly requiring getting the realpath, and it worked because lit.py's logic of doing that. Now that requirement is explicit in the test.

[MrTrillian]

Ping. Any brave reviewer to help here?

[benlangmuir]

clang/lib/Basic/FileManager.cpp

655

8k is a lot of stack space. The only reason this was 4k in the first place is it was originally using char[PATH_MAX] and unix realpath directly. I'd suggest just dropping to 128 per path.

663

Removing .. can change where the path points in the presence of symlinks; is this needed?

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

190

Why is this change needed?

[MrTrillian]

Reduced path small strings length to 256 (incidentally close to MAX_PATH on Windows), per @benlangmuir 's comment

[MrTrillian]

clang/lib/Basic/FileManager.cpp

663

Removing .. can change where the path points in the presence of symlinks; is this needed?

@benlangmuir That's true and not ideal, but makeAbsolute will not resolve /./ or /../ in paths, so it's not a canonicalization and some tests were failing because of that. One alternative would be to use makeAbsolute + remove_dots on Windows (where removing dot dots is semantically correct) and getRealPath on Unix, like I do in lit. Suggestions?

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

190

Why is this change needed?

@benlangmuir We don't want raw getRealPaths on Windows because of substitute drives and MAX_PATH issues. That is the idea behind this diff. If I leave the tryGetRealPathName here, I need to change the relative_include.m test as in this previous diff: https://reviews.llvm.org/D154130?id=539683 , which is undesirable.

[benlangmuir]

clang/lib/Basic/FileManager.cpp

663

Wouldn't removing .. have the same issue with symlinks on Windows? I know symlinks are less common there, but it's not clear to me why it would be correct. I guess you could also check if the paths resolve to the same file after removing ..

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

190

I wonder if we should just remove tryGetRealPathName; it's not actually the real path in many cases. Anyway, not for this patch, your change here seems fine.

llvm/utils/lit/lit/LitConfig.py

192 ↗

(On Diff #544743)

Why are you changing abspath (here and elsewhere)? I understand why you are changing realpath -> lit.util.abs_path_preserve_drive, but what's the issue with bare abspath?

llvm/utils/lit/lit/TestRunner.py

1297

This change drops the + ".tmp" that was previously added to %t:regex_replacement and %:t.

[MrTrillian]

Reverted some os.path.abspath that had been converted to abs_path_preserve_drive

[MrTrillian]

clang/lib/Basic/FileManager.cpp

663

@benlangmuir Windows simplifies \..\ in paths before starting to walk the filesystem. https://superuser.com/questions/1502360/different-behavior-of-double-dots-and-symlinks-in-windows-and-unix

If I detected that the path didn't resolve to the same file after removing .., what would I do?

I think the current logic will only end up using the else branch on Windows. At least I can't think of a scenario where the root would change from using realpath on unix.

llvm/utils/lit/lit/TestRunner.py

1297

This change drops the + ".tmp" that was previously added to %t:regex_replacement and %:t.

@benlangmuir Actually not! The path_substitutions list uses ("t", tmpName) which includes the .tmp, instead of tmpBase which doesn't. Looking at the original code, there didn't seem to be a reason to use tmpBase and re-append .tmp instead of using tmpName.

[MrTrillian]

llvm/utils/lit/lit/LitConfig.py

192 ↗

(On Diff #544743)

Why are you changing abspath (here and elsewhere)? I understand why you are changing realpath -> lit.util.abs_path_preserve_drive, but what's the issue with bare abspath?

You're right, this is overzealous. I think it made more sense with an older iteration of this code. I removed it.

[benlangmuir]

LGTM other than my suggestion to add a comment. @tahonermann are all your comments addressed at this point?

clang/lib/Basic/FileManager.cpp

663

Thanks, I wasn't aware of that subtlety! I suggest we add a comment about that here and also mention (or assert) that this is only reachable on Windows.

[MrTrillian]

@benlangmuir I made the root-preserving canonicalization logic Windows-only now that I found how to test for that. It changes the code shape a little but I think it's an improvement.

[MrTrillian]

clang/lib/Basic/FileManager.cpp

663

Thanks, I wasn't aware of that subtlety! I suggest we add a comment about that here and also mention (or assert) that this is only reachable on Windows.

@benlangmuir Accounted for in lastest revision. The root-preserving canonicalization logic is now Windows-only.

[benlangmuir]

clang/lib/Basic/FileManager.cpp

663

Thanks, looks good.

[MrTrillian]

Resolved remaining comments addressed with the FileManager.cpp change to branch on the native path format being Windows. This is ready to merge!

[RKSimon]

@MrTrillian This is failing for me with:

C:\LLVM\ninja>ninja check-llvm-codegen-x86
[0/1/0/1] Running lit suite C:/LLVM/llvm-project/llvm/test/CodeGen/X86llvm-lit.py: C:\LLVM\llvm-project\llvm\utils\lit\lit\TestingConfig.py:151: fatal: unable to parse config file 'C:\\LLVM\\llvm-project\\llvm\\test\\lit.cfg.py', traceback: Traceback (most recent call last):
  File "C:\LLVM\llvm-project\llvm\utils\lit\lit\TestingConfig.py", line 139, in load_from_path
    exec(compile(data, path, "exec"), cfg_globals, None)
  File "C:\LLVM\llvm-project\llvm\test\lit.cfg.py", line 21, in <module>
    config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'use_lit_shell'

[RKSimon]

llvm/utils/lit/lit/discovery.py

60

Found the problem - you have moved the os.path.normpath call inside abs_path_preserve_drive, causing the path canonicalization fails - removing it and going back to os.path.normcase(cfgpath) seems to fix the issue - I'll push my fix shortly.

[RKSimon]

llvm/utils/lit/lit/discovery.py

60

5ccfa1568130 fixed my builds but broke this - https://lab.llvm.org/buildbot/#/builders/216/builds/24966

Any suggestions would be appreciated, but I can't think of anything but reverting both commits until we get to the bottom of it.

[RKSimon]

llvm/utils/lit/lit/discovery.py

60

Panic over - I think I've fixed this with d6f1880c629d

[tahonermann]

@tahonermann are all your comments addressed at this point?

Apologies for the late response; I was on vacation last week.

No. I'm still skeptical that there is ever a desire to expand substitute drives.

clang/test/Lexer/case-insensitive-include-win.c

5–9

I'm still not following here. Are you saying that trySimplifyPath() will replace substitute drives? If so, that doesn't sound desirable and I would expect it to be problematic for your use case. I think we should follow this up ... somewhere (now that this review is closed).

[MrTrillian]

clang/test/Lexer/case-insensitive-include-win.c

5–9

@tahonermann . trySimplifyPath() does not replace substitute drives. It's a best-effort attempt to see if the included path mismatches the real file path only by case. It explicitly bails out without diagnostics if it finds that the included path has a different shape from the real file path, which will happen if the included path is on a substitute drive. It has to compare with the real file path because this is the only reasonable way to get the original path casing.

It was already the case that this diagnostic bailed out in the presence of symbolic links, so there are no behavioral differences. I needed to update the test because previously lit.py would enforce that %t was a real path, and now it doesn't, which means that we would hit the "bail out" code path and not produce the diagnostic.

[tahonermann]

clang/test/Lexer/case-insensitive-include-win.c

5–9

I think trySimplifyPath() is not particularly relevant as it just performs a simple canonicalization (removal of . and .. path components without regard for symlink traversal) and case insensitive comparison with the remaining components with a "real" path that is presumed to already be devoid of such components. It is therefore sensitive to structure, but only for the (non-. and non-..) components present in the (non-real) Components vector; the "real" path may have more leading components (the Components vector may represent a relative path). The presence of a substitute drive in the "real" path won't contribute to a structural difference unless the Components vector is absolute but with a drive other than the substitute drive or if it is relative but starting at a higher directory level than the substitute drive; neither of which should be the case for this test when %t is consistently used.

The only relevant user path provided to Clang is the argument to the /FI argument; previously \\?\%t.dir\FOO.h. In that case, %t should reflect use of a substitute drive. The question I have then is why/where Clang is replacing the substitute drive. I'm guessing (I haven't actually debugged) is that this is happening in FileManager::fillRealPathName(); this function assigns the path returned by FileEntry::tryGetRealPathName(). I'm curious if that only happens if /FI specifies a UNC path; does the test pass if the UNC prefix is dropped and the path with the substitute drive is used? Could you try that, please? /FI %t.dir\FOO.h. If that still passes, then I think there is a question of whether it is desirable for behavior to differ for \\?\ prefixed paths.

[dyung]

Sorry to comment so late, but I just had time to look into this a little bit. This change broke running lit tests for me on Windows, and I wanted to see if there was a way I could restore it.

For reference, my sources are stored locally at C:\Dev\git\dev, but I until this change, I was able to access them through a file system junction point at C:\src\git\dev and run lit tests using that. But after this change, I can no longer run the lit tests when I am in C:\src\git\dev. My system is setup this way because we have weird A/V scanning rules that only exclude certain named directories, and my original path wasn't one of them, so I used the file system junction point to get my builds excluded while continuing to use the path structure that was familiar to me and that I also use on other systems (mac/linux).

Since this change seems to be targeted towards preventing resolving paths that are greater than MAX_PATH on Windows, could we possibly leave the substitution in, but check if the resulting path is longer than MAX_PATH rather than just excluding it completely?

Specifically, where I am seeing the difference is in the new function abs_path_preserve_drive() in util.py where we are now using os.path.abspath() instead of os.path.realpath(). I'm not completely sure what the effects of such a change might be to the rest of this change, but keeping the old conversion and then checking against MAX_PATH it seems would still accomplish your goal while allowing cases like mine to continue working as they previously did.

Thoughts?

clang/include/clang/Basic/FileManager.h

public:
 
   /// Retrieve the canonical name for a given file.
   ///
   /// This is a very expensive operation, despite its results being cached,
   /// and should only be used when the physical layout of the file system is
   /// required, which is (almost) never.
   StringRef getCanonicalName(const FileEntry *File);
 
+private:
+  /// Retrieve the canonical name for a given file or directory.
+  ///
+  /// The first param is a key in the CanonicalNames array.
+  StringRef getCanonicalName(const void *Entry, StringRef Name);
+
+public:
   void PrintStats() const;
 };
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_FILEMANAGER_H

clang/lib/Basic/FileManager.cpp

for (llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>,
     }
 
   // Map virtual file entries
   for (const auto &VFE : VirtualFileEntries)
     UIDToFiles[VFE->getUID()] = VFE;
 }
 
 StringRef FileManager::getCanonicalName(DirectoryEntryRef Dir) {
-  auto Known = CanonicalNames.find(Dir);
-  if (Known != CanonicalNames.end())
-    return Known->second;
-
-  StringRef CanonicalName(Dir.getName());
-
-  SmallString<4096> CanonicalNameBuf;
-  if (!FS->getRealPath(Dir.getName(), CanonicalNameBuf))
-    CanonicalName = CanonicalNameBuf.str().copy(CanonicalNameStorage);
-
-  CanonicalNames.insert({Dir, CanonicalName});
-  return CanonicalName;
+  return getCanonicalName(Dir, Dir.getName());
 }
 
 StringRef FileManager::getCanonicalName(const FileEntry *File) {
-  llvm::DenseMap<const void *, llvm::StringRef>::iterator Known
-    = CanonicalNames.find(File);
+  return getCanonicalName(File, File->getName());
+}
+
+StringRef FileManager::getCanonicalName(const void *Entry, StringRef Name) {
+  llvm::DenseMap<const void *, llvm::StringRef>::iterator Known =
+      CanonicalNames.find(Entry);
   if (Known != CanonicalNames.end())
     return Known->second;
 
-  StringRef CanonicalName(File->getName());
-
-  SmallString<4096> CanonicalNameBuf;
-  if (!FS->getRealPath(File->getName(), CanonicalNameBuf))
-    CanonicalName = CanonicalNameBuf.str().copy(CanonicalNameStorage);
+  // Name comes from FileEntry/DirectoryEntry::getName(), so it is safe to
+  // store it in the DenseMap below.
+  StringRef CanonicalName(Name);
+
+  SmallString<256> AbsPathBuf;
+  SmallString<256> RealPathBuf;
+  if (!FS->getRealPath(Name, RealPathBuf)) {
+    if (is_style_windows(llvm::sys::path::Style::native)) {
+      // For Windows paths, only use the real path if it doesn't resolve
+      // a substitute drive, as those are used to avoid MAX_PATH issues.
+      AbsPathBuf = Name;
+      if (!FS->makeAbsolute(AbsPathBuf)) {
+        if (llvm::sys::path::root_name(RealPathBuf) ==
+            llvm::sys::path::root_name(AbsPathBuf)) {
+          CanonicalName = RealPathBuf.str().copy(CanonicalNameStorage);
+        } else {
+          // Fallback to using the absolute path.
+          // Simplifying /../ is semantically valid on Windows even in the
+          // presence of symbolic links.
+          llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true);
+          CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage);
+        }
+      }
+    } else {
+      CanonicalName = RealPathBuf.str().copy(CanonicalNameStorage);
+    }
+  }
 
-  CanonicalNames.insert({File, CanonicalName});
+  CanonicalNames.insert({Entry, CanonicalName});
   return CanonicalName;
 }
 
 void FileManager::PrintStats() const {
   llvm::errs() << "\n*** File Manager Stats:\n";
   llvm::errs() << UniqueRealFiles.size() << " real files found, "
                << UniqueRealDirs.size() << " real dirs found.\n";
   llvm::errs() << VirtualFileEntries.size() << " virtual files found, "
                << VirtualDirectoryEntries.size() << " virtual dirs found.\n";
   llvm::errs() << NumDirLookups << " dir lookups, "
                << NumDirCacheMisses << " dir cache misses.\n";
   llvm::errs() << NumFileLookups << " file lookups, "
                << NumFileCacheMisses << " file cache misses.\n";
 
   //llvm::errs() << PagesMapped << BytesOfPagesMapped << FSLookups;
 }

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

if (!File)
       return false;
 
     if (KnownFileEntries.count(File))
       return true;
 
     if (ExternalFileEntries.count(File))
       return false;
 
-    StringRef FileName = File->tryGetRealPathName().empty()
-                             ? File->getName()
-                             : File->tryGetRealPathName();
+    StringRef FileName = SM.getFileManager().getCanonicalName(File);
 
     // Try to reduce the include name the same way we tried to include it.
     bool IsQuoted = false;
     if (auto IncludeName = getRelativeIncludeName(CI, FileName, &IsQuoted))
       if (llvm::any_of(KnownFiles,
                        [&IsQuoted, &IncludeName](const auto &KnownFile) {
                          return KnownFile.first.equals(*IncludeName) &&
                                 KnownFile.second == IsQuoted;

clang/test/Lexer/case-insensitive-include-win.c

 // Most Microsoft-specific testing should go in case-insensitive-include-ms.c
 // This file should only include code that really needs a Windows host OS to
 // run.
 
+// Note: We must use the real path here, because the logic to detect case
+// mismatches must resolve the real path to figure out the original casing.
+// If we use %t and we are on a substitute drive S: mapping to C:\subst,
+// then we will compare "S:\test.dir\FOO.h" to "C:\subst\test.dir\foo.h"
+// and avoid emitting the diagnostic because the structure is different.
+
 // REQUIRES: system-windows
 // RUN: mkdir -p %t.dir
 // RUN: touch %t.dir/foo.h
-// RUN: not %clang_cl /FI\\?\%t.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: not %clang_cl /FI \\?\%{t:real}.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s
 
 // CHECK: non-portable path to file '"\\?\

llvm/cmake/modules/AddLLVM.cmake

sys.stdout.write(';'.join(relpath(p) for p in sys.argv[2].split(';')))"
   set(${out_pathlist} "${pathlist_relative}" PARENT_SCOPE)
 endfunction()
 
 # Converts a file that's relative to the current python file to an absolute
 # path. Since this uses __file__, it has to be emitted into python files that
 # use it and can't be in a lit module. Use with make_paths_relative().
 string(CONCAT LLVM_LIT_PATH_FUNCTION
   "# Allow generated file to be relocatable.\n"
-  "from pathlib import Path\n"
+  "import os\n"
+  "import platform\n"
   "def path(p):\n"
   "    if not p: return ''\n"
-  "    return str((Path(__file__).parent / p).resolve())\n"
+  "    # Follows lit.util.abs_path_preserve_drive, which cannot be imported here.\n"
+  "    if platform.system() == 'Windows':\n"
+  "        return os.path.abspath(os.path.join(os.path.dirname(__file__), p))\n"
+  "    else:\n"
+  "        return os.path.realpath(os.path.join(os.path.dirname(__file__), p))\n"
   )
 
 # This function provides an automatic way to 'configure'-like generate a file
 # based on a set of common and custom variables, specifically targeting the
 # variables needed for the 'lit.site.cfg' files. This function bundles the
 # common variables that any Lit instance is likely to need, and custom
 # variables can be passed in.
 # The keyword PATHS is followed by a list of cmake variable names that are

llvm/docs/CommandGuide/lit.rst

  %basename_t             The last path component of %t but without the ``.tmp`` extension
  %T                      parent directory of %t (not unique, deprecated, do not use)
  %%                      %
  %/s                     %s but ``\`` is replaced by ``/``
  %/S                     %S but ``\`` is replaced by ``/``
  %/p                     %p but ``\`` is replaced by ``/``
  %/t                     %t but ``\`` is replaced by ``/``
  %/T                     %T but ``\`` is replaced by ``/``
+ %{s:real}               %s after expanding all symbolic links and substitute drives
+ %{S:real}               %S after expanding all symbolic links and substitute drives
+ %{p:real}               %p after expanding all symbolic links and substitute drives
+ %{t:real}               %t after expanding all symbolic links and substitute drives
+ %{T:real}               %T after expanding all symbolic links and substitute drives
+ %{/s:real}              %/s after expanding all symbolic links and substitute drives
+ %{/S:real}              %/S after expanding all symbolic links and substitute drives
+ %{/p:real}              %/p after expanding all symbolic links and substitute drives
+ %{/t:real}              %/t after expanding all symbolic links and substitute drives
+ %{/T:real}              %/T after expanding all symbolic links and substitute drives
  %{/s:regex_replacement} %/s but escaped for use in the replacement of a ``s@@@`` command in sed
  %{/S:regex_replacement} %/S but escaped for use in the replacement of a ``s@@@`` command in sed
  %{/p:regex_replacement} %/p but escaped for use in the replacement of a ``s@@@`` command in sed
  %{/t:regex_replacement} %/t but escaped for use in the replacement of a ``s@@@`` command in sed
  %{/T:regex_replacement} %/T but escaped for use in the replacement of a ``s@@@`` command in sed
  %:s                     On Windows, %/s but a ``:`` is removed if its the second character.
                          Otherwise, %s but with a single leading ``/`` removed.
  %:S                     On Windows, %/S but a ``:`` is removed if its the second character.

llvm/docs/TestingGuide.rst

 ``${fs-tmp-root}``
    Expands to the root component of file system paths for the test's temporary
    directory, i.e. ``/`` on Unix systems or ``C:\`` (or another drive) on
    Windows.
 
 ``${fs-sep}``
    Expands to the file system separator, i.e. ``/`` or ``\`` on Windows.
 
-``%/s, %/S, %/t, %/T:``
+``%/s, %/S, %/t, %/T``
 
   Act like the corresponding substitution above but replace any ``\``
   character with a ``/``. This is useful to normalize path separators.
 
    Example: ``%s:  C:\Desktop Files/foo_test.s.tmp``
 
    Example: ``%/s: C:/Desktop Files/foo_test.s.tmp``
 
-``%:s, %:S, %:t, %:T:``
+``%{s:real}, %{S:real}, %{t:real}, %{T:real}``
+``%{/s:real}, %{/S:real}, %{/t:real}, %{/T:real}``
+
+  Act like the corresponding substitution, including with ``/``, but use
+  the real path by expanding all symbolic links and substitute drives.
+
+   Example: ``%s:  S:\foo_test.s.tmp``
+
+   Example: ``%{/s:real}: C:/SDrive/foo_test.s.tmp``
+
+``%:s, %:S, %:t, %:T``
 
   Act like the corresponding substitution above but remove colons at
   the beginning of Windows paths. This is useful to allow concatenation
   of absolute paths on Windows to produce a legal path.
 
    Example: ``%s:  C:\Desktop Files\foo_test.s.tmp``
 
    Example: ``%:s: C\Desktop Files\foo_test.s.tmp``

llvm/utils/lit/lit/TestRunner.py

def __init__(self, cwd, env):
         self.cwd = cwd
         self.env = dict(env)
         self.dirStack = []
 
     def change_dir(self, newdir):
         if os.path.isabs(newdir):
             self.cwd = newdir
         else:
-            self.cwd = os.path.realpath(os.path.join(self.cwd, newdir))
+            self.cwd = lit.util.abs_path_preserve_drive(os.path.join(self.cwd, newdir))
 
 
 class TimeoutHelper(object):
     """
     Object used to helper manage enforcing a timeout in
     _executeShCmd(). It is passed through recursive calls
     to collect processes that have been executed so that when
     the timeout happens they can be killed.
def executeBuiltinMkdir(cmd, cmd_shenv):
 
     stderr = StringIO()
     exitCode = 0
     for dir in args:
         cwd = cmd_shenv.cwd
         dir = to_unicode(dir) if kIsWindows else to_bytes(dir)
         cwd = to_unicode(cwd) if kIsWindows else to_bytes(cwd)
         if not os.path.isabs(dir):
-            dir = os.path.realpath(os.path.join(cwd, dir))
+            dir = lit.util.abs_path_preserve_drive(os.path.join(cwd, dir))
         if parent:
             lit.util.mkdir_p(dir)
         else:
             try:
                 lit.util.mkdir(dir)
             except OSError as err:
                 stderr.write("Error: 'mkdir' command failed, %s\n" % str(err))
                 exitCode = 1
def executeBuiltinRm(cmd, cmd_shenv):
 
     stderr = StringIO()
     exitCode = 0
     for path in args:
         cwd = cmd_shenv.cwd
         path = to_unicode(path) if kIsWindows else to_bytes(path)
         cwd = to_unicode(cwd) if kIsWindows else to_bytes(cwd)
         if not os.path.isabs(path):
-            path = os.path.realpath(os.path.join(cwd, path))
+            path = lit.util.abs_path_preserve_drive(os.path.join(cwd, path))
         if force and not os.path.exists(path):
             continue
         try:
             if os.path.isdir(path):
                 if not recursive:
                     stderr.write("Error: %s is a directory\n" % path)
                     exitCode = 1
                 if platform.system() == "Windows":
if normalize_slashes:
         sourcedir = sourcedir.replace("\\", "/")
         tmpDir = tmpDir.replace("\\", "/")
         tmpBase = tmpBase.replace("\\", "/")
 
     substitutions = []
     substitutions.extend(test.config.substitutions)
     tmpName = tmpBase + ".tmp"
     baseName = os.path.basename(tmpBase)
-    substitutions.extend(
-        [
-            ("%s", sourcepath),
-            ("%S", sourcedir),
-            ("%p", sourcedir),
-            ("%{pathsep}", os.pathsep),
-            ("%t", tmpName),
-            ("%basename_t", baseName),
-            ("%T", tmpDir),
-        ]
-    )
+
+    substitutions.append(("%{pathsep}", os.pathsep))
+    substitutions.append(("%basename_t", baseName))
     
     substitutions.extend(
         [
             ("%{fs-src-root}", pathlib.Path(sourcedir).anchor),
             ("%{fs-tmp-root}", pathlib.Path(tmpBase).anchor),
             ("%{fs-sep}", os.path.sep),
         ]
     )
 
-    # "%/[STpst]" should be normalized.
-    substitutions.extend(
-        [
-            ("%/s", sourcepath.replace("\\", "/")),
-            ("%/S", sourcedir.replace("\\", "/")),
-            ("%/p", sourcedir.replace("\\", "/")),
-            ("%/t", tmpBase.replace("\\", "/") + ".tmp"),
-            ("%/T", tmpDir.replace("\\", "/")),
-            ("%/et", tmpName.replace("\\", "\\\\\\\\\\\\\\\\")),
-        ]
-    )
+    substitutions.append(("%/et", tmpName.replace("\\", "\\\\\\\\\\\\\\\\")))
 
-    # "%{/[STpst]:regex_replacement}" should be normalized like "%/[STpst]" but we're
-    # also in a regex replacement context of a s@@@ regex.
     def regex_escape(s):
         s = s.replace("@", r"\@")
         s = s.replace("&", r"\&")
         return s
 
-    substitutions.extend(
-        [
-            ("%{/s:regex_replacement}", regex_escape(sourcepath.replace("\\", "/"))),
-            ("%{/S:regex_replacement}", regex_escape(sourcedir.replace("\\", "/"))),
-            ("%{/p:regex_replacement}", regex_escape(sourcedir.replace("\\", "/"))),
-            (
-                "%{/t:regex_replacement}",
-                regex_escape(tmpBase.replace("\\", "/")) + ".tmp",
-            ),
-            ("%{/T:regex_replacement}", regex_escape(tmpDir.replace("\\", "/"))),
-        ]
-    )
+    path_substitutions = [
+        ("s", sourcepath), ("S", sourcedir), ("p", sourcedir),
+        ("t", tmpName), ("T", tmpDir)
+    for path_substitution in path_substitutions:
+        letter = path_substitution[0]
+        path = path_substitution[1]
+
+        # Original path variant
+        substitutions.append(("%" + letter, path))
+
+        # Normalized path separator variant
+        substitutions.append(("%/" + letter, path.replace("\\", "/")))
+
+        # realpath variants
+        # Windows paths with substitute drives are not expanded by default
+        # as they are used to avoid MAX_PATH issues, but sometimes we do
+        # need the fully expanded path.
+        real_path = os.path.realpath(path)
+        substitutions.append(("%{" + letter + ":real}", real_path))
+        substitutions.append(("%{/" + letter + ":real}",
+            real_path.replace("\\", "/")))
+
+        # "%{/[STpst]:regex_replacement}" should be normalized like
+        # "%/[STpst]" but we're also in a regex replacement context
+        # of a s@@@ regex.
+        substitutions.append(
+            ("%{/" + letter + ":regex_replacement}",
+            regex_escape(path.replace("\\", "/"))))
+
+        # "%:[STpst]" are normalized paths without colons and without
+        # a leading slash.
+        substitutions.append(("%:" + letter, colonNormalizePath(path)))
 
-    # "%:[STpst]" are normalized paths without colons and without a leading
-    # slash.
-    substitutions.extend(
-        [
-            ("%:s", colonNormalizePath(sourcepath)),
-            ("%:S", colonNormalizePath(sourcedir)),
-            ("%:p", colonNormalizePath(sourcedir)),
-            ("%:t", colonNormalizePath(tmpBase + ".tmp")),
-            ("%:T", colonNormalizePath(tmpDir)),
-        ]
-    )
     return substitutions
 
 
 def _memoize(f):
     cache = {}  # Intentionally unbounded, see applySubstitutions()
 
     def memoized(x):
         if x not in cache:

llvm/utils/lit/lit/builtin_commands/diff.py

def main(argv):
     if len(args) != 2:
         sys.stderr.write("Error: missing or extra operand\n")
         sys.exit(1)
 
     exitCode = 0
     try:
         for file in args:
             if file != "-" and not os.path.isabs(file):
-                file = os.path.realpath(os.path.join(os.getcwd(), file))
+                file = util.abs_path_preserve_drive(file)
 
             if flags.recursive_diff:
                 if file == "-":
                     sys.stderr.write("Error: cannot recursively compare '-'\n")
                     sys.exit(1)
                 dir_trees.append(getDirTree(file))
             else:
                 filepaths.append(file)

llvm/utils/lit/lit/discovery.py

 """
 Test discovery functions.
 """
 
 import copy
 import os
 import sys
 
 from lit.TestingConfig import TestingConfig
-from lit import LitConfig, Test
+from lit import LitConfig, Test, util
 
 
 def chooseConfigFileFromDir(dir, config_names):
     for name in config_names:
         p = os.path.join(dir, name)
         if os.path.exists(p):
             return p
     return None
def search1(path):
         # This is a private builtin parameter which can be used to perform
         # translation of configuration paths.  Specifically, this parameter
         # can be set to a dictionary that the discovery process will consult
         # when it finds a configuration it is about to load.  If the given
         # path is in the map, the value of that key is a path to the
         # configuration to load instead.
         config_map = litConfig.params.get("config_map")
         if config_map:
-            cfgpath = os.path.realpath(cfgpath)
-            target = config_map.get(os.path.normcase(cfgpath))
+            cfgpath = util.abs_path_preserve_drive(cfgpath)
+            target = config_map.get(cfgpath)
             if target:
                 cfgpath = target
 
         # We found a test suite, create a new config for it and load it.
         if litConfig.debug:
             litConfig.note("loading suite config %r" % cfgpath)
 
         cfg = TestingConfig.fromdefaults(litConfig)
         cfg.load_from_path(cfgpath, litConfig)
-        source_root = os.path.realpath(cfg.test_source_root or path)
-        exec_root = os.path.realpath(cfg.test_exec_root or path)
+        source_root = util.abs_path_preserve_drive(cfg.test_source_root or path)
+        exec_root = util.abs_path_preserve_drive(cfg.test_exec_root or path)
         return Test.TestSuite(cfg.name, source_root, exec_root, cfg), ()
 
     def search(path):
         # Check for an already instantiated test suite.
-        real_path = os.path.realpath(path)
+        real_path = util.abs_path_preserve_drive(path)
         res = cache.get(real_path)
         if res is None:
             cache[real_path] = res = search1(path)
         return res
 
     # Canonicalize the path.
     item = os.path.normpath(os.path.join(os.getcwd(), item))
 

llvm/utils/lit/lit/util.py

def usable_core_count():
 
     # On Windows with more than 60 processes, multiprocessing's call to
     # _winapi.WaitForMultipleObjects() prints an error and lit hangs.
     if platform.system() == "Windows":
         return min(n, 60)
 
     return n
 
+def abs_path_preserve_drive(path):
+    """Return the absolute path without resolving drive mappings on Windows.
+
+    """
+    if platform.system() == "Windows":
+        # Windows has limitations on path length (MAX_PATH) that
+        # can be worked around using substitute drives, which map
+        # a drive letter to a longer path on another drive.
+        # Since Python 3.8, os.path.realpath resolves sustitute drives,
+        # so we should not use it. In Python 3.7, os.path.realpath
+        # was implemented as os.path.abspath.
+        return os.path.normpath(os.path.abspath(path))
+    else:
+        # On UNIX, the current directory always has symbolic links resolved,
+        # so any program accepting relative paths cannot preserve symbolic
+        # links in paths and we should always use os.path.realpath.
+        return os.path.normpath(os.path.realpath(path))
 
 def mkdir(path):
     try:
         if platform.system() == "Windows":
             from ctypes import windll
             from ctypes import GetLastError, WinError
 
             path = os.path.abspath(path)

llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py

 import lit.util
 import os
 import sys
 
-main_config = sys.argv[1]
-main_config = os.path.realpath(main_config)
-main_config = os.path.normcase(main_config)
+main_config = lit.util.abs_path_preserve_drive(sys.argv[1])
 
 config_map = {main_config: sys.argv[2]}
 builtin_parameters = {"config_map": config_map}
 
 if __name__ == "__main__":
     from lit.main import main
 
     main_config_dir = os.path.dirname(main_config)
     sys.argv = [sys.argv[0]] + sys.argv[3:] + [main_config_dir]
     main(builtin_parameters)

llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg

 import lit.formats
 import lit.util
 config.name = 'config-map'
 config.suffixes = ['.txt']
 config.test_format = lit.formats.ShTest()
 
 import os
-config.test_exec_root = os.path.realpath(os.path.dirname(__file__))
+config.test_exec_root = os.path.dirname(lit.util.abs_path_preserve_drive(__file__))
 config.test_source_root = os.path.join(config.test_exec_root, "tests")

llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg

 import lit.formats
+import lit.util
 
 config.name = "use-llvm-tool-required"
 config.suffixes = [".txt"]
 config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
 import os.path
 
-config.llvm_tools_dir = os.path.realpath(os.path.dirname(__file__))
+config.llvm_tools_dir = os.path.dirname(lit.util.abs_path_preserve_drive(__file__))
 import lit.llvm
 
 lit.llvm.initialize(lit_config, config)
 lit.llvm.llvm_config.use_llvm_tool("found", required=True)
 lit.llvm.llvm_config.use_llvm_tool("not-found", required=True)

llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg

 import lit.formats
+import lit.util
 
 config.name = "use-llvm-tool"
 config.suffixes = [".txt"]
 config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
 import os.path
 
-this_dir = os.path.realpath(os.path.dirname(__file__))
+this_dir = os.path.dirname(lit.util.abs_path_preserve_drive(__file__))
 config.llvm_tools_dir = os.path.join(this_dir, "build")
 import lit.llvm
 
 lit.llvm.initialize(lit_config, config)
 lit.llvm.llvm_config.with_environment("CASE1", os.path.join(this_dir, "env-case1"))
 lit.llvm.llvm_config.with_environment("CASE6", os.path.join(this_dir, "env-case6"))
 lit.llvm.llvm_config.with_environment(
     "PATH", os.path.join(this_dir, "path"), append_path=True

llvm/utils/llvm-lit/llvm-lit.in

 #!@Python3_EXECUTABLE@
 # -*- coding: utf-8 -*-
 
 import os
 import sys
 
 config_map = {}
 
 def map_config(source_dir, site_config):
     global config_map
-    source_dir = os.path.realpath(source_dir)
+    source_dir = os.path.abspath(source_dir)
     source_dir = os.path.normcase(source_dir)
     site_config = os.path.normpath(site_config)
     config_map[source_dir] = site_config
 
 # Set up some builtin parameters, so that by default the LLVM test suite
 # configuration file knows how to find the object tree.
 builtin_parameters = { 'build_mode' : '@BUILD_MODE@' }