[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

[Walter Erquinigo via lldb-commits]

https://github.com/walter-erquinigo updated https://github.com/llvm/llvm-project/pull/71828

>From b7fadbbe01d626fbffe2523c601b2e7ea727b591 Mon Sep 17 00:00:00 2001
From: walter erquinigo <walter at modular.com>
Date: Thu, 9 Nov 2023 11:40:23 -0500
Subject: [PATCH] [LLDB] Ensure the data of apple accelerator tables is kept
 around

lldb's AppleDWARFIndex is created with a few `llvm::AppleAcceleratorTable` variables, but they only hold pointers to the underlying data, which doesn't prevent the data owner (lldb's DataBufferHeap) from being disposed.

Because of this, an asan build of lldb was showing an error in which a jitted accelerator table was being fred before read, which made my lldb fall in an infinite loop trying to parse corrupted accel table data. This issue only happens when I'm using a jitted execution and not when debugging a precompiled binary, probably because in the jit case the underlying data has to necessarily be copied via gdb-remote instead of mmaping a debug info file.

The correct fix seems to also store the underlying storage along with the accelerator tables in AppleDWARFIndex.
---
 .../SymbolFile/DWARF/AppleDWARFIndex.cpp      |  9 ++++++++-
 .../SymbolFile/DWARF/AppleDWARFIndex.h        | 20 +++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 325517ca1d2499b..33537df4f507624 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -51,13 +51,20 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
   extract_and_check(apple_namespaces_table_up);
   extract_and_check(apple_types_table_up);
   extract_and_check(apple_objc_table_up);
+  assert(apple_names.GetByteSize() == 0 || apple_names.GetSharedDataBuffer());
+  assert(apple_namespaces.GetByteSize() == 0 ||
+         apple_namespaces.GetSharedDataBuffer());
+  assert(apple_types.GetByteSize() == 0 || apple_types.GetSharedDataBuffer());
+  assert(apple_objc.GetByteSize() == 0 || apple_objc.GetSharedDataBuffer());
 
   if (apple_names_table_up || apple_namespaces_table_up ||
       apple_types_table_up || apple_objc_table_up)
     return std::make_unique<AppleDWARFIndex>(
         module, std::move(apple_names_table_up),
         std::move(apple_namespaces_table_up), std::move(apple_types_table_up),
-        std::move(apple_objc_table_up));
+        std::move(apple_objc_table_up), apple_names.GetSharedDataBuffer(),
+        apple_namespaces.GetSharedDataBuffer(),
+        apple_types.GetSharedDataBuffer(), apple_objc.GetSharedDataBuffer());
 
   return nullptr;
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
index a1fb99700d10a21..73de75b583bd419 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -25,11 +25,19 @@ class AppleDWARFIndex : public DWARFIndex {
                   std::unique_ptr<llvm::AppleAcceleratorTable> apple_names,
                   std::unique_ptr<llvm::AppleAcceleratorTable> apple_namespaces,
                   std::unique_ptr<llvm::AppleAcceleratorTable> apple_types,
-                  std::unique_ptr<llvm::AppleAcceleratorTable> apple_objc)
+                  std::unique_ptr<llvm::AppleAcceleratorTable> apple_objc,
+                  lldb::DataBufferSP apple_names_storage,
+                  lldb::DataBufferSP apple_namespaces_storage,
+                  lldb::DataBufferSP apple_types_storage,
+                  lldb::DataBufferSP apple_objc_storage)
       : DWARFIndex(module), m_apple_names_up(std::move(apple_names)),
         m_apple_namespaces_up(std::move(apple_namespaces)),
         m_apple_types_up(std::move(apple_types)),
-        m_apple_objc_up(std::move(apple_objc)) {}
+        m_apple_objc_up(std::move(apple_objc)),
+        m_apple_names_storage(apple_names_storage),
+        m_apple_namespaces_storage(apple_namespaces_storage),
+        m_apple_types_storage(apple_types_storage),
+        m_apple_objc_storage(apple_objc_storage) {}
 
   void Preload() override {}
 
@@ -67,6 +75,14 @@ class AppleDWARFIndex : public DWARFIndex {
   std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_namespaces_up;
   std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_types_up;
   std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_objc_up;
+  /// The following storage variables hold the data that the apple accelerator
+  /// tables tables above point to.
+  /// {
+  lldb::DataBufferSP m_apple_names_storage;
+  lldb::DataBufferSP m_apple_namespaces_storage;
+  lldb::DataBufferSP m_apple_types_storage;
+  lldb::DataBufferSP m_apple_objc_storage;
+  /// }
 
   /// Search for entries whose name is `name` in `table`, calling `callback` for
   /// each match. If `search_for_tag` is provided, ignore entries whose tag is