https://github.com/rchamala created https://github.com/llvm/llvm-project/pull/120833
Summary: RFC https://discourse.llvm.org/t/rfc-python-callback-for-source-file-resolution/83545 Updated LineEntry::ApplyFileMappings to call resolve source file callback if set. include/lldb/Target/Platform.h, source/Target/Platform.cpp Implemented SetResolveSourceFileCallback and GetResolveSourceFileCallback include/lldb/Symbol/LineEntry.h, Source/Symbol/LineEntry.cpp Implemented CallResolveSourceFileCallbackIfSet Source/Target/StackFrame.cpp Source/Target/StackFrameList.cpp Source/Target/ThreadPlanStepRange.cpp Updated the caller to ApplyFileMappings unittests/Symbol/TestLineEntry.cpp Added comprehensive ResolveSourceFileCallback tests. Test Plan: Added unittests for LineEntry. ``` ninja check-lldb-unit ``` >From d8f99abee76a3d74d6c5c4904304f77cb2142087 Mon Sep 17 00:00:00 2001 From: Rahul Reddy Chamala <racha...@fb.com> Date: Thu, 19 Dec 2024 20:56:21 -0800 Subject: [PATCH] [lldb][ResolveSourceFileCallback] Call resolve source file callback Summary: RFC https://discourse.llvm.org/t/rfc-python-callback-for-source-file-resolution/83545 Updated LineEntry::ApplyFileMappings to call resolve source file callback if set. include/lldb/Target/Platform.h, source/Target/Platform.cpp Implemented SetResolveSourceFileCallback and GetResolveSourceFileCallback include/lldb/Symbol/LineEntry.h, Source/Symbol/LineEntry.cpp Implemented CallResolveSourceFileCallbackIfSet Source/Target/StackFrame.cpp Source/Target/StackFrameList.cpp Source/Target/ThreadPlanStepRange.cpp Updated the caller to ApplyFileMappings unittests/Symbol/TestLineEntry.cpp Added comprehensive ResolveSourceFileCallback tests. Test Plan: Added unittests for LineEntry. ``` ninja check-lldb-unit ``` --- lldb/include/lldb/Symbol/LineEntry.h | 5 +- lldb/include/lldb/Target/Platform.h | 19 ++ lldb/source/Symbol/LineEntry.cpp | 37 +++- lldb/source/Target/Platform.cpp | 55 ++++++ lldb/source/Target/StackFrame.cpp | 2 +- lldb/source/Target/StackFrameList.cpp | 3 +- lldb/source/Target/ThreadPlanStepRange.cpp | 7 +- lldb/unittests/Symbol/CMakeLists.txt | 2 + .../Symbol/Inputs/inlined-functions.cpp | 22 +++ lldb/unittests/Symbol/TestLineEntry.cpp | 164 +++++++++++++++++- 10 files changed, 302 insertions(+), 14 deletions(-) create mode 100644 lldb/unittests/Symbol/Inputs/inlined-functions.cpp diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h index 8da59cf0bd24aa..9fc334ed87fdab 100644 --- a/lldb/include/lldb/Symbol/LineEntry.h +++ b/lldb/include/lldb/Symbol/LineEntry.h @@ -128,7 +128,10 @@ struct LineEntry { /// /// \param[in] target_sp /// Shared pointer to the target this LineEntry belongs to. - void ApplyFileMappings(lldb::TargetSP target_sp); + /// + /// \param[in] module_sp + /// Shared pointer to the module this LineEntry belongs to. + void ApplyFileMappings(lldb::TargetSP target_sp, lldb::ModuleSP module_sp); /// Helper to access the file. const FileSpec &GetFile() const { return file_sp->GetSpecOnly(); } diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h index a702abb540fd93..3265d0d2145dd6 100644 --- a/lldb/include/lldb/Target/Platform.h +++ b/lldb/include/lldb/Target/Platform.h @@ -316,6 +316,12 @@ class Platform : public PluginInterface { virtual bool GetModuleSpec(const FileSpec &module_file_spec, const ArchSpec &arch, ModuleSpec &module_spec); + void + CallResolveSourceFileCallbackIfSet(const lldb::ModuleSP &module_sp, + const FileSpec &original_source_file_spec, + FileSpec &resolved_source_file_spec, + bool *did_create_ptr); + virtual Status ConnectRemote(Args &args); virtual Status DisconnectRemote(); @@ -967,6 +973,11 @@ class Platform : public PluginInterface { FileSpec &symbol_file_spec)> LocateModuleCallback; + typedef std::function<Status(const lldb::ModuleSP &module_sp, + const FileSpec &original_file_spec, + FileSpec &resolved_file_spec)> + ResolveSourceFileCallback; + /// Set locate module callback. This allows users to implement their own /// module cache system. For example, to leverage artifacts of build system, /// to bypass pulling files from remote platform, or to search symbol files @@ -975,6 +986,13 @@ class Platform : public PluginInterface { LocateModuleCallback GetLocateModuleCallback() const; + /// Set resolve source file callback. This allows users to implement their own + /// source file cache system. For example, to search source files from source + /// servers. + void SetResolveSourceFileCallback(ResolveSourceFileCallback callback); + + ResolveSourceFileCallback GetResolveSourceFileCallback() const; + protected: /// Create a list of ArchSpecs with the given OS and a architectures. The /// vendor field is left as an "unspecified unknown". @@ -1022,6 +1040,7 @@ class Platform : public PluginInterface { bool m_calculated_trap_handlers; const std::unique_ptr<ModuleCache> m_module_cache; LocateModuleCallback m_locate_module_callback; + ResolveSourceFileCallback m_resolve_source_file_callback; /// Ask the Platform subclass to fill in the list of trap handler names /// diff --git a/lldb/source/Symbol/LineEntry.cpp b/lldb/source/Symbol/LineEntry.cpp index c941a6927cb93f..1a966cc1c8387d 100644 --- a/lldb/source/Symbol/LineEntry.cpp +++ b/lldb/source/Symbol/LineEntry.cpp @@ -7,7 +7,10 @@ //===----------------------------------------------------------------------===// #include "lldb/Symbol/LineEntry.h" +#include "lldb/Core/Module.h" #include "lldb/Symbol/CompileUnit.h" +#include "lldb/Symbol/SymbolContext.h" +#include "lldb/Target/Platform.h" #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" @@ -241,13 +244,33 @@ AddressRange LineEntry::GetSameLineContiguousAddressRange( return complete_line_range; } -void LineEntry::ApplyFileMappings(lldb::TargetSP target_sp) { - if (target_sp) { - // Apply any file remappings to our file. - if (auto new_file_spec = target_sp->GetSourcePathMap().FindFile( - original_file_sp->GetSpecOnly())) { - file_sp = std::make_shared<SupportFile>(*new_file_spec, - original_file_sp->GetChecksum()); +void LineEntry::ApplyFileMappings(lldb::TargetSP target_sp, + lldb::ModuleSP module_sp) { + if (!target_sp) { + return; + } + + // Apply any file remappings to our file. + if (auto new_file_spec = target_sp->GetSourcePathMap().FindFile( + original_file_sp->GetSpecOnly())) { + file_sp = std::make_shared<SupportFile>(*new_file_spec, + original_file_sp->GetChecksum()); + } + + lldb::PlatformSP platform_sp = target_sp->GetPlatform(); + if (module_sp && platform_sp) { + FileSpec original_file_spec = original_file_sp->GetSpecOnly(); + FileSpec resolved_source_file_spec; + bool didResolveSourceFile = false; + + // Call resolve source file callback if set. This allows users to implement + // their own source file cache system. For example, to search source files + // from source servers. + platform_sp->CallResolveSourceFileCallbackIfSet( + module_sp, original_file_spec, resolved_source_file_spec, + &didResolveSourceFile); + if (didResolveSourceFile) { + file_sp = std::make_shared<SupportFile>(resolved_source_file_spec); } } } diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index 3e7546048e5862..9f56888693fc42 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -1696,6 +1696,51 @@ void Platform::CallLocateModuleCallbackIfSet(const ModuleSpec &module_spec, } } +void Platform::CallResolveSourceFileCallbackIfSet( + const lldb::ModuleSP &module_sp, const FileSpec &original_source_file_spec, + FileSpec &resolved_source_file_spec, bool *did_create_ptr) { + if (!m_resolve_source_file_callback) { + // Resolve source file callback is not set. + return; + } + + Status error = m_resolve_source_file_callback( + module_sp, original_source_file_spec, resolved_source_file_spec); + + // Resolve source file callback is set and called. Check the error. + Log *log = GetLog(LLDBLog::Platform); + if (error.Fail()) { + LLDB_LOGF(log, "%s: Resolve source file callback failed: %s", + LLVM_PRETTY_FUNCTION, error.AsCString()); + return; + } + + if (!resolved_source_file_spec) { + LLDB_LOGF(log, + "%s: Resolve source file callback did not set " + "resolved_source_file_spec", + LLVM_PRETTY_FUNCTION); + return; + } + + // If the callback returned a source file, it should exist. + if (resolved_source_file_spec && + !FileSystem::Instance().Exists(resolved_source_file_spec)) { + LLDB_LOGF(log, + "%s: Resolve source file callback set a non-existent file to " + "source_file_spec: %s", + LLVM_PRETTY_FUNCTION, + resolved_source_file_spec.GetPath().c_str()); + // Clear source_file_spec for the error. + resolved_source_file_spec.Clear(); + return; + } + + if (did_create_ptr) { + *did_create_ptr = true; + } +} + bool Platform::GetCachedSharedModule(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp, bool *did_create_ptr) { @@ -2104,6 +2149,16 @@ Platform::LocateModuleCallback Platform::GetLocateModuleCallback() const { return m_locate_module_callback; } +void Platform::SetResolveSourceFileCallback( + ResolveSourceFileCallback callback) { + m_resolve_source_file_callback = callback; +} + +Platform::ResolveSourceFileCallback +Platform::GetResolveSourceFileCallback() const { + return m_resolve_source_file_callback; +} + PlatformSP PlatformList::GetOrCreate(llvm::StringRef name) { std::lock_guard<std::recursive_mutex> guard(m_mutex); for (const PlatformSP &platform_sp : m_platforms) { diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 2633c976c13bf4..404d93e75642f4 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -396,7 +396,7 @@ StackFrame::GetSymbolContext(SymbolContextItem resolve_scope) { if ((resolved & eSymbolContextLineEntry) && !m_sc.line_entry.IsValid()) { m_sc.line_entry = sc.line_entry; - m_sc.line_entry.ApplyFileMappings(m_sc.target_sp); + m_sc.line_entry.ApplyFileMappings(m_sc.target_sp, m_sc.module_sp); } } } else { diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 9c6208e9e0a65a..d7211da1ef94e0 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -494,7 +494,8 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx, while (unwind_sc.GetParentOfInlinedScope( curr_frame_address, next_frame_sc, next_frame_address)) { - next_frame_sc.line_entry.ApplyFileMappings(target_sp); + next_frame_sc.line_entry.ApplyFileMappings(target_sp, + unwind_sc.module_sp); behaves_like_zeroth_frame = false; StackFrameSP frame_sp(new StackFrame( m_thread.shared_from_this(), m_frames.size(), idx, diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index de4cd5995f6953..773af67b49ff9a 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -430,8 +430,11 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { new SupportFile(call_site_file_spec)); top_most_line_entry.range = range; top_most_line_entry.file_sp.reset(); - top_most_line_entry.ApplyFileMappings( - GetThread().CalculateTarget()); + + lldb::ModuleSP module_sp = + run_to_address.CalculateSymbolContextModule(); + top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget(), + module_sp); if (!top_most_line_entry.file_sp) top_most_line_entry.file_sp = top_most_line_entry.original_file_sp; diff --git a/lldb/unittests/Symbol/CMakeLists.txt b/lldb/unittests/Symbol/CMakeLists.txt index e1d24357e33db0..25de6e6fde9819 100644 --- a/lldb/unittests/Symbol/CMakeLists.txt +++ b/lldb/unittests/Symbol/CMakeLists.txt @@ -19,6 +19,7 @@ add_lldb_unittest(SymbolTests lldbUtilityHelpers lldbPluginObjectFileELF lldbPluginObjectFileMachO + lldbPluginPlatformLinux lldbPluginSymbolFileDWARF lldbPluginSymbolFileSymtab lldbPluginTypeSystemClang @@ -26,6 +27,7 @@ add_lldb_unittest(SymbolTests ) set(test_inputs + inlined-functions.cpp inlined-functions.yaml ) add_unittest_inputs(SymbolTests "${test_inputs}") diff --git a/lldb/unittests/Symbol/Inputs/inlined-functions.cpp b/lldb/unittests/Symbol/Inputs/inlined-functions.cpp new file mode 100644 index 00000000000000..7d8f53371721cb --- /dev/null +++ b/lldb/unittests/Symbol/Inputs/inlined-functions.cpp @@ -0,0 +1,22 @@ +inline __attribute__((always_inline)) int sum2(int a, int b) { + int result = a + b; + return result; +} + +int sum3(int a, int b, int c) { + int result = a + b + c; + return result; +} + +inline __attribute__((always_inline)) int sum4(int a, int b, int c, int d) { + int result = sum2(a, b) + sum2(c, d); + result += 0; + return result; +} + +int main(int argc, char **argv) { + sum3(3, 4, 5) + sum2(1, 2); + int sum = sum4(1, 2, 3, 4); + sum2(5, 6); + return 0; +} diff --git a/lldb/unittests/Symbol/TestLineEntry.cpp b/lldb/unittests/Symbol/TestLineEntry.cpp index 592504bc2198ab..92454d55ae8ea7 100644 --- a/lldb/unittests/Symbol/TestLineEntry.cpp +++ b/lldb/unittests/Symbol/TestLineEntry.cpp @@ -11,13 +11,16 @@ #include <optional> #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h" +#include "Plugins/Platform/Linux/PlatformLinux.h" #include "Plugins/SymbolFile/DWARF/DWARFASTParserClang.h" #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h" #include "Plugins/TypeSystem/Clang/TypeSystemClang.h" #include "TestingSupport/SubsystemRAII.h" #include "TestingSupport/TestUtilities.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" +#include "lldb/Core/PluginManager.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Symbol/CompileUnit.h" @@ -29,21 +32,36 @@ using namespace lldb; using namespace lldb_private; +using namespace lldb_private::platform_linux; using namespace lldb_private::plugin::dwarf; +using namespace testing; + +namespace { + +constexpr llvm::StringLiteral k_source_file("inlined-functions.cpp"); class LineEntryTest : public testing::Test { - SubsystemRAII<FileSystem, HostInfo, ObjectFileMachO, SymbolFileDWARF, - TypeSystemClang> + SubsystemRAII<FileSystem, HostInfo, ObjectFileMachO, PlatformLinux, + SymbolFileDWARF, TypeSystemClang> subsystem; public: void SetUp() override; + void TearDown() override; protected: llvm::Expected<SymbolContextList> GetLineEntriesForLine(uint32_t line, std::optional<uint16_t> column); + void CheckNoCallback(); + void CheckCallbackWithArgs(const ModuleSP &module_sp, + const FileSpec &resolved_file_spec, + const ModuleSP &expected_module_sp); + FileSpec GetTestSourceFile(); std::optional<TestFile> m_file; ModuleSP m_module_sp; + DebuggerSP m_debugger_sp; + PlatformSP m_platform_sp; + TargetSP m_target_sp; }; void LineEntryTest::SetUp() { @@ -51,6 +69,59 @@ void LineEntryTest::SetUp() { ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded()); m_file.emplace(std::move(*ExpectedFile)); m_module_sp = std::make_shared<Module>(m_file->moduleSpec()); + + // Create Debugger. + m_debugger_sp = Debugger::CreateInstance(); + EXPECT_TRUE(m_debugger_sp); + + // Create Platform. + ArchSpec host_arch("i386-pc-linux"); + m_platform_sp = + platform_linux::PlatformLinux::CreateInstance(true, &host_arch); + Platform::SetHostPlatform(m_platform_sp); + EXPECT_TRUE(m_platform_sp); + + // Create Target. + m_debugger_sp->GetTargetList().CreateTarget(*m_debugger_sp, "", host_arch, + eLoadDependentsNo, m_platform_sp, + m_target_sp); + EXPECT_TRUE(m_target_sp); + + m_target_sp->SetExecutableModule(m_module_sp); +} + +void LineEntryTest::TearDown() { + if (m_module_sp) { + ModuleList::RemoveSharedModule(m_module_sp); + } +} + +void LineEntryTest::CheckNoCallback() { + EXPECT_FALSE(m_platform_sp->GetResolveSourceFileCallback()); +} + +void LineEntryTest::CheckCallbackWithArgs(const ModuleSP &module_sp, + const FileSpec &resolved_file_spec, + const ModuleSP &expected_module_sp) { + EXPECT_TRUE(module_sp == expected_module_sp); + EXPECT_FALSE(resolved_file_spec); +} + +FileSpec LineEntryTest::GetTestSourceFile() { + const auto *info = UnitTest::GetInstance()->current_test_info(); + FileSpec test_file = HostInfo::GetProcessTempDir(); + test_file.AppendPathComponent(std::string(info->test_case_name()) + "-" + + info->name()); + test_file.AppendPathComponent(k_source_file); + + std::error_code ec = + llvm::sys::fs::create_directory(test_file.GetDirectory().GetCString()); + EXPECT_FALSE(ec); + + ec = llvm::sys::fs::copy_file(GetInputFilePath(k_source_file), + test_file.GetPath().c_str()); + EXPECT_FALSE(ec); + return test_file; } // TODO: Handle SourceLocationSpec column information @@ -78,6 +149,8 @@ llvm::Expected<SymbolContextList> LineEntryTest::GetLineEntriesForLine( return sc_line_entries; } +} // end namespace + // This tests if we can get all line entries that match the passed line, if // no column is specified. TEST_F(LineEntryTest, GetAllExactLineMatchesWithoutColumn) { @@ -125,6 +198,93 @@ TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeNestedInline) { ASSERT_EQ(range.GetByteSize(), (uint64_t)0x33); } +TEST_F(LineEntryTest, ResolveSourceFileCallbackNotSetTest) { + // Resolve source file callback is not set. ApplyFileMappings should succeed + // to return the original file spec from line entry + auto sc_line_entries = GetLineEntriesForLine(12); + ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded()); + auto line_entry = sc_line_entries.get()[0].line_entry; + + const lldb::SupportFileSP original_file_sp = line_entry.file_sp; + CheckNoCallback(); + + line_entry.ApplyFileMappings(m_target_sp, m_module_sp); + ASSERT_EQ(line_entry.file_sp, original_file_sp); +} + +TEST_F(LineEntryTest, ResolveSourceFileCallbackSetFailTest) { + // Resolve source file callback fails for some reason. + // CallResolveSourceFileCallbackIfSet should fail and ApplyFileMappings should + // return the original file spec. + auto sc_line_entries = GetLineEntriesForLine(12); + ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded()); + auto line_entry = sc_line_entries.get()[0].line_entry; + + const lldb::SupportFileSP original_file_sp = line_entry.file_sp; + m_platform_sp->SetResolveSourceFileCallback( + [this](const lldb::ModuleSP &module_sp, + const FileSpec &original_file_spec, FileSpec &resolved_file_spec) { + CheckCallbackWithArgs(module_sp, resolved_file_spec, m_module_sp); + return Status::FromErrorString( + "The resolve source file callback failed"); + }); + + line_entry.ApplyFileMappings(m_target_sp, m_module_sp); + ASSERT_EQ(line_entry.file_sp, original_file_sp); +} + +TEST_F(LineEntryTest, ResolveSourceFileCallbackSetNonExistentPathTest) { + // Resolve source file callback succeeds but returns a non-existent path. + // CallResolveSourceFileCallbackIfSet should succeed and ApplyFileMappings + // should return the original file spec. + auto sc_line_entries = GetLineEntriesForLine(12); + ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded()); + auto line_entry = sc_line_entries.get()[0].line_entry; + + const lldb::SupportFileSP original_file_sp = line_entry.file_sp; + FileSpec callback_file_spec; + const FileSpec expected_callback_file_spec = + FileSpec("/this path does not exist"); + + m_platform_sp->SetResolveSourceFileCallback( + [this, &expected_callback_file_spec, &callback_file_spec]( + const lldb::ModuleSP &module_sp, const FileSpec &original_file_spec, + FileSpec &resolved_file_spec) { + CheckCallbackWithArgs(module_sp, resolved_file_spec, m_module_sp); + resolved_file_spec.SetPath(expected_callback_file_spec.GetPath()); + callback_file_spec.SetPath(resolved_file_spec.GetPath()); + return Status(); + }); + + line_entry.ApplyFileMappings(m_target_sp, m_module_sp); + + ASSERT_EQ(callback_file_spec, expected_callback_file_spec); + ASSERT_EQ(line_entry.file_sp, original_file_sp); +} + +TEST_F(LineEntryTest, ResolveSourceFileCallbackSetSuccessTest) { + // Resolve source file callback is set. CallResolveSourceFileCallbackIfSet + // should succeed and ApplyFileMappings should return the new file spec from + // the callback. + auto sc_line_entries = GetLineEntriesForLine(12); + ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded()); + auto line_entry = sc_line_entries.get()[0].line_entry; + + const FileSpec expected_callback_file_spec = GetTestSourceFile(); + + m_platform_sp->SetResolveSourceFileCallback( + [this, &expected_callback_file_spec](const lldb::ModuleSP &module_sp, + const FileSpec &original_file_spec, + FileSpec &resolved_file_spec) { + CheckCallbackWithArgs(module_sp, resolved_file_spec, m_module_sp); + resolved_file_spec.SetPath(expected_callback_file_spec.GetPath()); + return Status(); + }); + + line_entry.ApplyFileMappings(m_target_sp, m_module_sp); + ASSERT_EQ(line_entry.file_sp->GetSpecOnly(), expected_callback_file_spec); +} + /* # inlined-functions.cpp inline __attribute__((always_inline)) int sum2(int a, int b) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
