diff options
author | Saleem Abdulrasool <compnerd@compnerd.org> | 2023-08-01 07:23:42 -0700 |
---|---|---|
committer | Saleem Abdulrasool <compnerd@compnerd.org> | 2023-08-01 11:00:27 -0700 |
commit | 05d613ea931b6de1b46dfe04b8e55285359047f4 (patch) | |
tree | 15b7372cd29b3d2663fabee307d57b788275fb7f /clang | |
parent | cdb7d5767c4118dfc8f768f2f3982241a3b46314 (diff) | |
download | llvm-05d613ea931b6de1b46dfe04b8e55285359047f4.zip llvm-05d613ea931b6de1b46dfe04b8e55285359047f4.tar.gz llvm-05d613ea931b6de1b46dfe04b8e55285359047f4.tar.bz2 |
[lit][clang] Avoid realpath on Windows due to MAX_PATH limitations
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 (i.e. resolving to a path
under a different root).
How tested: built with `-DLLVM_ENABLE_PROJECTS=clang` and built `check-all` on both Windows
Differential Revision: https://reviews.llvm.org/D154130
Reviewed By: @benlangmuir
Patch by Tristan Labelle <tristan@thebrowser.company>!
Diffstat (limited to 'clang')
-rw-r--r-- | clang/include/clang/Basic/FileManager.h | 7 | ||||
-rw-r--r-- | clang/lib/Basic/FileManager.cpp | 55 | ||||
-rw-r--r-- | clang/lib/ExtractAPI/ExtractAPIConsumer.cpp | 4 | ||||
-rw-r--r-- | clang/test/Lexer/case-insensitive-include-win.c | 8 |
4 files changed, 50 insertions, 24 deletions
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index 502b69c..58ff42ee 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -329,6 +329,13 @@ public: /// 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; }; diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index f92c1ae..c3eec80 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -632,33 +632,48 @@ void FileManager::GetUniqueIDMapping( } 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; } diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp index eb533a9..50c2236 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -187,9 +187,7 @@ struct LocationFileChecker { 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; diff --git a/clang/test/Lexer/case-insensitive-include-win.c b/clang/test/Lexer/case-insensitive-include-win.c index 6a17d0b..ed722fe 100644 --- a/clang/test/Lexer/case-insensitive-include-win.c +++ b/clang/test/Lexer/case-insensitive-include-win.c @@ -2,9 +2,15 @@ // 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 '"\\?\ |