use widePath and _w variants on Windows for the file APIs

Author: daveinglis

lib/Basic/PlatformUtility.cpp

@@ -38,9 +38,10 @@ using namespace llbuild::basic;
 
 bool sys::chdir(const char *fileName) {
 #if defined(_WIN32)
-  llvm::SmallVector<llvm::UTF16, 20> wFileName;
-  llvm::convertUTF8ToUTF16String(fileName, wFileName);
-  return SetCurrentDirectoryW((LPCWSTR)wFileName.data());
+  llvm::SmallVector<wchar_t, MAX_PATH> wFileName;
+  if (llvm::sys::path::widenPath(fileName, wFileName))
+    return false;
+  return ::_wchdir(wFileName.data()) == 0;
 #else
   return ::chdir(fileName) == 0;
 #endif
@@ -63,10 +64,13 @@ time_t filetimeToTime_t(FILETIME ft) {
 
 int sys::lstat(const char *fileName, sys::StatStruct *buf) {
 #if defined(_WIN32)
-  llvm::SmallVector<llvm::UTF16, 20> wfilename;
-  llvm::convertUTF8ToUTF16String(fileName, wfilename);
+  llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
+  if (llvm::sys::path::widenPath(fileName, wfilename)) {
+    errno = EINVAL;
+    return -1;
+  }
   HANDLE h = CreateFileW(
-      /*lpFileName=*/(LPCWSTR)wfilename.data(),
+      /*lpFileName=*/wfilename.data(),
       /*dwDesiredAccess=*/0,
       /*dwShareMode=*/FILE_SHARE_READ,
       /*lpSecurityAttributes=*/NULL,
@@ -123,7 +127,10 @@ int sys::lstat(const char *fileName, sys::StatStruct *buf) {
 
 bool sys::mkdir(const char* fileName) {
 #if defined(_WIN32)
-  return _mkdir(fileName) == 0;
+  llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
+  if (llvm::sys::path::widenPath(fileName, wfilename))
+    return false;
+  return ::_wmkdir(wfilename.data()) == 0;
 #else
   return ::mkdir(fileName, S_IRWXU | S_IRWXG |  S_IRWXO) == 0;
 #endif
@@ -164,15 +171,25 @@ int sys::read(int fileHandle, void *destinationBuffer,
 
 int sys::rmdir(const char *path) {
 #if defined(_WIN32)
-  return ::_rmdir(path);
+  llvm::SmallVector<wchar_t, MAX_PATH> wpath;
+  if (llvm::sys::path::widenPath(path, wpath)) {
+    errno = EINVAL;
+    return -1;
+  }
+  return ::_wrmdir(wpath.data());
 #else
   return ::rmdir(path);
 #endif
 }
 
 int sys::stat(const char *fileName, StatStruct *buf) {
 #if defined(_WIN32)
-  return ::_stat(fileName, buf);
+  llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
+  if (llvm::sys::path::widenPath(fileName, wfilename)) {
+    errno = EINVAL;
+    return -1;
+  }
+  return ::_wstat(wfilename.data(), buf);
 #else
   return ::stat(fileName, buf);
 #endif
@@ -181,19 +198,21 @@ int sys::stat(const char *fileName, StatStruct *buf) {
 // Create a symlink named linkPath which contains the string pointsTo
 int sys::symlink(const char *pointsTo, const char *linkPath) {
 #if defined(_WIN32)
-  llvm::SmallVector<llvm::UTF16, 20> wPointsTo;
-  llvm::convertUTF8ToUTF16String(pointsTo, wPointsTo);
-  llvm::SmallVector<llvm::UTF16, 20> wLinkPath;
-  llvm::convertUTF8ToUTF16String(linkPath, wLinkPath);
-  DWORD attributes = GetFileAttributesW((LPCWSTR)wPointsTo.data());
+  llvm::SmallVector<wchar_t, MAX_PATH> wPointsTo;
+  if (llvm::sys::path::widenPath(pointsTo, wPointsTo))
+    return -1;
+  llvm::SmallVector<wchar_t, MAX_PATH> wLinkPath;
+  if (llvm::sys::path::widenPath(linkPath, wLinkPath))
+    return -1;
+  DWORD attributes = GetFileAttributesW(wPointsTo.data());
   DWORD directoryFlag = (attributes != INVALID_FILE_ATTRIBUTES &&
                          attributes & FILE_ATTRIBUTE_DIRECTORY)
                             ? SYMBOLIC_LINK_FLAG_DIRECTORY
                             : 0;
   // Note that CreateSymbolicLinkW takes its arguments in reverse order
   // compared to symlink/_symlink
   return !::CreateSymbolicLinkW(
-      (LPCWSTR)wLinkPath.data(), (LPCWSTR)wPointsTo.data(),
+      wLinkPath.data(), wPointsTo.data(),
       SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE | directoryFlag);
 #else
   return ::symlink(pointsTo, linkPath);
@@ -202,7 +221,12 @@ int sys::symlink(const char *pointsTo, const char *linkPath) {
 
 int sys::unlink(const char *fileName) {
 #if defined(_WIN32)
-  return ::_unlink(fileName);
+  llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
+  if (llvm::sys::path::widenPath(fileName, wfilename)) {
+    errno = EINVAL;
+    return -1;
+  }
+  return ::_wunlink(wfilename.data());
 #else
   return ::unlink(fileName);
 #endif
@@ -263,14 +287,15 @@ int sys::raiseOpenFileLimit(llbuild_rlim_t limit) {
 sys::MATCH_RESULT sys::filenameMatch(const std::string& pattern,
                                      const std::string& filename) {
 #if defined(_WIN32)
-  llvm::SmallVector<llvm::UTF16, 20> wpattern;
-  llvm::SmallVector<llvm::UTF16, 20> wfilename;
+  llvm::SmallVector<wchar_t, MAX_PATH> wpattern;
+  llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
 
-  llvm::convertUTF8ToUTF16String(pattern, wpattern);
-  llvm::convertUTF8ToUTF16String(filename, wfilename);
+  if (llvm::sys::path::widenPath(pattern, wpattern) ||
+      llvm::sys::path::widenPath(filename, wfilename))
+    return sys::MATCH_ERROR;
 
   bool result =
-      PathMatchSpecW((LPCWSTR)wfilename.data(), (LPCWSTR)wpattern.data());
+      PathMatchSpecW(wfilename.data(), wpattern.data());
   return result ? sys::MATCH : sys::NO_MATCH;
 #else
   int result = fnmatch(pattern.c_str(), filename.c_str(), 0);
@@ -342,9 +367,17 @@ std::string sys::makeTmpDir() {
 #if defined(_WIN32)
   char path[MAX_PATH];
   tmpnam_s(path, MAX_PATH);
-  llvm::SmallVector<llvm::UTF16, 20> wPath;
-  llvm::convertUTF8ToUTF16String(path, wPath);
-  CreateDirectoryW((LPCWSTR)wPath.data(), NULL);
+  llvm::SmallVector<wchar_t, MAX_PATH> wPath;
+  if (llvm::sys::path::widenPath(path, wPath))
+    return std::string();
+  if (!CreateDirectoryW(wPath.data(), NULL)) {
+    DWORD error = GetLastError();
+    if (error != ERROR_ALREADY_EXISTS) {
+      fprintf(stderr, "Failed to create temporary directory '%s': error code %lu\n",
+              path, (unsigned long)error);
+      return std::string();
+    }
+  } 
   return std::string(path);
 #else
   if (const char *tmpDir = std::getenv("TMPDIR")) {
@@ -371,15 +404,10 @@ std::string sys::getPathSeparators() {
 
 sys::ModuleTraits<>::Handle sys::OpenLibrary(const char *path) {
 #if defined(_WIN32)
-  int cchLength =
-      MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, path, strlen(path),
-                          nullptr, 0);
-  std::u16string buffer(cchLength + 1, 0);
-  MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, path, strlen(path),
-                      const_cast<LPWSTR>(reinterpret_cast<LPCWSTR>(buffer.data())),
-                      buffer.size());
-
-  return LoadLibraryW(reinterpret_cast<LPCWSTR>(buffer.data()));
+  llvm::SmallVector<wchar_t, MAX_PATH> wPath;
+  if (llvm::sys::path::widenPath(path, wPath))
+    return nullptr;
+  return LoadLibraryW(wPath.data());
 #else
   return dlopen(path, RTLD_LAZY);
 #endif

src/llbuild3/LocalExecutor.cpp

@@ -246,9 +246,19 @@ std::string formatWindowsCommandString(std::vector<std::string> args) {
 
 std::error_code checkExecutable(const std::filesystem::path& path) {
 
+#if defined(_WIN32)
+  llvm::SmallVector<wchar_t, 128> wpath;
+  if (llvm::sys::path::widenPath(path.c_str(), wpath)) {
+    return std::make_error_code(std::errc::invalid_argument);
+  }
+  if (::_waccess(path.c_str(), R_OK | X_OK) == -1) {
+    return std::error_code(errno, std::generic_category());
+  }
+#else
   if (::access(path.c_str(), R_OK | X_OK) == -1) {
     return std::error_code(errno, std::generic_category());
   }
+#endif
 
   // Don't say that directories are executable.
 #if defined(_WIN32)
@@ -258,7 +268,7 @@ std::error_code checkExecutable(const std::filesystem::path& path) {
 #endif
 
 #if defined(_WIN32)
-  if (0 != ::_stat(path.c_str(), &buf)) {
+  if (0 != ::_wstat(wpath.data(), &buf)) {
 #else
   if (0 != ::stat(path.c_str(), &buf)) {
 #endif

unittests/Basic/CMakeLists.txt

@@ -16,3 +16,18 @@ if(NOT ${CMAKE_SYSTEM_NAME} STREQUAL "Windows")
   target_link_libraries(BasicTests PRIVATE
     curses)
 endif()
+
+# Add widenPath performance benchmark (Windows only)
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Windows")
+  add_executable(widenPathBenchmark
+    widenPathBenchmark.cpp
+    )
+  
+  target_link_libraries(widenPathBenchmark PRIVATE
+    llbuildBasic
+    llvmSupport)
+  
+  # Set output directory to make it easy to find
+  set_target_properties(widenPathBenchmark PROPERTIES
+    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
+endif()

unittests/Basic/FileSystemTest.cpp

@@ -24,6 +24,9 @@
 
 #include "gtest/gtest.h"
 
+#include <chrono>
+#include <vector>
+
 using namespace llbuild;
 using namespace llbuild::basic;
 
@@ -222,4 +225,132 @@ TEST(ChecksumOnlyFileSystem, basic) {
   EXPECT_FALSE(ec);
 }
 
+#if defined(_WIN32)
+TEST(FileSystemTest, widenPathPerformance) {
+  // Performance test for widenPath function on Windows
+  using namespace std::chrono;
+  
+  // Test data: various path types that exercise different code paths
+  std::vector<std::string> testPaths = {
+    // Short absolute paths (should hit early return optimization)
+    "C:\\Windows\\System32\\kernel32.dll",
+    "C:\\Program Files\\test.exe",
+    "D:\\temp\\file.txt",
+    
+    // Relative paths (require current directory lookup)
+    "..\\..\\test.txt",
+    "src\\main.cpp",
+    "build\\debug\\output.exe",
+    
+    // Long paths that require \\?\ prefix
+    "C:\\very\\long\\path\\that\\exceeds\\the\\normal\\MAX_PATH\\limit\\and\\requires\\special\\handling\\with\\the\\long\\path\\prefix\\to\\work\\correctly\\on\\windows\\systems\\file.txt",
+    
+    // Paths with . and .. components (require canonicalization)
+    "C:\\Windows\\..\\Program Files\\..\\Windows\\System32\\kernel32.dll",
+    "..\\src\\..\\build\\..\\src\\main.cpp",
+    
+    // UNC paths
+    "\\\\server\\share\\file.txt",
+    
+    // Already prefixed paths
+    "\\\\?\\C:\\already\\prefixed\\path.txt"
+  };
+  
+  const int iterations = 1000;
+  
+  // Warm up
+  for (const auto& path : testPaths) {
+    llvm::SmallVector<wchar_t, 260> result;
+    llvm::sys::path::widenPath(path, result);
+  }
+  
+  // Measure performance
+  auto start = high_resolution_clock::now();
+  
+  for (int i = 0; i < iterations; ++i) {
+    for (const auto& path : testPaths) {
+      llvm::SmallVector<wchar_t, 260> result;
+      std::error_code ec = llvm::sys::path::widenPath(path, result);
+      EXPECT_FALSE(ec) << "Failed to widen path: " << path;
+    }
+  }
+  
+  auto end = high_resolution_clock::now();
+  auto duration = duration_cast<microseconds>(end - start);
+  
+  double avgTimePerCall = static_cast<double>(duration.count()) / (iterations * testPaths.size());
+  
+  // Print performance results
+  std::cout << "\nwidenPath Performance Results:\n";
+  std::cout << "Total iterations: " << iterations << "\n";
+  std::cout << "Test paths: " << testPaths.size() << "\n";
+  std::cout << "Total calls: " << (iterations * testPaths.size()) << "\n";
+  std::cout << "Total time: " << duration.count() << " microseconds\n";
+  std::cout << "Average time per call: " << avgTimePerCall << " microseconds\n";
+  
+  // Performance expectations (these are reasonable targets)
+  EXPECT_LT(avgTimePerCall, 50.0) << "widenPath is taking too long on average";
+  
+  // Test correctness of results
+  for (const auto& path : testPaths) {
+    llvm::SmallVector<wchar_t, 260> result;
+    std::error_code ec = llvm::sys::path::widenPath(path, result);
+    EXPECT_FALSE(ec) << "widenPath failed for: " << path;
+    EXPECT_GT(result.size(), 0) << "widenPath returned empty result for: " << path;
+  }
+}
+
+TEST(FileSystemTest, widenPathCorrectnessAndOptimizations) {
+  // Test that optimizations don't break correctness
+  
+  // Test early return for short absolute paths
+  {
+    std::string shortAbsPath = "C:\\Windows\\System32\\test.exe";
+    llvm::SmallVector<wchar_t, 260> result;
+    std::error_code ec = llvm::sys::path::widenPath(shortAbsPath, result);
+    EXPECT_FALSE(ec);
+    EXPECT_GT(result.size(), 0);
+    
+    // Convert back to check correctness
+    llvm::SmallVector<char, 260> backConverted;
+    llvm::sys::windows::UTF16ToUTF8(result.data(), result.size(), backConverted);
+    std::string converted(backConverted.data(), backConverted.size());
+    EXPECT_EQ(converted, shortAbsPath);
+  }
+  
+  // Test relative path handling
+  {
+    std::string relPath = "..\\test.txt";
+    llvm::SmallVector<wchar_t, 260> result;
+    std::error_code ec = llvm::sys::path::widenPath(relPath, result);
+    EXPECT_FALSE(ec);
+    EXPECT_GT(result.size(), 0);
+  }
+  
+  // Test long path with \\?\ prefix
+  {
+    std::string longPath = "C:\\very\\long\\path\\that\\exceeds\\the\\normal\\MAX_PATH\\limit\\and\\requires\\special\\handling\\with\\the\\long\\path\\prefix\\to\\work\\correctly\\on\\windows\\systems\\and\\should\\trigger\\the\\long\\path\\support\\mechanism\\file.txt";
+    llvm::SmallVector<wchar_t, 1024> result;
+    std::error_code ec = llvm::sys::path::widenPath(longPath, result);
+    EXPECT_FALSE(ec);
+    EXPECT_GT(result.size(), 0);
+    
+    // Should have \\?\ prefix for long paths
+    if (result.size() >= 4) {
+      std::wstring prefix(result.data(), 4);
+      EXPECT_EQ(prefix, L"\\\\?\\") << "Long path should have \\\\?\\ prefix";
+    }
+  }
+  
+  // Test path canonicalization
+  {
+    std::string pathWithDots = "C:\\Windows\\..\\Program Files\\..\\Windows\\System32\\kernel32.dll";
+    llvm::SmallVector<wchar_t, 1024> result;
+    std::error_code ec = llvm::sys::path::widenPath(pathWithDots, result);
+    EXPECT_FALSE(ec);
+    EXPECT_GT(result.size(), 0);
+  }
+}
+#endif // _WIN32
+
 }