Skip to content

Commit

Permalink
[lldb] Correctly fix a usage of PATH_MAX, and fix unit tests (llvm#…
Browse files Browse the repository at this point in the history 
…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 llvm#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.
  • Loading branch information
@royitaqi
royitaqi authored Aug 16, 2024
1 parent 4695c16 commit 26670e7
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 64 deletions.
3 changes: 3 additions & 0 deletions lldb/include/lldb/lldb-private-types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "lldb/lldb-private.h"

#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallString.h"

#include <type_traits>

Expand All @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Utility/RealpathPrefixes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
73 changes: 40 additions & 33 deletions lldb/unittests/Utility/FileSpecListTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}
Loading

0 comments on commit 26670e7

Please sign in to comment.