[lldb] Collect the executable, symbol file, and core in diagnostics

[JDevlieghere]

The diagnostics bundle already gathers logs, statistics, and a snapshot of the first triage commands, but sometimes the binaries are required to reproduce an issue. Collect the main executable, its symbol file, and the core file into the bundle.

Gate this on a new global setting, diagnostics.collect-binaries, which defaults to false: these files can be large and privacy sensitive, so including them is opt-in.

Assisted-by: Claude

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The diagnostics bundle already gathers logs, statistics, and a snapshot of the first triage commands, but sometimes the binaries are required to reproduce an issue. Collect the main executable, its symbol file, and the core file into the bundle.

Gate this on a new global setting, diagnostics.collect-binaries, which defaults to false: these files can be large and privacy sensitive, so including them is opt-in.

Assisted-by: Claude

---

Full diff: https://github.com/llvm/llvm-project/pull/210391.diff

5 Files Affected:

• (modified) lldb/include/lldb/Core/Diagnostics.h (+21-3)
• (modified) lldb/source/Core/CoreProperties.td (+7)
• (modified) lldb/source/Core/Debugger.cpp (+4)
• (modified) lldb/source/Core/Diagnostics.cpp (+76)
• (added) lldb/test/Shell/Diagnostics/TestDiagnosticsCollectBinaries.test (+44)

diff --git a/lldb/include/lldb/Core/Diagnostics.h b/lldb/include/lldb/Core/Diagnostics.h
index b1ee0d2fbeda0..554314e32e8ce 100644
--- a/lldb/include/lldb/Core/Diagnostics.h
+++ b/lldb/include/lldb/Core/Diagnostics.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_CORE_DIAGNOSTICS_H
 #define LLDB_CORE_DIAGNOSTICS_H
 
