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

Reply via email to