Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 119 additions & 34 deletions lib/Basic/PlatformUtility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 SetCurrentDirectoryW(wFileName.data());
#else
return ::chdir(fileName) == 0;
#endif
Expand All @@ -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,
Expand Down Expand Up @@ -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 CreateDirectoryW(wfilename.data(), NULL) != 0;
#else
return ::mkdir(fileName, S_IRWXU | S_IRWXG | S_IRWXO) == 0;
#endif
Expand Down Expand Up @@ -164,15 +171,72 @@ 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;
}
if (RemoveDirectoryW(wpath.data())) {
return 0;
}
int err = GetLastError();
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
errno = ENOENT;
} else if (err == ERROR_ACCESS_DENIED) {
errno = EACCES;
} else if (err == ERROR_DIR_NOT_EMPTY) {
errno = ENOTEMPTY;
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary } else {. Just use } and the rest of the body can follow.

errno = EINVAL;
}
return -1;
#else
return ::rmdir(path);
#endif
}

int sys::stat(const char *fileName, StatStruct *buf) {
#if defined(_WIN32)
return ::_stat(fileName, buf);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@compnerd Would it be simpler to use _wstat, _wrmdir, etc., vs the Win32 functions? They support long paths too, as they are just wrappers around Win32 anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there would need some char to wchar_t conversion here to use those, so Im not sure it saves that much since the widePath API does this already.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might've misunderstood what I meant. I was suggesting that instead of writing thirty lines of code, you simply call:

llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
if (llvm::sys::path::widenPath(fileName, wfilename)) {
    errno = EINVAL;
    return -1;
}
return ::_wstat(wfilename, buf);

...as opposed to GetFileAttributesExW and all the conversion ceremony that follows. I'm also concerned this is subtly changing behavior as you're filling in the stat info differently from how _stat and _wstat do it, now.

I think you should:

  • Switch CreateDirectoryW to _wmkdir
  • Switch RemoveDirectoryW to _wrmdir
  • Switch GetFileAttributesExW to _wstat
  • Switch DeleteFileW to _wunlink

That will ensure error handling and behavior is consistent with previously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these _w functions accept the //?/ prefixed paths?

Copy link
Member

@compnerd compnerd Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Microsoft implementation does support the NT style paths I believe but they are disjoint from Win32. e.g. usingSetEnvironmentVariableW and wgetenv would give unexpected results. I think that using win32 might unify some behaviors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these _w functions accept the //?/ prefixed paths?

Like I said, they just wrap the underlying Win32 functions with a little ceremony on top. So by definition, yes. e.g. the implementation of _wmkdir literally just calls CreateDirectoryW and does nothing but that other than set errno to an appropriate value based on GetLastError.

ucrt's source is also available in the Windows SDK and you can inspect it.

e.g. usingSetEnvironmentVariableW and wgetenv would give unexpected results.

Environment is a little different than the other functions.

llvm::SmallVector<wchar_t, MAX_PATH> wfilename;
if (llvm::sys::path::widenPath(fileName, wfilename)) {
errno = EINVAL;
return -1;
}

WIN32_FILE_ATTRIBUTE_DATA fileData;
if (!GetFileAttributesExW(wfilename.data(), GetFileExInfoStandard, &fileData)) {
int err = GetLastError();
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
errno = ENOENT;
} else if (err == ERROR_ACCESS_DENIED) {
errno = EACCES;
} else {
errno = EINVAL;
}
return -1;
}

// Fill the stat structure
buf->st_gid = 0;
buf->st_atime = filetimeToTime_t(fileData.ftLastAccessTime);
buf->st_ctime = filetimeToTime_t(fileData.ftCreationTime);
buf->st_dev = 0;
buf->st_ino = 0;
buf->st_rdev = 0;
buf->st_mode = (fileData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? _S_IFDIR : _S_IFREG;
buf->st_mode |= (fileData.dwFileAttributes & FILE_ATTRIBUTE_READONLY) ? _S_IREAD : _S_IREAD | _S_IWRITE;

llvm::StringRef extension = llvm::sys::path::extension(llvm::StringRef(fileName));
if (extension == ".exe" || extension == ".cmd" || extension == ".bat" || extension == ".com") {
buf->st_mode |= _S_IEXEC;
}

buf->st_mtime = filetimeToTime_t(fileData.ftLastWriteTime);
buf->st_nlink = 1;
buf->st_size = ((long long)fileData.nFileSizeHigh << 32) | fileData.nFileSizeLow;
buf->st_uid = 0;

return 0;
#else
return ::stat(fileName, buf);
#endif
Expand All @@ -181,19 +245,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);
Expand All @@ -202,7 +268,23 @@ 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;
}
if (DeleteFileW(wfilename.data())) {
return 0;
}
int err = GetLastError();
if (err == ERROR_FILE_NOT_FOUND) {
errno = ENOENT;
} else if (err == ERROR_ACCESS_DENIED) {
errno = EACCES;
} else {
errno = EINVAL;
}
return -1;
#else
return ::unlink(fileName);
#endif
Expand Down Expand Up @@ -263,14 +345,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);
Expand Down Expand Up @@ -342,9 +425,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")) {
Expand All @@ -371,15 +462,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
Expand All @@ -401,4 +487,3 @@ void sys::CloseLibrary(sys::ModuleTraits<>::Handle handle) {
dlclose(handle);
#endif
}

34 changes: 27 additions & 7 deletions src/llbuild3/LocalExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,27 +246,47 @@ std::string formatWindowsCommandString(std::vector<std::string> args) {

std::error_code checkExecutable(const std::filesystem::path& path) {

#if defined(_WIN32)
llvm::SmallVector<wchar_t, MAX_PATH> wpath;
if (llvm::sys::path::widenPath(path.c_str(), wpath)) {
return std::make_error_code(std::errc::invalid_argument);
}

DWORD attributes = GetFileAttributesW(wpath.data());
if (attributes == INVALID_FILE_ATTRIBUTES) {
DWORD err = GetLastError();
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
return std::make_error_code(std::errc::no_such_file_or_directory);
} else if (err == ERROR_ACCESS_DENIED) {
return std::make_error_code(std::errc::permission_denied);
} else {
return std::error_code(err, std::system_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)
struct ::_stat buf;
WIN32_FILE_ATTRIBUTE_DATA fileData;
if (!GetFileAttributesExW(wpath.data(), GetFileExInfoStandard, &fileData)) {
return std::make_error_code(std::errc::permission_denied);
}

if (fileData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
return std::make_error_code(std::errc::permission_denied);
#else
struct ::stat buf;
#endif

#if defined(_WIN32)
if (0 != ::_stat(path.c_str(), &buf)) {
#else
if (0 != ::stat(path.c_str(), &buf)) {
#endif
return std::make_error_code(std::errc::permission_denied);
}

if (!S_ISREG(buf.st_mode))
return std::make_error_code(std::errc::permission_denied);
#endif

return std::error_code();
}
Expand Down