+#include "lldb/Core/UserSettingsController.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Error.h"
@@ -28,6 +29,16 @@ namespace lldb_private {
 class Debugger;
 class ExecutionContext;
 
+/// The global diagnostics settings, exposed under `diagnostics` in the settings
+/// hierarchy.
+class DiagnosticsProperties : public Properties {
+public:
+  DiagnosticsProperties();
+
+  bool GetCollectBinaries() const;
+  bool SetCollectBinaries(bool collect);
+};
+
 /// Diagnostics maintain an always-on, in-memory log of recent diagnostic
 /// messages that can be written out to help investigate bugs and troubleshoot
 /// issues.
@@ -60,9 +71,11 @@ class Diagnostics {
   /// Collect a full diagnostics bundle into \p dir and return its report.
   ///
   /// Writes the always-on log, the debugger's file-backed logs, statistics,
-  /// and a snapshot of the commands a triager runs first. Collection is
-  /// best-effort: a failure to produce one artifact never aborts the rest, so
-  /// a partial bundle is always better than none.
+  /// and a snapshot of the commands a triager runs first. When the
+  /// `collect-binaries` setting is enabled it also copies the executable, its
+  /// symbol file, and the core file. Collection is best-effort: a failure to
+  /// produce one artifact never aborts the rest, so a partial bundle is always
+  /// better than none.
   llvm::Expected<Report> Collect(Debugger &debugger,
                                  const ExecutionContext &exe_ctx,
                                  const FileSpec &dir);
@@ -79,6 +92,8 @@ class Diagnostics {
 
   static Diagnostics &Instance();
 
+  static DiagnosticsProperties &GetGlobalProperties();
+
   static bool Enabled();
   static void Initialize();
   static void Terminate();
@@ -104,6 +119,9 @@ class Diagnostics {
                               const ExecutionContext &exe_ctx,
                               const FileSpec &dir,
                               std::vector<std::string> &files);
+  static void CollectBinaries(const ExecutionContext &exe_ctx,
+                              const FileSpec &dir,
+                              std::vector<std::string> &files);
   /// @}
 
   /// Scalars carried in the report rather than written as files.
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index 97bfd4a361032..dee4cd40448bf 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -53,6 +53,13 @@ let Definition = "modulelist", Path = "symbols" in {
     Desc<"Enable on demand symbol loading in LLDB. LLDB will load debug info on demand for each module based on various conditions (e.g. matched breakpoint, resolved stack frame addresses and matched global variables/function symbols in symbol table) to improve performance. Please refer to docs/use/ondemand.rst for details.">;
 }
 
+let Definition = "diagnostics", Path = "diagnostics" in {
+  def CollectBinaries: Property<"collect-binaries", "Boolean">,
+    Global,
+    DefaultFalse,
+    Desc<"When enabled, a diagnostics bundle also collects the main executable, its symbol file, and the core file (when debugging one) to help reproduce the bug. These files can be large and may contain sensitive data (a core file captures process memory), so this is off by default.">;
+}
+
 #ifndef NDEBUG
 let Definition = "testing", Path = "testing" in {
   def InjectVarLocListError
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 907a85a55964c..170bef020e3ea 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Core/DebuggerEvents.h"
+#include "lldb/Core/Diagnostics.h"
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/Mangled.h"
 #include "lldb/Core/ModuleList.h"
@@ -1068,6 +1069,9 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
   m_collection_sp->AppendProperty(
       LanguageProperties::GetSettingName(), "Language settings.", true,
       Language::GetGlobalLanguageProperties().GetValueProperties());
+  m_collection_sp->AppendProperty(
+      "diagnostics", "Diagnostics settings.", true,
+      Diagnostics::GetGlobalProperties().GetValueProperties());
   if (m_command_interpreter_up) {
     m_collection_sp->AppendProperty(
         "interpreter",
diff --git a/lldb/source/Core/Diagnostics.cpp b/lldb/source/Core/Diagnostics.cpp
index 6377771f66224..311d8fab2b1bc 100644
--- a/lldb/source/Core/Diagnostics.cpp
+++ b/lldb/source/Core/Diagnostics.cpp
@@ -8,12 +8,16 @@
 
 #include "lldb/Core/Diagnostics.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Statistics.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Args.h"
@@ -35,6 +39,38 @@ using namespace lldb_private;
 using namespace lldb;
 using namespace llvm;
 
+namespace {
+
+#define LLDB_PROPERTIES_diagnostics
+#include "CoreProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_diagnostics
+#include "CorePropertiesEnum.inc"
+};
+
+} // namespace
+
+DiagnosticsProperties::DiagnosticsProperties() {
+  m_collection_sp = std::make_shared<OptionValueProperties>("diagnostics");
+  m_collection_sp->Initialize(g_diagnostics_properties_def);
+}
+
+bool DiagnosticsProperties::GetCollectBinaries() const {
+  const uint32_t idx = ePropertyCollectBinaries;
+  return GetPropertyAtIndexAs<bool>(
+      idx, g_diagnostics_properties[idx].default_uint_value != 0);
+}
+
+bool DiagnosticsProperties::SetCollectBinaries(bool collect) {
+  return SetPropertyAtIndex(ePropertyCollectBinaries, collect);
+}
+
+DiagnosticsProperties &Diagnostics::GetGlobalProperties() {
+  static DiagnosticsProperties g_settings;
+  return g_settings;
+}
+
 static constexpr size_t g_num_log_messages = 100;
 
 void Diagnostics::Initialize() {
@@ -132,6 +168,28 @@ static void WriteArtifact(const FileSpec &dir, llvm::StringRef name,
   files.push_back(name.str());
 }
 
+// Copy a file into the bundle, best-effort like WriteArtifact. The basename is
+// disambiguated because an executable and its symbol file can share one (a
+// Mach-O and the DWARF binary inside its .dSYM), which would otherwise clobber.
+static void CopyBinary(const FileSpec &src, const FileSpec &dir,
+                       std::vector<std::string> &files) {
+  if (!src || !FileSystem::Instance().Exists(src))
+    return;
+
+  std::string name = src.GetFilename().str();
+  FileSpec dst = dir.CopyByAppendingPathComponent(name);
+  for (unsigned i = 1; FileSystem::Instance().Exists(dst); ++i) {
+    name = formatv("{0}.{1}", src.GetFilename(), i).str();
+    dst = dir.CopyByAppendingPathComponent(name);
+  }
+
+  if (llvm::sys::fs::copy_file(src.GetPath(), dst.GetPath()))
+    return;
+  llvm::sys::fs::setPermissions(dst.GetPath(), llvm::sys::fs::owner_read |
+                                                   llvm::sys::fs::owner_write);
+  files.push_back(std::move(name));
+}
+
 // Run a command through the interpreter and return its combined output and
 // error text, for inclusion as a snapshot in the bundle.
 static std::string CaptureCommand(Debugger &debugger, llvm::StringRef command) {
@@ -193,6 +251,8 @@ Diagnostics::Collect(Debugger &debugger, const ExecutionContext &exe_ctx,
   CollectLogs(debugger, dir, report.attachments.files);
   CollectStatistics(debugger, exe_ctx, dir, report.attachments.files);
   CollectCommands(debugger, exe_ctx, dir, report.attachments.files);
+  if (GetGlobalProperties().GetCollectBinaries())
+    CollectBinaries(exe_ctx, dir, report.attachments.files);
 
   report.version = lldb_private::GetVersion();
   report.os = GetHostDescription(exe_ctx);
@@ -241,6 +301,22 @@ void Diagnostics::CollectCommands(Debugger &debugger,
   WriteArtifact(dir, "commands.txt", snapshot, files);
 }
 
+void Diagnostics::CollectBinaries(const ExecutionContext &exe_ctx,
+                                  const FileSpec &dir,
+                                  std::vector<std::string> &files) {
+  if (Target *target = exe_ctx.GetTargetPtr()) {
+    if (Module *exe = target->GetExecutableModulePointer()) {
+      CopyBinary(exe->GetFileSpec(), dir, files);
+      // Skip when symbols are inline: the symbol file is then the executable.
+      if (exe->GetSymbolFileFileSpec() != exe->GetFileSpec())
+        CopyBinary(exe->GetSymbolFileFileSpec(), dir, files);
+    }
+  }
+
+  if (Process *process = exe_ctx.GetProcessPtr())
+    CopyBinary(process->GetCoreFile(), dir, files);
+}
+
 std::string Diagnostics::GetHostDescription(const ExecutionContext &exe_ctx) {
   std::string os = HostInfo::GetTargetTriple().str();
   Target *target = exe_ctx.GetTargetPtr();
diff --git a/lldb/test/Shell/Diagnostics/TestDiagnosticsCollectBinaries.test b/lldb/test/Shell/Diagnostics/TestDiagnosticsCollectBinaries.test
new file mode 100644
index 0000000000000..711b0449c259e
--- /dev/null
+++ b/lldb/test/Shell/Diagnostics/TestDiagnosticsCollectBinaries.test
@@ -0,0 +1,44 @@
+# Verify that a diagnostics bundle collects the core file, gated by the
+# diagnostics.collect-binaries setting.
+
+# RUN: yaml2obj %s -o %t.core
+
+# The setting is off by default.
+# RUN: %lldb -x -b -o 'settings show diagnostics.collect-binaries' 2>/dev/null \
+# RUN:   | FileCheck %s --check-prefix SETTING
+# SETTING: diagnostics.collect-binaries (boolean) = false
+
+# When enabled, the core file is copied into the bundle.
+# RUN: rm -rf %t.on && mkdir -p %t.on
+# RUN: %lldb -x -b -c %t.core \
+# RUN:   -o 'settings set diagnostics.collect-binaries true' \
+# RUN:   -o 'diagnostics dump -d %t.on' 2>/dev/null
+# RUN: ls %t.on | FileCheck %s --check-prefix COLLECT
+# COLLECT: {{.*}}.core
+
+# By default (setting off) the core file is left out while the textual
+# artifacts are still written.
+# RUN: rm -rf %t.off && mkdir -p %t.off
+# RUN: %lldb -x -b -c %t.core -o 'diagnostics dump -d %t.off' 2>/dev/null
+# RUN: ls %t.off | FileCheck %s --check-prefix NOCOLLECT
+# NOCOLLECT-DAG: commands.txt
+# NOCOLLECT-DAG: diagnostics.log
+# NOCOLLECT-NOT: core
+
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Platform ID:     Linux
+    CSD Version:     'Linux 3.13'
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            ModuleList
+    Modules:
+      - Base of Image:   0x0000000000400000
+        Size of Image:   0x00017000
+        Module Name:     'a.out'
+        CodeView Record: ''
+...

[medismailben]

LGTM