Author: royitaqi Date: 2024-08-15T17:24:39-07:00 New Revision: 26670e7fa4f032a019d23d56c6a02926e854e8af
URL: https://github.com/llvm/llvm-project/commit/26670e7fa4f032a019d23d56c6a02926e854e8af DIFF: https://github.com/llvm/llvm-project/commit/26670e7fa4f032a019d23d56c6a02926e854e8af.diff LOG: [lldb] Correctly fix a usage of `PATH_MAX`, and fix unit tests (#104502) # Part 1: Correctly fix a usage of `PATH_MAX` TL;DR: Adding a typedef `lldb_private::PathSmallString` which contains a hardcoded initial size (128). # Part 2: Fix unit tests After https://github.com/llvm/llvm-project/pull/104493 fixed the build break for Windows, unit test failure showed up for Windows. The root-cause is that the `FileSpec`'s in the unit tests are not style-specific. The fix is to apply either `WindowsSpec` or `PosixSpec` specifically. Added: Modified: lldb/include/lldb/lldb-private-types.h lldb/source/Utility/RealpathPrefixes.cpp lldb/unittests/Utility/FileSpecListTest.cpp lldb/unittests/Utility/RealpathPrefixesTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h index 055eea9f6456d6..b82a2b8aa05744 100644 --- a/lldb/include/lldb/lldb-private-types.h +++ b/lldb/include/lldb/lldb-private-types.h @@ -12,6 +12,7 @@ #include "lldb/lldb-private.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallString.h" #include <type_traits> @@ -26,6 +27,8 @@ class Platform; class ExecutionContext; class RegisterFlags; +typedef llvm::SmallString<256> PathSmallString; + typedef llvm::sys::DynamicLibrary (*LoadPluginCallbackType)( const lldb::DebuggerSP &debugger_sp, const FileSpec &spec, Status &error); diff --git a/lldb/source/Utility/RealpathPrefixes.cpp b/lldb/source/Utility/RealpathPrefixes.cpp index ee1c404c899676..f6e1d17e30c696 100644 --- a/lldb/source/Utility/RealpathPrefixes.cpp +++ b/lldb/source/Utility/RealpathPrefixes.cpp @@ -8,11 +8,11 @@ #include "lldb/Utility/RealpathPrefixes.h" -#include "lldb/Host/PosixApi.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/FileSpecList.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/lldb-private-types.h" using namespace lldb_private; @@ -54,7 +54,7 @@ RealpathPrefixes::ResolveSymlinks(const FileSpec &file_spec) { LLDB_LOGF(log, "Realpath'ing support file %s", file_spec_path.c_str()); // One prefix matched. Try to realpath. - llvm::SmallString<PATH_MAX> buff; + PathSmallString buff; std::error_code ec = m_fs->getRealPath(file_spec_path, buff); if (ec) return std::nullopt; diff --git a/lldb/unittests/Utility/FileSpecListTest.cpp b/lldb/unittests/Utility/FileSpecListTest.cpp index 4c04f434261301..d3f89ad0dfcb3c 100644 --- a/lldb/unittests/Utility/FileSpecListTest.cpp +++ b/lldb/unittests/Utility/FileSpecListTest.cpp @@ -133,20 +133,21 @@ TEST(SupportFileListTest, RelativePathMatchesWindows) { TEST(SupportFileListTest, SymlinkedAbsolutePaths) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("/symlink_dir/foo.h"), FileSpec("/real_dir/foo.h"))); + PosixSpec("/symlink_dir/foo.h"), PosixSpec("/real_dir/foo.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/symlink_dir"); + file_spec_list.Append(PosixSpec("/symlink_dir")); RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("/symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("/symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("/real_dir/foo.h"), &prefixes); + 0, PosixSpec("/real_dir/foo.h"), &prefixes); EXPECT_EQ(ret, (size_t)0); } @@ -157,20 +158,21 @@ TEST(SupportFileListTest, SymlinkedAbsolutePaths) { TEST(SupportFileListTest, RootDirectory) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("/symlink_dir/foo.h"), FileSpec("/real_dir/foo.h"))); + PosixSpec("/symlink_dir/foo.h"), PosixSpec("/real_dir/foo.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/"); + file_spec_list.Append(PosixSpec("/")); RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("/symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("/symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("/real_dir/foo.h"), &prefixes); + 0, PosixSpec("/real_dir/foo.h"), &prefixes); EXPECT_EQ(ret, (size_t)0); } @@ -181,20 +183,21 @@ TEST(SupportFileListTest, RootDirectory) { TEST(SupportFileListTest, SymlinkedRelativePaths) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("symlink_dir/foo.h"), FileSpec("real_dir/foo.h"))); + PosixSpec("symlink_dir/foo.h"), PosixSpec("real_dir/foo.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("symlink_dir"); + file_spec_list.Append(PosixSpec("symlink_dir")); RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("real_dir/foo.h"), &prefixes); + 0, PosixSpec("real_dir/foo.h"), &prefixes); EXPECT_EQ(ret, (size_t)0); } @@ -205,20 +208,21 @@ TEST(SupportFileListTest, SymlinkedRelativePaths) { TEST(SupportFileListTest, RealpathOnlyMatchFileName) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("symlink_dir/foo.h"), FileSpec("real_dir/foo.h"))); + PosixSpec("symlink_dir/foo.h"), PosixSpec("real_dir/foo.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("symlink_dir"); + file_spec_list.Append(PosixSpec("symlink_dir")); RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("some_other_dir/foo.h"), &prefixes); + 0, PosixSpec("some_other_dir/foo.h"), &prefixes); EXPECT_EQ(ret, UINT32_MAX); } @@ -228,21 +232,22 @@ TEST(SupportFileListTest, RealpathOnlyMatchFileName) { TEST(SupportFileListTest, DirectoryMatchStringPrefixButNotWholeDirectoryName) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("symlink_dir/foo.h"), FileSpec("real_dir/foo.h"))); + PosixSpec("symlink_dir/foo.h"), PosixSpec("real_dir/foo.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("symlink"); // This is a string prefix of the - // symlink but not a path prefix. + file_spec_list.Append(PosixSpec("symlink")); // This is a string prefix of the + // symlink but not a path prefix. RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("real_dir/foo.h"), &prefixes); + 0, PosixSpec("real_dir/foo.h"), &prefixes); EXPECT_EQ(ret, UINT32_MAX); } @@ -253,20 +258,21 @@ TEST(SupportFileListTest, DirectoryMatchStringPrefixButNotWholeDirectoryName) { TEST(SupportFileListTest, PartialBreakpointPath) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("symlink_dir/foo.h"), FileSpec("/real_dir/foo.h"))); + PosixSpec("symlink_dir/foo.h"), PosixSpec("/real_dir/foo.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("symlink_dir"); + file_spec_list.Append(PosixSpec("symlink_dir")); RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("real_dir/foo.h"), &prefixes); + 0, PosixSpec("real_dir/foo.h"), &prefixes); EXPECT_EQ(ret, (size_t)0); } @@ -277,20 +283,21 @@ TEST(SupportFileListTest, PartialBreakpointPath) { TEST(SupportFileListTest, DifferentBasename) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("/symlink_dir/foo.h"), FileSpec("/real_dir/bar.h"))); + PosixSpec("/symlink_dir/foo.h"), PosixSpec("/real_dir/bar.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/symlink_dir"); + file_spec_list.Append(PosixSpec("/symlink_dir")); RealpathPrefixes prefixes(file_spec_list, fs); // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("/symlink_dir/foo.h")); + support_file_list.Append(PosixSpec("/symlink_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("real_dir/bar.h"), &prefixes); + 0, PosixSpec("real_dir/bar.h"), &prefixes); EXPECT_EQ(ret, UINT32_MAX); } @@ -300,11 +307,11 @@ TEST(SupportFileListTest, DifferentBasename) { TEST(SupportFileListTest, NoPrefixes) { // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("/real_dir/bar.h")); + support_file_list.Append(PosixSpec("/real_dir/bar.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("/real_dir/foo.h"), nullptr); + 0, PosixSpec("/real_dir/foo.h"), nullptr); EXPECT_EQ(ret, UINT32_MAX); } @@ -314,10 +321,10 @@ TEST(SupportFileListTest, NoPrefixes) { TEST(SupportFileListTest, SameFile) { // Prepare support file list SupportFileList support_file_list; - support_file_list.EmplaceBack(FileSpec("/real_dir/foo.h")); + support_file_list.Append(PosixSpec("/real_dir/foo.h")); // Test size_t ret = support_file_list.FindCompatibleIndex( - 0, FileSpec("/real_dir/foo.h"), nullptr); + 0, PosixSpec("/real_dir/foo.h"), nullptr); EXPECT_EQ(ret, (size_t)0); } diff --git a/lldb/unittests/Utility/RealpathPrefixesTest.cpp b/lldb/unittests/Utility/RealpathPrefixesTest.cpp index 872ddf1fd223aa..a4dc7d6dd9822d 100644 --- a/lldb/unittests/Utility/RealpathPrefixesTest.cpp +++ b/lldb/unittests/Utility/RealpathPrefixesTest.cpp @@ -15,57 +15,66 @@ using namespace lldb_private; +static FileSpec PosixSpec(llvm::StringRef path) { + return FileSpec(path, FileSpec::Style::posix); +} + +static FileSpec WindowsSpec(llvm::StringRef path) { + return FileSpec(path, FileSpec::Style::windows); +} + // Should resolve a symlink which match an absolute prefix TEST(RealpathPrefixesTest, MatchingAbsolutePrefix) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h"))); + PosixSpec("/dir1/link.h"), PosixSpec("/dir2/real.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/dir1"); + file_spec_list.Append(PosixSpec("/dir1")); RealpathPrefixes prefixes(file_spec_list, fs); // Test std::optional<FileSpec> ret = - prefixes.ResolveSymlinks(FileSpec("/dir1/link.h")); - EXPECT_EQ(ret, FileSpec("/dir2/real.h")); + prefixes.ResolveSymlinks(PosixSpec("/dir1/link.h")); + EXPECT_EQ(ret, PosixSpec("/dir2/real.h")); } // Should resolve a symlink which match a relative prefix TEST(RealpathPrefixesTest, MatchingRelativePrefix) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("dir1/link.h"), FileSpec("dir2/real.h"))); + PosixSpec("dir1/link.h"), PosixSpec("dir2/real.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("dir1"); + file_spec_list.Append(PosixSpec("dir1")); RealpathPrefixes prefixes(file_spec_list, fs); // Test std::optional<FileSpec> ret = - prefixes.ResolveSymlinks(FileSpec("dir1/link.h")); - EXPECT_EQ(ret, FileSpec("dir2/real.h")); + prefixes.ResolveSymlinks(PosixSpec("dir1/link.h")); + EXPECT_EQ(ret, PosixSpec("dir2/real.h")); } // Should resolve in Windows and/or with a case-insensitive support file TEST(RealpathPrefixesTest, WindowsAndCaseInsensitive) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("f:\\dir1\\link.h", FileSpec::Style::windows), - FileSpec("f:\\dir2\\real.h", FileSpec::Style::windows), + WindowsSpec("f:\\dir1\\link.h"), WindowsSpec("f:\\dir2\\real.h"), FileSpec::Style::windows)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack(FileSpec("f:\\dir1", FileSpec::Style::windows)); + file_spec_list.Append(WindowsSpec("f:\\dir1")); RealpathPrefixes prefixes(file_spec_list, fs); // Test - std::optional<FileSpec> ret = prefixes.ResolveSymlinks( - FileSpec("F:\\DIR1\\LINK.H", FileSpec::Style::windows)); - EXPECT_EQ(ret, FileSpec("f:\\dir2\\real.h", FileSpec::Style::windows)); + std::optional<FileSpec> ret = + prefixes.ResolveSymlinks(WindowsSpec("F:\\DIR1\\LINK.H")); + EXPECT_EQ(ret, WindowsSpec("f:\\dir2\\real.h")); } // Should resolve a symlink when there is mixture of matching and mismatching @@ -73,52 +82,56 @@ TEST(RealpathPrefixesTest, WindowsAndCaseInsensitive) { TEST(RealpathPrefixesTest, MatchingAndMismatchingPrefix) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h"))); + PosixSpec("/dir1/link.h"), PosixSpec("/dir2/real.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/fake/path1"); - file_spec_list.EmplaceBack("/dir1"); // Matching prefix - file_spec_list.EmplaceBack("/fake/path2"); + file_spec_list.Append(PosixSpec("/fake/path1")); + file_spec_list.Append(PosixSpec("/dir1")); // Matching prefix + file_spec_list.Append(PosixSpec("/fake/path2")); RealpathPrefixes prefixes(file_spec_list, fs); // Test std::optional<FileSpec> ret = - prefixes.ResolveSymlinks(FileSpec("/dir1/link.h")); - EXPECT_EQ(ret, FileSpec("/dir2/real.h")); + prefixes.ResolveSymlinks(PosixSpec("/dir1/link.h")); + EXPECT_EQ(ret, PosixSpec("/dir2/real.h")); } // Should resolve a symlink when the prefixes matches after normalization TEST(RealpathPrefixesTest, ComplexPrefixes) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("dir1/link.h"), FileSpec("dir2/real.h"))); + PosixSpec("dir1/link.h"), PosixSpec("dir2/real.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("./dir1/foo/../bar/.."); // Equivalent to "/dir1" + file_spec_list.Append( + PosixSpec("./dir1/foo/../bar/..")); // Equivalent to "/dir1" RealpathPrefixes prefixes(file_spec_list, fs); // Test std::optional<FileSpec> ret = - prefixes.ResolveSymlinks(FileSpec("dir1/link.h")); - EXPECT_EQ(ret, FileSpec("dir2/real.h")); + prefixes.ResolveSymlinks(PosixSpec("dir1/link.h")); + EXPECT_EQ(ret, PosixSpec("dir2/real.h")); } // Should not resolve a symlink which doesn't match any prefixes TEST(RealpathPrefixesTest, MismatchingPrefixes) { // Prepare FS llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem( - FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h"))); + PosixSpec("/dir1/link.h"), PosixSpec("/dir2/real.h"), + FileSpec::Style::posix)); // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/dir3"); + file_spec_list.Append(PosixSpec("/dir3")); RealpathPrefixes prefixes(file_spec_list, fs); // Test std::optional<FileSpec> ret = - prefixes.ResolveSymlinks(FileSpec("/dir1/link.h")); + prefixes.ResolveSymlinks(PosixSpec("/dir1/link.h")); EXPECT_EQ(ret, std::nullopt); } @@ -130,11 +143,11 @@ TEST(RealpathPrefixesTest, Realpath) { // Prepare RealpathPrefixes FileSpecList file_spec_list; - file_spec_list.EmplaceBack("/symlink_dir"); + file_spec_list.Append(PosixSpec("/symlink_dir")); RealpathPrefixes prefixes(file_spec_list, fs); // Test std::optional<FileSpec> ret = - prefixes.ResolveSymlinks(FileSpec("/dir/real.h")); + prefixes.ResolveSymlinks(PosixSpec("/dir/real.h")); EXPECT_EQ(ret, std::nullopt); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits