https://github.com/jroelofs created https://github.com/llvm/llvm-project/pull/184688
The API was a bit easy to accidentally mis-use, as it silently accepted Model strings without any randomness marker characters (`%`), which results in a unique path that's not at all unique. To avoid this foot-gun, we now assert that the model contains at least one, and update all the broken usages accordingly. rdar://170349565 >From 4e9d37f5693e7cd96811b31f154fb6f75d160174 Mon Sep 17 00:00:00 2001 From: Jon Roelofs <[email protected]> Date: Wed, 4 Mar 2026 13:46:38 -0800 Subject: [PATCH] [llvm][Support] Make createUniquePath require at least one wildcard marker The API was a bit easy to accidentally mis-use, as it silently accepted Model strings without any randomness marker characters (`%`), which results in a unique path that's not at all unique. To avoid this foot-gun, we now assert that the model contains at least one, and update all the broken usages accordingly. rdar://170349565 --- lldb/unittests/Host/SocketTest.cpp | 4 ++-- lldb/unittests/Interpreter/TestCompletion.cpp | 2 +- llvm/include/llvm/Support/FileSystem.h | 2 +- llvm/lib/Support/Path.cpp | 3 +++ .../Orc/LibraryResolverTest.cpp | 2 +- llvm/unittests/Support/Path.cpp | 22 +++++++++---------- .../Support/raw_socket_stream_test.cpp | 13 ++++++----- 7 files changed, 27 insertions(+), 21 deletions(-) diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp index 5d0085210dd44..c702dfaf5147d 100644 --- a/lldb/unittests/Host/SocketTest.cpp +++ b/lldb/unittests/Host/SocketTest.cpp @@ -403,8 +403,8 @@ TEST_F(SocketTest, DomainSocketFromBoundNativeSocket) { TEST_F(SocketTest, AbstractSocketFromBoundNativeSocket) { // Generate a name for the abstract socket. llvm::SmallString<100> name; - llvm::sys::fs::createUniquePath("AbstractSocketFromBoundNativeSocket", name, - true); + llvm::sys::fs::createUniquePath("AbstractSocketFromBoundNativeSocket-%%%%%%", + name, true); llvm::sys::path::append(name, "test"); // Skip the test if the $TMPDIR is too long to hold a domain socket. diff --git a/lldb/unittests/Interpreter/TestCompletion.cpp b/lldb/unittests/Interpreter/TestCompletion.cpp index 3d4942787fdf5..faa2a93cc49fa 100644 --- a/lldb/unittests/Interpreter/TestCompletion.cpp +++ b/lldb/unittests/Interpreter/TestCompletion.cpp @@ -88,7 +88,7 @@ class CompletionTest : public testing::Test { for (auto File : llvm::zip(FileNames, Files)) { auto &Path = *std::get<1>(File); Path = BaseDir; - path::append(Path, std::get<0>(File)); + path::append(Path, std::get<0>(File), "-%%%%%%"); int FD; ASSERT_NO_ERROR(fs::createUniqueFile(Path, FD, Path)); ::close(FD); diff --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h index 547d732dc3053..b56aa302fedb1 100644 --- a/llvm/include/llvm/Support/FileSystem.h +++ b/llvm/include/llvm/Support/FileSystem.h @@ -794,7 +794,7 @@ enum OpenFlags : unsigned { /// /// Example: clang-%%-%%-%%-%%-%%.s => clang-a0-b1-c2-d3-e4.s /// -/// @param Model Name to base unique path off of. +/// @param Model Name to base unique path off of. Must contain at least one '%'. /// @param ResultPath Set to the file's path. /// @param MakeAbsolute Whether to use the system temp directory. LLVM_ABI void createUniquePath(const Twine &Model, diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp index a53beb9359bc0..130b33c0336ce 100644 --- a/llvm/lib/Support/Path.cpp +++ b/llvm/lib/Support/Path.cpp @@ -848,6 +848,9 @@ void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath, SmallString<128> ModelStorage; Model.toVector(ModelStorage); + assert(llvm::is_contained(ModelStorage, '%') && + "createUniquePath: Model must contain at least one '%'"); + if (MakeAbsolute) { // Make model absolute by prepending a temp directory if it's not already. if (!sys::path::is_absolute(Twine(ModelStorage))) { diff --git a/llvm/unittests/ExecutionEngine/Orc/LibraryResolverTest.cpp b/llvm/unittests/ExecutionEngine/Orc/LibraryResolverTest.cpp index 54237e5279df0..8561fcf8c3650 100644 --- a/llvm/unittests/ExecutionEngine/Orc/LibraryResolverTest.cpp +++ b/llvm/unittests/ExecutionEngine/Orc/LibraryResolverTest.cpp @@ -575,7 +575,7 @@ TEST_F(LibraryResolverIT, PathResolverCachesResults) { SmallString<128> TmpDylib; std::error_code EC; - EC = sys::fs::createUniqueFile(withext("A-copy"), TmpDylib); + EC = sys::fs::createUniqueFile(withext("A-copy-%%%%%%"), TmpDylib); if (EC) GTEST_SKIP() << "Failed to create temp dylib" << EC.message(); diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index b27ed6f950b10..9fb3ac155c7fe 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -940,19 +940,19 @@ TEST_F(FileSystemTest, TempFiles) { #ifdef _WIN32 // Path name > 260 chars should get an error. const char *Path270 = - "abcdefghijklmnopqrstuvwxyz9abcdefghijklmnopqrstuvwxyz8" - "abcdefghijklmnopqrstuvwxyz7abcdefghijklmnopqrstuvwxyz6" - "abcdefghijklmnopqrstuvwxyz5abcdefghijklmnopqrstuvwxyz4" - "abcdefghijklmnopqrstuvwxyz3abcdefghijklmnopqrstuvwxyz2" - "abcdefghijklmnopqrstuvwxyz1abcdefghijklmnopqrstuvwxyz0"; + "abcdefghijklmnopqrstuvwxyz9abcdefghijklmnopqrstuvwxyz8" + "abcdefghijklmnopqrstuvwxyz7abcdefghijklmnopqrstuvwxyz6" + "abcdefghijklmnopqrstuvwxyz5abcdefghijklmnopqrstuvwxyz4" + "abcdefghijklmnopqrstuvwxyz3abcdefghijklmnopqrstuvwxyz2" + "abcdefghijklmnopqrstuvwxyz1abcdefghijklmnopqrstuvwxyz%"; EXPECT_EQ(fs::createUniqueFile(Path270, FileDescriptor, TempPath), errc::invalid_argument); // Relative path < 247 chars, no problem. const char *Path216 = - "abcdefghijklmnopqrstuvwxyz7abcdefghijklmnopqrstuvwxyz6" - "abcdefghijklmnopqrstuvwxyz5abcdefghijklmnopqrstuvwxyz4" - "abcdefghijklmnopqrstuvwxyz3abcdefghijklmnopqrstuvwxyz2" - "abcdefghijklmnopqrstuvwxyz1abcdefghijklmnopqrstuvwxyz0"; + "abcdefghijklmnopqrstuvwxyz7abcdefghijklmnopqrstuvwxyz6" + "abcdefghijklmnopqrstuvwxyz5abcdefghijklmnopqrstuvwxyz4" + "abcdefghijklmnopqrstuvwxyz3abcdefghijklmnopqrstuvwxyz2" + "abcdefghijklmnopqrstuvwxyz1abcdefghijklmnopqrstu%%%%%%"; ASSERT_NO_ERROR(fs::createTemporaryFile(Path216, "", TempPath)); ASSERT_NO_ERROR(fs::remove(Twine(TempPath))); #endif @@ -2117,8 +2117,8 @@ TEST_F(FileSystemTest, is_local) { int FD; SmallString<128> TempPath; - ASSERT_NO_ERROR( - fs::createUniqueFile(Twine(TestDirectory) + "/temp", FD, TempPath)); + ASSERT_NO_ERROR(fs::createUniqueFile(Twine(TestDirectory) + "/temp-%%%%%%", + FD, TempPath)); FileRemover Cleanup(TempPath); // Make sure it exists. diff --git a/llvm/unittests/Support/raw_socket_stream_test.cpp b/llvm/unittests/Support/raw_socket_stream_test.cpp index 348fb4bb3e089..fd4d7f1e28030 100644 --- a/llvm/unittests/Support/raw_socket_stream_test.cpp +++ b/llvm/unittests/Support/raw_socket_stream_test.cpp @@ -32,7 +32,8 @@ TEST(raw_socket_streamTest, CLIENT_TO_SERVER_AND_SERVER_TO_CLIENT) { GTEST_SKIP(); SmallString<100> SocketPath; - llvm::sys::fs::createUniquePath("client_server_comms.sock", SocketPath, true); + llvm::sys::fs::createUniquePath("client_server_comms-%%%%%%.sock", SocketPath, + true); // Make sure socket file does not exist. May still be there from the last test std::remove(SocketPath.c_str()); @@ -73,7 +74,8 @@ TEST(raw_socket_streamTest, READ_WITH_TIMEOUT) { GTEST_SKIP(); SmallString<100> SocketPath; - llvm::sys::fs::createUniquePath("read_with_timeout.sock", SocketPath, true); + llvm::sys::fs::createUniquePath("read_with_timeout-%%%%%%.sock", SocketPath, + true); // Make sure socket file does not exist. May still be there from the last test std::remove(SocketPath.c_str()); @@ -105,7 +107,8 @@ TEST(raw_socket_streamTest, ACCEPT_WITH_TIMEOUT) { GTEST_SKIP(); SmallString<100> SocketPath; - llvm::sys::fs::createUniquePath("accept_with_timeout.sock", SocketPath, true); + llvm::sys::fs::createUniquePath("accept_with_timeout-%%%%%%.sock", SocketPath, + true); // Make sure socket file does not exist. May still be there from the last test std::remove(SocketPath.c_str()); @@ -126,8 +129,8 @@ TEST(raw_socket_streamTest, ACCEPT_WITH_SHUTDOWN) { GTEST_SKIP(); SmallString<100> SocketPath; - llvm::sys::fs::createUniquePath("accept_with_shutdown.sock", SocketPath, - true); + llvm::sys::fs::createUniquePath("accept_with_shutdown-%%%%%%.sock", + SocketPath, true); // Make sure socket file does not exist. May still be there from the last test std::remove(SocketPath.c_str()); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
