llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> Register a Diagnostics artifact provider from ProcessGDBRemote so a diagnostics bundle captures the GDB-remote packet history, the same data "process plugin packet history" prints. --- <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub> --- Full diff: https://github.com/llvm/llvm-project/pull/210418.diff 4 Files Affected: - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+19) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+4) - (modified) lldb/unittests/Process/gdb-remote/CMakeLists.txt (+1) - (added) lldb/unittests/Process/gdb-remote/ProcessGDBRemoteTest.cpp (+103) ``````````diff diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index c80e7e48b30b8..9e8db42d4572d 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -33,6 +33,7 @@ #include "lldb/Breakpoint/WatchpointAlgorithms.h" #include "lldb/Breakpoint/WatchpointResource.h" #include "lldb/Core/Debugger.h" +#include "lldb/Core/Diagnostics.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" @@ -346,10 +347,28 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, m_use_g_packet_for_reading = GetGlobalPluginProperties().GetUseGPacketForReading(); + + // Contribute the packet history to diagnostics bundles, keyed on the unique + // id so concurrent processes don't collide. + if (Diagnostics::Enabled()) { + std::string name = + llvm::formatv("gdb-remote-packet-history-{0}.txt", GetUniqueID()); + m_diagnostics_artifact_id = Diagnostics::Instance().AddArtifactProvider( + std::move(name), [this]() -> std::string { + StreamString stream; + DumpPluginHistory(stream); + return stream.GetString().str(); + }); + } } // Destructor ProcessGDBRemote::~ProcessGDBRemote() { + // Unregister before teardown so a concurrent collection can't run the + // provider on a half-destroyed process. + if (m_diagnostics_artifact_id && Diagnostics::Enabled()) + Diagnostics::Instance().RemoveArtifactProvider(*m_diagnostics_artifact_id); + // m_mach_process.UnregisterNotificationCallbacks (this); Clear(); // We need to call finalize on the process before destroying ourselves to diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 26966b6020b0e..ca75899bc5cbf 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -16,6 +16,7 @@ #include <string> #include <vector> +#include "lldb/Core/Diagnostics.h" #include "lldb/Core/LoadedModuleInfoList.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/ThreadSafeValue.h" @@ -285,6 +286,9 @@ class ProcessGDBRemote : public Process, GDBRemoteCommunicationClient m_gdb_comm; std::atomic<lldb::pid_t> m_debugserver_pid; + /// Registration for the packet-history diagnostics provider, if enabled. + std::optional<Diagnostics::ArtifactProviderID> m_diagnostics_artifact_id; + std::optional<StringExtractorGDBRemote> m_last_stop_packet; std::recursive_mutex m_last_stop_packet_mutex; diff --git a/lldb/unittests/Process/gdb-remote/CMakeLists.txt b/lldb/unittests/Process/gdb-remote/CMakeLists.txt index 50a97e5cebaa9..95c79c699f108 100644 --- a/lldb/unittests/Process/gdb-remote/CMakeLists.txt +++ b/lldb/unittests/Process/gdb-remote/CMakeLists.txt @@ -5,6 +5,7 @@ add_lldb_unittest(ProcessGdbRemoteTests GDBRemoteCommunicationServerTest.cpp GDBRemoteCommunicationTest.cpp GDBRemoteTestUtils.cpp + ProcessGDBRemoteTest.cpp LINK_COMPONENTS Support diff --git a/lldb/unittests/Process/gdb-remote/ProcessGDBRemoteTest.cpp b/lldb/unittests/Process/gdb-remote/ProcessGDBRemoteTest.cpp new file mode 100644 index 0000000000000..783af0b7020f4 --- /dev/null +++ b/lldb/unittests/Process/gdb-remote/ProcessGDBRemoteTest.cpp @@ -0,0 +1,103 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/Process/gdb-remote/ProcessGDBRemote.h" +#include "Plugins/Platform/MacOSX/PlatformMacOSX.h" +#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Diagnostics.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/Platform.h" +#include "lldb/Target/Target.h" +#include "lldb/Target/TargetList.h" +#include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/Listener.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Testing/Support/Error.h" +#include "gtest/gtest.h" + +#include <mutex> + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::process_gdb_remote; + +namespace { +class ProcessGDBRemoteDiagnosticsTest : public ::testing::Test { +public: + void SetUp() override { + FileSystem::Initialize(); + HostInfo::Initialize(); + PlatformMacOSX::Initialize(); + static std::once_flag g_debugger_initialize_flag; + std::call_once(g_debugger_initialize_flag, + [] { Debugger::Initialize(nullptr); }); + Diagnostics::Initialize(); + } + void TearDown() override { + Diagnostics::Terminate(); + PlatformMacOSX::Terminate(); + HostInfo::Terminate(); + FileSystem::Terminate(); + } +}; +} // namespace + +// A ProcessGDBRemote registers a packet-history provider while alive and +// unregisters it when destroyed. +TEST_F(ProcessGDBRemoteDiagnosticsTest, PacketHistoryContributesToBundle) { + ArchSpec arch("x86_64-apple-macosx-"); + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + PlatformSP platform_sp; + TargetSP target_sp; + debugger_sp->GetTargetList().CreateTarget( + *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp = Listener::MakeListener("test-listener"); + ProcessSP process_sp = ProcessGDBRemote::CreateInstance( + target_sp, listener_sp, /*crash_file_path=*/nullptr, + /*can_connect=*/true); + ASSERT_TRUE(process_sp); + + // Named after the process's unique id to avoid collisions. + std::string artifact = llvm::formatv("gdb-remote-packet-history-{0}.txt", + process_sp->GetUniqueID()); + ExecutionContext exe_ctx; + + // Alive: the provider contributes the history file. + { + llvm::Expected<FileSpec> dir = Diagnostics::CreateUniqueDirectory(); + ASSERT_THAT_EXPECTED(dir, llvm::Succeeded()); + llvm::Expected<Diagnostics::Report> report = + Diagnostics::Instance().Collect(*debugger_sp, exe_ctx, *dir); + ASSERT_THAT_EXPECTED(report, llvm::Succeeded()); + EXPECT_TRUE(llvm::is_contained(report->attachments.files, artifact)); + llvm::sys::fs::remove_directories(dir->GetPath()); + } + + // Destroyed: the provider is gone. + process_sp.reset(); + { + llvm::Expected<FileSpec> dir = Diagnostics::CreateUniqueDirectory(); + ASSERT_THAT_EXPECTED(dir, llvm::Succeeded()); + llvm::Expected<Diagnostics::Report> report = + Diagnostics::Instance().Collect(*debugger_sp, exe_ctx, *dir); + ASSERT_THAT_EXPECTED(report, llvm::Succeeded()); + EXPECT_FALSE(llvm::is_contained(report->attachments.files, artifact)); + llvm::sys::fs::remove_directories(dir->GetPath()); + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/210418 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
