https://github.com/jtstogel updated https://github.com/llvm/llvm-project/pull/209370
>From d921be35acebfa3a1db9683763a758ea02d6d78d Mon Sep 17 00:00:00 2001 From: jtstogel <[email protected]> Date: Mon, 13 Jul 2026 20:41:17 -0700 Subject: [PATCH 1/2] [libc][realpath][test] Create test files/directories Follow-up PRs will add path validation etc to `realpath`, which will require paths to exist in the filesystem. --- libc/test/UnitTest/HermeticTestUtils.cpp | 2 +- libc/test/src/stdlib/CMakeLists.txt | 13 ++ libc/test/src/stdlib/realpath_test.cpp | 250 +++++++++++++++++++++-- 3 files changed, 252 insertions(+), 13 deletions(-) diff --git a/libc/test/UnitTest/HermeticTestUtils.cpp b/libc/test/UnitTest/HermeticTestUtils.cpp index 3c82d1e963643..6fa1b0177f00b 100644 --- a/libc/test/UnitTest/HermeticTestUtils.cpp +++ b/libc/test/UnitTest/HermeticTestUtils.cpp @@ -45,7 +45,7 @@ namespace { // requires. Hence, as a work around for this problem, we use a simple allocator // which just hands out continuous blocks from a statically allocated chunk of // memory. -static constexpr uint64_t MEMORY_SIZE = 65336; +static constexpr uint64_t MEMORY_SIZE = 1 << 20; // 1 MiB alignas(ALIGNMENT) static uint8_t memory[MEMORY_SIZE]; static uint8_t *ptr = memory; diff --git a/libc/test/src/stdlib/CMakeLists.txt b/libc/test/src/stdlib/CMakeLists.txt index 0101386f5b2fc..bc55bc502f310 100644 --- a/libc/test/src/stdlib/CMakeLists.txt +++ b/libc/test/src/stdlib/CMakeLists.txt @@ -385,13 +385,26 @@ add_libc_test( realpath_test.cpp DEPENDS libc.hdr.errno_macros + libc.hdr.func.free libc.hdr.limits_macros libc.hdr.types.size_t libc.src.__support.CPP.string + libc.src.__support.CPP.string_view + libc.src.__support.CPP.utility + libc.src.__support.fixedvector + libc.src.__support.libc_assert + libc.src.__support.libc_errno libc.src.__support.macros.config libc.src.__support.OSUtil.path + libc.src.fcntl.openat libc.src.stdlib.realpath + libc.src.string.strdup + libc.src.sys.stat.mkdirat + libc.src.unistd.close + libc.src.unistd.getpid + libc.src.unistd.unlinkat libc.test.UnitTest.ErrnoCheckingTest + libc.test.UnitTest.ErrnoSetterMatcher ) add_libc_test( diff --git a/libc/test/src/stdlib/realpath_test.cpp b/libc/test/src/stdlib/realpath_test.cpp index a73bed334f506..30fde61817eb6 100644 --- a/libc/test/src/stdlib/realpath_test.cpp +++ b/libc/test/src/stdlib/realpath_test.cpp @@ -12,23 +12,148 @@ //===----------------------------------------------------------------------===// #include "hdr/errno_macros.h" +#include "hdr/func/free.h" #include "hdr/limits_macros.h" #include "hdr/types/size_t.h" #include "src/__support/CPP/string.h" +#include "src/__support/CPP/string_view.h" +#include "src/__support/CPP/utility.h" #include "src/__support/OSUtil/path.h" +#include "src/__support/fixedvector.h" +#include "src/__support/libc_assert.h" +#include "src/__support/libc_errno.h" #include "src/__support/macros/config.h" +#include "src/fcntl/openat.h" #include "src/stdlib/realpath.h" +#include "src/string/strdup.h" +#include "src/sys/stat/mkdirat.h" +#include "src/unistd/close.h" +#include "src/unistd/getpid.h" +#include "src/unistd/unlinkat.h" #include "test/UnitTest/ErrnoCheckingTest.h" +#include "test/UnitTest/ErrnoSetterMatcher.h" namespace cpp = LIBC_NAMESPACE::cpp; namespace path = LIBC_NAMESPACE::path; -using LIBC_NAMESPACE::testing::ErrnoCheckingTest; +using LIBC_NAMESPACE::FixedVector; +using LIBC_NAMESPACE::testing::tlog; +using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; // This test assumes the following values, so fail early if they mismatch. static_assert(path::SEPARATOR == '/'); static_assert(path::CURRENT_DIR_COMPONENT == "."); static_assert(path::PARENT_DIR_COMPONENT == ".."); +// Size of a path separator. +constexpr size_t PATH_SEP_SIZE = 1; + +// Creates the given directory if it does not exist. Returns zero on success. +[[nodiscard]] int ensure_directory_exists(int dirfd, const char *path, + mode_t mode = 0755) { + if (LIBC_NAMESPACE::mkdirat(dirfd, path, mode) == 0) + return 0; + if (libc_errno == EEXIST) + libc_errno = 0; + + if (libc_errno != 0) { + tlog << "Failed to create temp directory: " << path << "\n"; + return -1; + } + return 0; +} + +// A test directory that removes itself on destruction. +class TestDir { + // The test directory's absolute path. + cpp::string path_; + + // File descriptor of the test directory. + int fd_ = -1; + + // Files created in this directory tree. + // Use a C string instead of cpp::string because FixedVector + // only supports trivially destructable types. + FixedVector<char *, 64> files_; + + // Subdirectories created in this directory tree. + FixedVector<char *, 64> dirs_; + +public: + TestDir() = default; + + // Initializes this TestDir container with the given path. + void initialize(cpp::string directory_path, int dirfd) { + LIBC_ASSERT(fd_ == -1); + path_ = directory_path; + fd_ = dirfd; + } + + ~TestDir() { + if (path_.empty()) + return; + + for (size_t i = 0; i < files_.size(); i++) { + LIBC_NAMESPACE::unlinkat(fd_, files_[i], 0); + ::free(files_[i]); + } + + // Remove directories in reverse order so they'll be empty. + for (size_t i = dirs_.size(); i > 0; i--) { + LIBC_NAMESPACE::unlinkat(fd_, dirs_[i - 1], AT_REMOVEDIR); + ::free(dirs_[i - 1]); + } + + LIBC_NAMESPACE::close(fd_); + LIBC_NAMESPACE::unlinkat(AT_FDCWD, path_.c_str(), AT_REMOVEDIR); + } + + TestDir(TestDir &other) = delete; + TestDir &operator=(TestDir &other) = delete; + TestDir(TestDir &&other) = delete; + TestDir &operator=(TestDir &&other) = delete; + + // Returns the absolute path of `relpath` in this test directory. + cpp::string abspath(cpp::string_view relpath) const { + return path_ + "/" + relpath; + } + + // Returns this test directory path as a C string. + const char *c_str() const { return path_.c_str(); } + + // Returns this test directory path as a string view. + const cpp::string_view view() const { return path_; } + + // Creates a directory relative to TestDir. Returns zero on success. + [[nodiscard]] int mkdir(const char *relpath, mode_t mode = 0755) { + char *path = LIBC_NAMESPACE::strdup(relpath); + if (path == nullptr) + return -1; + + if (!dirs_.push_back(path)) { + tlog << "Not enough space in TestDir::dirs_\n"; + return -1; + } + + return ensure_directory_exists(fd_, relpath, mode); + } + + // Creates an empty file relative to TestDir. Returns zero on success. + [[nodiscard]] int touch(const char *relpath, mode_t mode = 0644) { + char *path = LIBC_NAMESPACE::strdup(relpath); + if (path == nullptr) + return -1; + + if (!files_.push_back(path)) { + tlog << "Not enough space in TestDir::files_\n"; + return -1; + } + int fd = LIBC_NAMESPACE::openat(fd_, relpath, O_RDONLY | O_CREAT, mode); + if (fd < 0) + return -1; + return LIBC_NAMESPACE::close(fd); + } +}; + class LlvmLibcRealpathTest : public LIBC_NAMESPACE::testing::ErrnoCheckingTest { public: char *realpath_buffered(const char *path) { @@ -39,6 +164,32 @@ class LlvmLibcRealpathTest : public LIBC_NAMESPACE::testing::ErrnoCheckingTest { return realpath_buffered(path.c_str()); } + // Creates a test directory in dst. Returns true if successful. + // + // While we would prefer to return cpp::optional<TestDir> here, + // LLVM-libc's optional expects types to be trivially destructible. + [[nodiscard]] bool create_test_dir(const char *name, TestDir &dst) { + // Use /tmp instead of the typical libc_make_test_file_path to ensure + // the path is absolute and does not contain symlinks. + cpp::string test_dir_path = "/tmp/LlvmLibcRealpathTest."; + test_dir_path += name; + test_dir_path += "."; + + // Include the pid in case multiple builds of this test are running at once. + test_dir_path += cpp::to_string(static_cast<int>(LIBC_NAMESPACE::getpid())); + + if (ensure_directory_exists(AT_FDCWD, test_dir_path.c_str())) + return false; + int fd = LIBC_NAMESPACE::openat(AT_FDCWD, test_dir_path.c_str(), + O_RDONLY | O_DIRECTORY); + if (fd < 0) + return false; + + dst.initialize(cpp::move(test_dir_path), fd); + return true; + } + +private: char buf_[PATH_MAX]; }; @@ -61,13 +212,61 @@ TEST_F(LlvmLibcRealpathTest, OkIfPathArgIsExactlyMaxSize) { ASSERT_STREQ(realpath_buffered(s), "/"); } +// Creates a test directory that has an absolute path with exactpy +// `desired_size` characters. +[[nodiscard]] bool create_abspath_with_size(TestDir &test_dir, + size_t desired_size, + cpp::string &out) { + if (desired_size < test_dir.view().size() + PATH_SEP_SIZE) { + tlog << "Test directory is already too long in create_abspath_with_size: " + << test_dir.view().size() << "\n"; + return false; + } + size_t remaining_size = desired_size - test_dir.view().size() - PATH_SEP_SIZE; + + cpp::string relpath; + relpath.reserve(remaining_size); + + while (remaining_size != 0) { + if (!relpath.empty()) { + relpath += '/'; + remaining_size -= PATH_SEP_SIZE; + } + + size_t component_size = NAME_MAX; + if (component_size > remaining_size) + component_size = remaining_size; + + // If adding a component of this size would leave us in a state where + // we only have enough space for a separator, shorten the component. + if (remaining_size - component_size == PATH_SEP_SIZE) + component_size -= 1; + + for (size_t i = 0; i < component_size; ++i) + relpath += 'a'; + remaining_size -= component_size; + + if (test_dir.mkdir(relpath.c_str())) + return false; + } + + out = test_dir.abspath(relpath); + + if (out.size() != desired_size) { + tlog << "Failed to create path of size=" << desired_size << "\n"; + return false; + } + return true; +} + TEST_F(LlvmLibcRealpathTest, OkIfResolvedPathIsExactlyMaxSize) { - // PATH_MAX counts null terminator, so construct a path of size PATH_MAX-1. - cpp::string s(PATH_MAX - 1, 'a'); - for (size_t i = 0; i < s.size() - 1; i += 8) - s[i] = '/'; + TestDir test_dir; + ASSERT_TRUE(create_test_dir("OkIfResolvedPathIsExactlyMaxSize", test_dir)); + + cpp::string path; + ASSERT_TRUE(create_abspath_with_size(test_dir, PATH_MAX - 1, path)); - ASSERT_STREQ(realpath_buffered(s), s.c_str()); + ASSERT_STREQ(realpath_buffered(path), path.c_str()); } TEST_F(LlvmLibcRealpathTest, ErrorsWithNameTooLongIfPathArgExceedsMaxSize) { @@ -89,23 +288,50 @@ TEST_F(LlvmLibcRealpathTest, RootDotDotTraversalStaysAtRoot) { } TEST_F(LlvmLibcRealpathTest, SimpleAbsolutePath) { - ASSERT_STREQ(realpath_buffered("/a/b/c"), "/a/b/c"); + TestDir test_dir; + ASSERT_TRUE(create_test_dir("SimpleAbsolutePath", test_dir)); + + ASSERT_THAT(test_dir.mkdir("a"), Succeeds()); + ASSERT_THAT(test_dir.mkdir("a/b"), Succeeds()); + + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a/b")), + test_dir.abspath("a/b").c_str()); } TEST_F(LlvmLibcRealpathTest, DotDotTraversesParent) { - ASSERT_STREQ(realpath_buffered("/a/b/.."), "/a"); + TestDir test_dir; + ASSERT_TRUE(create_test_dir("DotDotTraversesParent", test_dir)); + + ASSERT_THAT(test_dir.mkdir("a"), Succeeds()); + ASSERT_THAT(test_dir.mkdir("a/b"), Succeeds()); + + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a/b/..")), + test_dir.abspath("a").c_str()); } TEST_F(LlvmLibcRealpathTest, DotTraversalIsNop) { - ASSERT_STREQ(realpath_buffered("/a/b/./"), "/a/b"); + TestDir test_dir; + ASSERT_TRUE(create_test_dir("DotTraversalIsNop", test_dir)); + + ASSERT_THAT(test_dir.mkdir("a"), Succeeds()); + ASSERT_THAT(test_dir.mkdir("a/b"), Succeeds()); + + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a/b/./")), + test_dir.abspath("a/b").c_str()); } TEST_F(LlvmLibcRealpathTest, ConsecutiveSeparatorsIgnored) { - ASSERT_STREQ(realpath_buffered("////a///..///a//"), "/a"); + TestDir test_dir; + ASSERT_TRUE(create_test_dir("ConsecutiveSeparatorsIgnored", test_dir)); + + ASSERT_THAT(test_dir.mkdir("a"), Succeeds()); + + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a//..///a//")), + test_dir.abspath("a").c_str()); } TEST_F(LlvmLibcRealpathTest, AllocatesResultWhenBufferIsNull) { - char *result = LIBC_NAMESPACE::realpath("/a", nullptr); - ASSERT_STREQ(result, "/a"); + char *result = LIBC_NAMESPACE::realpath("/", nullptr); + ASSERT_STREQ(result, "/"); ::free(result); } >From f65f52f23ab2c0724cb0ac178dc3338c3206d1a1 Mon Sep 17 00:00:00 2001 From: jtstogel <[email protected]> Date: Fri, 10 Jul 2026 16:26:05 -0700 Subject: [PATCH 2/2] [libc][realpath] Validate path components. This PR updates `realpath` to validate paths and return `ENOTDIR` or `ENOENT` according to https://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html. This PR also bumps the memory limit in `HermeticTestUtils.cpp`. The realpath unit tests allocate quite a few strings. Since memory in hermetic tests is never free'd, the unit test quickly reaches the limit. --- libc/src/stdlib/linux/CMakeLists.txt | 5 +++ libc/src/stdlib/linux/realpath.cpp | 34 ++++++++++++++++++ libc/test/src/stdlib/realpath_test.cpp | 49 ++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/libc/src/stdlib/linux/CMakeLists.txt b/libc/src/stdlib/linux/CMakeLists.txt index 9189c985d9a27..357073f61962b 100644 --- a/libc/src/stdlib/linux/CMakeLists.txt +++ b/libc/src/stdlib/linux/CMakeLists.txt @@ -17,7 +17,10 @@ add_entrypoint_object( ../realpath.h DEPENDS libc.hdr.errno_macros + libc.hdr.fcntl_macros libc.hdr.limits_macros + libc.hdr.sys_stat_macros + libc.hdr.types.mode_t libc.hdr.types.size_t libc.src.__support.common libc.src.__support.CPP.optional @@ -26,6 +29,8 @@ add_entrypoint_object( libc.src.__support.error_or libc.src.__support.libc_errno libc.src.__support.macros.config + libc.src.__support.OSUtil.linux.stat.kernel_statx_types + libc.src.__support.OSUtil.linux.syscall_wrappers.statx libc.src.__support.OSUtil.path libc.src.string.memory_utils.inline_memcpy ) diff --git a/libc/src/stdlib/linux/realpath.cpp b/libc/src/stdlib/linux/realpath.cpp index 63e9676ad3cae..bebfc14f3d256 100644 --- a/libc/src/stdlib/linux/realpath.cpp +++ b/libc/src/stdlib/linux/realpath.cpp @@ -13,11 +13,16 @@ #include "src/stdlib/realpath.h" #include "hdr/errno_macros.h" +#include "hdr/fcntl_macros.h" #include "hdr/limits_macros.h" +#include "hdr/sys_stat_macros.h" +#include "hdr/types/mode_t.h" #include "hdr/types/size_t.h" #include "src/__support/CPP/optional.h" #include "src/__support/CPP/string.h" #include "src/__support/CPP/string_view.h" +#include "src/__support/OSUtil/linux/stat/kernel_statx_types.h" +#include "src/__support/OSUtil/linux/syscall_wrappers/statx.h" #include "src/__support/OSUtil/path.h" #include "src/__support/common.h" #include "src/__support/error_or.h" @@ -67,6 +72,8 @@ class ResolvedPath { // Must be free'd by the caller. char *release() { return path_.release_c_str(); } + const char *c_str() const { return path_.c_str(); } + // Copies the content of this path to `dst`. void copy_to(char *dst) { inline_memcpy(dst, path_.c_str(), path_.size() + 1); @@ -136,6 +143,20 @@ class PendingPath { cpp::string_view view_; }; +ErrorOr<mode_t> read_file_type(const char *path) { + internal::kernel_statx_buf buf; + ErrorOr<int> ret = + linux_syscalls::statx(AT_FDCWD, path, AT_SYMLINK_NOFOLLOW, + internal::KERNEL_STATX_TYPE_MASK, &buf); + if (!ret) + return Error(ret.error()); + + if (!(buf.stx_mask & internal::KERNEL_STATX_TYPE_MASK)) + return Error(EIO); + + return static_cast<mode_t>(buf.stx_mode); +} + cpp::optional<Error> resolve_path(PendingPath &pending_path, ResolvedPath &resolved_path) { while (!pending_path.empty()) { @@ -150,6 +171,19 @@ cpp::optional<Error> resolve_path(PendingPath &pending_path, if (cpp::optional<Error> err = resolved_path.push_component(component); err) return err; + + ErrorOr<mode_t> mode = read_file_type(resolved_path.c_str()); + if (!mode) + return Error(mode.error()); + + // TODO: Resolve symbolic links. + if (S_ISLNK(*mode)) + return Error(ENOSYS); + + // If the path is not a directory, but there is more to resolve, then error. + // For example, realpath("/path/to/file.txt/") give ENOTDIR. + if (!S_ISDIR(*mode) && !pending_path.empty()) + return Error(ENOTDIR); } return cpp::nullopt; diff --git a/libc/test/src/stdlib/realpath_test.cpp b/libc/test/src/stdlib/realpath_test.cpp index 30fde61817eb6..f0827bf260567 100644 --- a/libc/test/src/stdlib/realpath_test.cpp +++ b/libc/test/src/stdlib/realpath_test.cpp @@ -335,3 +335,52 @@ TEST_F(LlvmLibcRealpathTest, AllocatesResultWhenBufferIsNull) { ASSERT_STREQ(result, "/"); ::free(result); } + +TEST_F(LlvmLibcRealpathTest, ErrorsWithNotDirWhenFileIsInPath) { + TestDir test_dir; + ASSERT_TRUE(create_test_dir("ErrorsWithNotDirWhenFileIsInPath", test_dir)); + + ASSERT_THAT(test_dir.touch("file"), Succeeds()); + + ASSERT_EQ(realpath_buffered(test_dir.abspath("file/.")), nullptr); + ASSERT_ERRNO_EQ(ENOTDIR); + + ASSERT_EQ(realpath_buffered(test_dir.abspath("file/")), nullptr); + ASSERT_ERRNO_EQ(ENOTDIR); +} + +TEST_F(LlvmLibcRealpathTest, FileAtEndOfPathIsOk) { + TestDir test_dir; + ASSERT_TRUE(create_test_dir("FileAtEndOfPathIsOk", test_dir)); + + ASSERT_THAT(test_dir.mkdir("a"), Succeeds()); + ASSERT_THAT(test_dir.touch("a/file"), Succeeds()); + + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a/file")), + test_dir.abspath("a/file").c_str()); +} + +TEST_F(LlvmLibcRealpathTest, ErrorsWithNoEntWhenComponentDoesNotExist) { + TestDir test_dir; + ASSERT_TRUE( + create_test_dir("ErrorsWithNoEntWhenComponentDoesNotExist", test_dir)); + + // A missing directory should give ENOENT. + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a/b")), nullptr); + ASSERT_ERRNO_EQ(ENOENT); + + // Should fail if the final compnent doesn't exist. + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a")), nullptr); + ASSERT_ERRNO_EQ(ENOENT); +} + +TEST_F(LlvmLibcRealpathTest, ErrorsWithNoAccesWhenDirectoryNotSearchable) { + TestDir test_dir; + ASSERT_TRUE( + create_test_dir("ErrorsWithNoAccesWhenDirectoryNotSearchable", test_dir)); + + ASSERT_THAT(test_dir.mkdir("a", /* mode= */ 0644), Succeeds()); + + ASSERT_STREQ(realpath_buffered(test_dir.abspath("a/b")), nullptr); + ASSERT_ERRNO_EQ(EACCES); +} _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
