diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index 8eca0fd64f0b5..b192dc9fb37b9 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -339,6 +339,13 @@ class FileManager : public RefCountedBase { /// 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 f9b2afffa5913..1c32aab2fb562 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -651,34 +651,48 @@ void FileManager::GetUniqueIDMapping( } StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) { - llvm::DenseMap::iterator 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::iterator Known - = CanonicalNames.find(File); + return getCanonicalName(File, File->getName()); +} + +StringRef FileManager::getCanonicalName(const void *Entry, StringRef Name) { + llvm::DenseMap::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 5a490080f38c6..35748ecedd4b0 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -184,9 +184,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 6a17d0b552448..ed722feb3d994 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 '"\\?\ diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake index a8a52a45e4973..b3cee26cab051 100644 --- a/llvm/cmake/modules/AddLLVM.cmake +++ b/llvm/cmake/modules/AddLLVM.cmake @@ -1688,10 +1688,15 @@ endfunction() # 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 diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst index e9bd46e84769a..580ba9e33b7ca 100644 --- a/llvm/docs/CommandGuide/lit.rst +++ b/llvm/docs/CommandGuide/lit.rst @@ -541,6 +541,16 @@ TestRunner.py: %/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 diff --git a/llvm/docs/TestingGuide.rst b/llvm/docs/TestingGuide.rst index cd18cb4a4a5a1..b62dc280074f4 100644 --- a/llvm/docs/TestingGuide.rst +++ b/llvm/docs/TestingGuide.rst @@ -581,7 +581,7 @@ RUN lines: ``${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. @@ -590,7 +590,17 @@ RUN lines: 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 diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index 664235beaae6b..eb595e5194825 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -70,7 +70,7 @@ 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): """ @@ -401,7 +401,7 @@ def executeBuiltinMkdir(cmd, cmd_shenv): 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: @@ -446,7 +446,7 @@ def on_rm_error(func, path, exc_info): 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: @@ -1153,58 +1153,59 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False): 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.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('\\', '\\\\\\\\\\\\\\\\')), - ]) - - # "%{/[STpst]:regex_replacement}" should be normalized like "%/[STpst]" but we're - # also in a regex replacement context of a s@@@ regex. + + 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), + ] + ) + + substitutions.append(("%/et", tmpName.replace("\\", "\\\\\\\\\\\\\\\\"))) + 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('\\', '/'))), - ]) - - # "%:[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)), - ]) + + 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))) + return substitutions def _memoize(f): diff --git a/llvm/utils/lit/lit/builtin_commands/diff.py b/llvm/utils/lit/lit/builtin_commands/diff.py index a12e5e100b1bd..7233d81e72dc3 100644 --- a/llvm/utils/lit/lit/builtin_commands/diff.py +++ b/llvm/utils/lit/lit/builtin_commands/diff.py @@ -229,7 +229,7 @@ def main(argv): 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 == "-": diff --git a/llvm/utils/lit/lit/discovery.py b/llvm/utils/lit/lit/discovery.py index bb77aa310c713..254b8ae5050ad 100644 --- a/llvm/utils/lit/lit/discovery.py +++ b/llvm/utils/lit/lit/discovery.py @@ -7,7 +7,7 @@ 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: @@ -52,7 +52,7 @@ def search1(path): # configuration to load instead. config_map = litConfig.params.get('config_map') if config_map: - cfgpath = os.path.realpath(cfgpath) + cfgpath = util.abs_path_preserve_drive(cfgpath) target = config_map.get(os.path.normcase(cfgpath)) if target: cfgpath = target @@ -63,13 +63,13 @@ def search1(path): 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) diff --git a/llvm/utils/lit/lit/util.py b/llvm/utils/lit/lit/util.py index 75cface848390..4d3802db31fab 100644 --- a/llvm/utils/lit/lit/util.py +++ b/llvm/utils/lit/lit/util.py @@ -127,6 +127,23 @@ def usable_core_count(): 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.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.realpath(path) def mkdir(path): try: diff --git a/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py b/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py index db9141b9b1bf9..19a74faeca73e 100644 --- a/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py +++ b/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py @@ -2,8 +2,7 @@ import os import sys -main_config = sys.argv[1] -main_config = os.path.realpath(main_config) +main_config = lit.util.abs_path_preserve_drive(sys.argv[1]) main_config = os.path.normcase(main_config) config_map = {main_config : sys.argv[2]} diff --git a/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg b/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg index c7b303f50a05c..27860e192fdc1 100644 --- a/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg +++ b/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg @@ -5,5 +5,5 @@ 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") diff --git a/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg b/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg index b1c510e05ab74..f5101dbf2b052 100644 --- a/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg +++ b/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg @@ -1,11 +1,14 @@ 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) diff --git a/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg b/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg index 2bd401aacde35..f248ec596c78c 100644 --- a/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg +++ b/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg @@ -1,11 +1,14 @@ 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) diff --git a/llvm/utils/llvm-lit/llvm-lit.in b/llvm/utils/llvm-lit/llvm-lit.in index 33ec8017cf05f..0b6683b6b6230 100755 --- a/llvm/utils/llvm-lit/llvm-lit.in +++ b/llvm/utils/llvm-lit/llvm-lit.in @@ -8,7 +8,7 @@ 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