From f4d83c56c99deb52769aab12bbd22b9bfeb0c617 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Martin=20Storsj=C3=B6?= Date: Tue, 5 Oct 2021 10:17:36 +0000 Subject: [PATCH] [Support] [Windows] Convert paths to the preferred form This normalizes most paths (except ones input from the user as command line arguments) into the preferred form, if `real_style()` evaluates to `windows_forward`. Differential Revision: https://reviews.llvm.org/D111880 --- llvm/lib/Support/Windows/Path.inc | 27 +++++++++++++++-- llvm/lib/Support/Windows/Process.inc | 1 + llvm/lib/Support/Windows/Program.inc | 1 + llvm/unittests/Support/CommandLineTest.cpp | 6 ++++ llvm/unittests/Support/Path.cpp | 48 ++++++++++++++++++++++++++++-- llvm/unittests/Support/ProgramTest.cpp | 15 ++++++++-- 6 files changed, 90 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc index f4c2628..b15e71a 100644 --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -74,6 +74,11 @@ std::error_code widenPath(const Twine &Path8, SmallVectorImpl &Path16, SmallString Path8Str; Path8.toVector(Path8Str); + // If the path is a long path, mangled into forward slashes, normalize + // back to backslashes here. + if (Path8Str.startswith("//?/")) + llvm::sys::path::native(Path8Str, path::Style::windows_backslash); + if (std::error_code EC = UTF8ToUTF16(Path8Str, Path16)) return EC; @@ -100,8 +105,10 @@ std::error_code widenPath(const Twine &Path8, SmallVectorImpl &Path16, } // Remove '.' and '..' because long paths treat these as real path components. + // Explicitly use the backslash form here, as we're prepending the \\?\ + // prefix. llvm::sys::path::native(Path8Str, path::Style::windows); - llvm::sys::path::remove_dots(Path8Str, true); + llvm::sys::path::remove_dots(Path8Str, true, path::Style::windows); const StringRef RootName = llvm::sys::path::root_name(Path8Str); assert(!RootName.empty() && @@ -145,6 +152,7 @@ std::string getMainExecutable(const char *argv0, void *MainExecAddr) { if (UTF16ToUTF8(PathName.data(), PathName.size(), PathNameUTF8)) return ""; + llvm::sys::path::make_preferred(PathNameUTF8); return std::string(PathNameUTF8.data()); } @@ -207,7 +215,13 @@ std::error_code current_path(SmallVectorImpl &result) { // On success, GetCurrentDirectoryW returns the number of characters not // including the null-terminator. cur_path.set_size(len); - return UTF16ToUTF8(cur_path.begin(), cur_path.size(), result); + + if (std::error_code EC = + UTF16ToUTF8(cur_path.begin(), cur_path.size(), result)) + return EC; + + llvm::sys::path::make_preferred(result); + return std::error_code(); } std::error_code set_current_path(const Twine &path) { @@ -388,7 +402,11 @@ static std::error_code realPathFromHandle(HANDLE H, } // Convert the result from UTF-16 to UTF-8. - return UTF16ToUTF8(Data, CountChars, RealPath); + if (std::error_code EC = UTF16ToUTF8(Data, CountChars, RealPath)) + return EC; + + llvm::sys::path::make_preferred(RealPath); + return std::error_code(); } std::error_code is_local(int FD, bool &Result) { @@ -1407,6 +1425,8 @@ static bool getKnownFolderPath(KNOWNFOLDERID folderId, bool ok = !UTF16ToUTF8(path, ::wcslen(path), result); ::CoTaskMemFree(path); + if (ok) + llvm::sys::path::make_preferred(result); return ok; } @@ -1467,6 +1487,7 @@ void system_temp_directory(bool ErasedOnReboot, SmallVectorImpl &Result) { // Fall back to a system default. const char *DefaultResult = "C:\\Temp"; Result.append(DefaultResult, DefaultResult + strlen(DefaultResult)); + llvm::sys::path::make_preferred(Result); } } // end namespace path diff --git a/llvm/lib/Support/Windows/Process.inc b/llvm/lib/Support/Windows/Process.inc index a0d94e6..6732063 100644 --- a/llvm/lib/Support/Windows/Process.inc +++ b/llvm/lib/Support/Windows/Process.inc @@ -261,6 +261,7 @@ windows::GetCommandLineArguments(SmallVectorImpl &Args, EC = GetExecutableName(Filename); if (EC) return EC; + sys::path::make_preferred(Arg0); sys::path::append(Arg0, Filename); Args[0] = Saver.save(Arg0).data(); return std::error_code(); diff --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc index 824834c..a9cf2db 100644 --- a/llvm/lib/Support/Windows/Program.inc +++ b/llvm/lib/Support/Windows/Program.inc @@ -103,6 +103,7 @@ ErrorOr sys::findProgramByName(StringRef Name, if (U8Result.empty()) return mapWindowsError(::GetLastError()); + llvm::sys::path::make_preferred(U8Result); return std::string(U8Result.begin(), U8Result.end()); } diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index d8fd6f6..db7255e 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -1112,6 +1112,11 @@ TEST(CommandLineTest, PositionalEatArgsError) { } #ifdef _WIN32 +void checkSeparators(StringRef Path) { + char UndesiredSeparator = sys::path::get_separator()[0] == '/' ? '\\' : '/'; + ASSERT_EQ(Path.find(UndesiredSeparator), StringRef::npos); +} + TEST(CommandLineTest, GetCommandLineArguments) { int argc = __argc; char **argv = __argv; @@ -1121,6 +1126,7 @@ TEST(CommandLineTest, GetCommandLineArguments) { EXPECT_EQ(llvm::sys::path::is_absolute(argv[0]), llvm::sys::path::is_absolute(__argv[0])); + checkSeparators(argv[0]); EXPECT_TRUE( llvm::sys::path::filename(argv[0]).equals_insensitive("supporttests.exe")) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index cbde1f1..35c1de7 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -64,6 +64,13 @@ using namespace llvm::sys; namespace { +void checkSeparators(StringRef Path) { +#ifdef _WIN32 + char UndesiredSeparator = sys::path::get_separator()[0] == '/' ? '\\' : '/'; + ASSERT_EQ(Path.find(UndesiredSeparator), StringRef::npos); +#endif +} + struct FileDescriptorCloser { explicit FileDescriptorCloser(int FD) : FD(FD) {} ~FileDescriptorCloser() { ::close(FD); } @@ -436,6 +443,9 @@ std::string getEnvWin(const wchar_t *Var) { ArrayRef ref{reinterpret_cast(path), pathLen * sizeof(wchar_t)}; convertUTF16ToUTF8String(ref, expected); + SmallString<32> Buf(expected); + path::make_preferred(Buf); + expected.assign(Buf.begin(), Buf.end()); } return expected; } @@ -583,9 +593,15 @@ TEST(Support, TempDirectory) { #ifdef _WIN32 static std::string path2regex(std::string Path) { size_t Pos = 0; + bool Forward = path::get_separator()[0] == '/'; while ((Pos = Path.find('\\', Pos)) != std::string::npos) { - Path.replace(Pos, 1, "\\\\"); - Pos += 2; + if (Forward) { + Path.replace(Pos, 1, "/"); + Pos += 1; + } else { + Path.replace(Pos, 1, "\\\\"); + Pos += 2; + } } return Path; } @@ -732,10 +748,12 @@ TEST_F(FileSystemTest, RealPath) { // how we specified it. Make sure to compare against the real_path of the // TestDirectory, and not just the value of TestDirectory. ASSERT_NO_ERROR(fs::real_path(TestDirectory, RealBase)); + checkSeparators(RealBase); path::native(Twine(RealBase) + "/test1/test2", Expected); ASSERT_NO_ERROR(fs::real_path( Twine(TestDirectory) + "/././test1/../test1/test2/./test3/..", Actual)); + checkSeparators(Actual); EXPECT_EQ(Expected, Actual); @@ -744,7 +762,9 @@ TEST_F(FileSystemTest, RealPath) { // This can fail if $HOME is not set and getpwuid fails. bool Result = llvm::sys::path::home_directory(HomeDir); if (Result) { + checkSeparators(HomeDir); ASSERT_NO_ERROR(fs::real_path(HomeDir, Expected)); + checkSeparators(Expected); ASSERT_NO_ERROR(fs::real_path("~", Actual, true)); EXPECT_EQ(Expected, Actual); ASSERT_NO_ERROR(fs::real_path("~/", Actual, true)); @@ -2266,6 +2286,30 @@ TEST_F(FileSystemTest, widenPath) { ASSERT_NO_ERROR(windows::widenPath(Input, Result)); EXPECT_EQ(Result, Expected); + // Pass a path with forward slashes, check that it ends up with + // backslashes when widened with the long path prefix. + SmallString InputForward(Input); + path::make_preferred(InputForward, path::Style::windows_slash); + ASSERT_NO_ERROR(windows::widenPath(InputForward, Result)); + EXPECT_EQ(Result, Expected); + + // Pass a path which has the long path prefix prepended originally, but + // which is short enough to not require the long path prefix. If such a + // path is passed with forward slashes, make sure it gets normalized to + // backslashes. + SmallString PrefixedPath("\\\\?\\C:\\foldername"); + ASSERT_NO_ERROR(windows::UTF8ToUTF16(PrefixedPath, Expected)); + // Mangle the input to forward slashes. + path::make_preferred(PrefixedPath, path::Style::windows_slash); + ASSERT_NO_ERROR(windows::widenPath(PrefixedPath, Result)); + EXPECT_EQ(Result, Expected); + + // A short path with an inconsistent prefix is passed through as-is; this + // is a degenerate case that we currently don't care about handling. + PrefixedPath.assign("/\\?/C:/foldername"); + ASSERT_NO_ERROR(windows::UTF8ToUTF16(PrefixedPath, Expected)); + ASSERT_NO_ERROR(windows::widenPath(PrefixedPath, Result)); + EXPECT_EQ(Result, Expected); // Test that UNC paths are handled correctly. const std::string ShareName("\\\\sharename\\"); diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp index 98eb81c..d899026 100644 --- a/llvm/unittests/Support/ProgramTest.cpp +++ b/llvm/unittests/Support/ProgramTest.cpp @@ -110,17 +110,26 @@ protected: }; #ifdef _WIN32 +void checkSeparators(StringRef Path) { + char UndesiredSeparator = sys::path::get_separator()[0] == '/' ? '\\' : '/'; + ASSERT_EQ(Path.find(UndesiredSeparator), StringRef::npos); +} + TEST_F(ProgramEnvTest, CreateProcessLongPath) { if (getenv("LLVM_PROGRAM_TEST_LONG_PATH")) exit(0); // getMainExecutable returns an absolute path; prepend the long-path prefix. - std::string MyAbsExe = - sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1); + SmallString<128> MyAbsExe( + sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1)); + checkSeparators(MyAbsExe); + // Force a path with backslashes, when we are going to prepend the \\?\ + // prefix. + sys::path::native(MyAbsExe, sys::path::Style::windows_backslash); std::string MyExe; if (!StringRef(MyAbsExe).startswith("\\\\?\\")) MyExe.append("\\\\?\\"); - MyExe.append(MyAbsExe); + MyExe.append(std::string(MyAbsExe.begin(), MyAbsExe.end())); StringRef ArgV[] = {MyExe, "--gtest_filter=ProgramEnvTest.CreateProcessLongPath"}; -- 2.7.4