[lldb] Fix local buffer not being popped (swiftlang#10718)

On a recent change in GetDynamicTypeAndAddress_Existential, the
local buffer was being pushed but never popped. This fixes the issue by
introducing a new RAII data structure that will pop the local buffer
when out of scope.

rdar://148137949

Author: augusto2112

lldb/source/Plugins/LanguageRuntime/Swift/LLDBMemoryReader.cpp

@@ -16,8 +16,8 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 
-#include "llvm/Support/MathExtras.h"
 #include "swift/Demangling/Demangle.h"
+#include "llvm/Support/MathExtras.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -275,8 +275,7 @@ LLDBMemoryReader::resolvePointer(swift::remote::RemoteAddress address,
 
   auto *section_list = module_containing_pointer->GetSectionList();
   if (section_list->GetSize() == 0) {
-    LLDB_LOG(log,
-             "[MemoryReader] Module with empty section list.");
+    LLDB_LOG(log, "[MemoryReader] Module with empty section list.");
     return {};
   }
 
@@ -452,11 +451,18 @@ bool LLDBMemoryReader::readString(swift::remote::RemoteAddress address,
   return false;
 }
 
-void LLDBMemoryReader::pushLocalBuffer(uint64_t local_buffer,
-                                       uint64_t local_buffer_size) {
+MemoryReaderLocalBufferHolder::~MemoryReaderLocalBufferHolder() {
+  if (m_memory_reader)
+  m_memory_reader->popLocalBuffer();
+}
+
+MemoryReaderLocalBufferHolder
+LLDBMemoryReader::pushLocalBuffer(uint64_t local_buffer,
+                                  uint64_t local_buffer_size) {
   lldbassert(!m_local_buffer);
   m_local_buffer = local_buffer;
   m_local_buffer_size = local_buffer_size;
+  return MemoryReaderLocalBufferHolder(this);
 }
 
 void LLDBMemoryReader::popLocalBuffer() {
@@ -579,8 +585,7 @@ LLDBMemoryReader::getFileAddressAndModuleForTaggedAddress(
   ModuleSP module = pair_iterator->second;
   auto *section_list = module->GetSectionList();
   if (section_list->GetSize() == 0) {
-    LLDB_LOG(log,
-             "[MemoryReader] Module with empty section list.");
+    LLDB_LOG(log, "[MemoryReader] Module with empty section list.");
     return {};
   }
   uint64_t file_address;
@@ -620,8 +625,8 @@ LLDBMemoryReader::resolveRemoteAddress(uint64_t address) const {
   Address resolved(file_address, object_file->GetSectionList());
 
   // If the address doesn't have a section it means we couldn't find a section
-  // that contains that file address, and the "resolved" instance is wrong. 
-  // Calculate the virtual address by finding out the slide of the associated 
+  // that contains that file address, and the "resolved" instance is wrong.
+  // Calculate the virtual address by finding out the slide of the associated
   // module, and adding that to the file address.
   if (resolved.GetSection()) {
     LLDB_LOGV(log,

lldb/source/Plugins/LanguageRuntime/Swift/LLDBMemoryReader.h

@@ -24,8 +24,35 @@
 #include "llvm/Support/Memory.h"
 
 namespace lldb_private {
+class LLDBMemoryReader;
+class MemoryReaderLocalBufferHolder {
+public:
+  MemoryReaderLocalBufferHolder() : m_memory_reader(nullptr) {}
+  MemoryReaderLocalBufferHolder(MemoryReaderLocalBufferHolder &&other)
+      : m_memory_reader(other.m_memory_reader) {
+    other.m_memory_reader = nullptr;
+  }
+
+  MemoryReaderLocalBufferHolder &operator=(MemoryReaderLocalBufferHolder &&other) {
+    this->m_memory_reader = other.m_memory_reader;
+    other.m_memory_reader = nullptr;
+    return *this;
+  }
+
+  ~MemoryReaderLocalBufferHolder(); 
+private:
+  friend LLDBMemoryReader;
+
+  MemoryReaderLocalBufferHolder(LLDBMemoryReader *memory_reader)
+      : m_memory_reader(memory_reader) {}
+
+  LLDBMemoryReader *m_memory_reader;
+};
+
 class LLDBMemoryReader : public swift::remote::MemoryReader {
 public:
+
+
   LLDBMemoryReader(Process &p,
                    std::function<swift::remote::RemoteAbsolutePointer(
                        swift::remote::RemoteAbsolutePointer)>
@@ -56,9 +83,7 @@ class LLDBMemoryReader : public swift::remote::MemoryReader {
   bool readString(swift::remote::RemoteAddress address,
                   std::string &dest) override;
 
-  void pushLocalBuffer(uint64_t local_buffer, uint64_t local_buffer_size);
-
-  void popLocalBuffer();
+  MemoryReaderLocalBufferHolder pushLocalBuffer(uint64_t local_buffer, uint64_t local_buffer_size);
 
   /// Adds the module to the list of modules we're tracking using tagged
   /// addresses, so we can read memory from the file cache whenever possible.
@@ -71,6 +96,10 @@ class LLDBMemoryReader : public swift::remote::MemoryReader {
   bool readMetadataFromFileCacheEnabled() const;
 
 private:
+  friend MemoryReaderLocalBufferHolder;
+
+  void popLocalBuffer();
+
   /// Gets the file address and module that were mapped to a given tagged
   /// address.
   std::optional<std::pair<uint64_t, lldb::ModuleSP>>

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h

@@ -63,6 +63,7 @@ class SwiftLanguageRuntimeStub;
 class SwiftLanguageRuntimeImpl;
 class ReflectionContextInterface;
 class LLDBMemoryReader;
+class MemoryReaderLocalBufferHolder;
 struct SuperClassType;
 
 using ThreadSafeReflectionContext = LockGuarded<ReflectionContextInterface>;
@@ -726,9 +727,8 @@ class SwiftLanguageRuntime : public LanguageRuntime {
 
   std::shared_ptr<LLDBMemoryReader> GetMemoryReader();
 
-  void PushLocalBuffer(uint64_t local_buffer, uint64_t local_buffer_size);
-
-  void PopLocalBuffer();
+  MemoryReaderLocalBufferHolder PushLocalBuffer(uint64_t local_buffer,
+                                                uint64_t local_buffer_size);
 
   // These are the helper functions for GetObjectDescription for various
   // types of swift objects.

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp

@@ -260,15 +260,12 @@ std::shared_ptr<LLDBMemoryReader> SwiftLanguageRuntime::GetMemoryReader() {
   return m_memory_reader_sp;
 }
 
-void SwiftLanguageRuntime::PushLocalBuffer(uint64_t local_buffer,
+MemoryReaderLocalBufferHolder SwiftLanguageRuntime::PushLocalBuffer(uint64_t local_buffer,
                                            uint64_t local_buffer_size) {
-  ((LLDBMemoryReader *)GetMemoryReader().get())
+  return ((LLDBMemoryReader *)GetMemoryReader().get())
       ->pushLocalBuffer(local_buffer, local_buffer_size);
 }
 
-void SwiftLanguageRuntime::PopLocalBuffer() {
-  ((LLDBMemoryReader *)GetMemoryReader().get())->popLocalBuffer();
-}
 
 class LLDBTypeInfoProvider : public swift::remote::TypeInfoProvider {
   SwiftLanguageRuntime &m_runtime;
@@ -1599,8 +1596,7 @@ llvm::Expected<std::string> SwiftLanguageRuntime::GetEnumCaseName(
     return "";
 
   auto *eti = llvm::cast<EnumTypeInfo>(ti);
-  PushLocalBuffer((int64_t)data.GetDataStart(), data.GetByteSize());
-  auto defer = llvm::make_scope_exit([&] { PopLocalBuffer(); });
+  auto buffer_holder = PushLocalBuffer((int64_t)data.GetDataStart(), data.GetByteSize());
   RemoteAddress addr(data.GetDataStart());
   int case_index;
   if (eti->projectEnumValue(*GetMemoryReader(), addr, &case_index))
@@ -2313,85 +2309,87 @@ bool SwiftLanguageRuntime::GetDynamicTypeAndAddress_Existential(
     return false;
   }
 
+  // This scope is needed because the validation code will call PushLocalBuffer,
+  // so we need to pop it before that call.
+  {
+    MemoryReaderLocalBufferHolder holder;
+    if (use_local_buffer)
+      holder = PushLocalBuffer(
+          existential_address,
+          llvm::expectedToOptional(in_value.GetByteSize()).value_or(0));
 
-  if (use_local_buffer)
-    PushLocalBuffer(
-        existential_address,
-        llvm::expectedToOptional(in_value.GetByteSize()).value_or(0));
-
-  swift::remote::RemoteAddress remote_existential(existential_address);
-
-  ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
-  if (!reflection_ctx)
-    return false;
-  auto tr_ts = tss->GetTypeSystemSwiftTypeRef();
-  if (!tr_ts)
-    return false;
-
-  auto flavor = SwiftLanguageRuntime::GetManglingFlavor(
-      existential_type.GetMangledTypeName());
-  CompilerType dynamic_type;
-  uint64_t dynamic_address = 0;
-  if (flavor == swift::Mangle::ManglingFlavor::Default) {
-    auto pair = reflection_ctx->ProjectExistentialAndUnwrapClass(
-        remote_existential, *existential_typeref, tr_ts->GetDescriptorFinder());
-
-    if (!pair) {
-      if (log)
-        log->Printf("Runtime failed to get dynamic type of existential");
-      return false;
-    }
-
-    const swift::reflection::TypeRef *typeref;
-    swift::remote::RemoteAddress out_address(nullptr);
-    std::tie(typeref, out_address) = *pair;
+    swift::remote::RemoteAddress remote_existential(existential_address);
 
-    auto ts = tss->GetTypeSystemSwiftTypeRef();
-    if (!ts)
-      return false;
-    swift::Demangle::Demangler dem;
-    swift::Demangle::NodePointer node = typeref->getDemangling(dem);
-    dynamic_type = ts->RemangleAsType(dem, node, flavor);
-    dynamic_address = out_address.getAddressData();
-  } else {
-    // In the embedded Swift case, the existential container just points to the
-    // instance.
-    auto reflection_ctx = GetReflectionContext();
+    ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
     if (!reflection_ctx)
       return false;
-    auto maybe_addr_or_symbol =
-        reflection_ctx->ReadPointer(existential_address);
-    if (!maybe_addr_or_symbol)
+    auto tr_ts = tss->GetTypeSystemSwiftTypeRef();
+    if (!tr_ts)
       return false;
 
-    uint64_t address = 0;
-    if (maybe_addr_or_symbol->isResolved()) {
-      address = maybe_addr_or_symbol->getOffset();
+    auto flavor = SwiftLanguageRuntime::GetManglingFlavor(
+        existential_type.GetMangledTypeName());
+    CompilerType dynamic_type;
+    uint64_t dynamic_address = 0;
+    if (flavor == swift::Mangle::ManglingFlavor::Default) {
+      auto pair = reflection_ctx->ProjectExistentialAndUnwrapClass(
+          remote_existential, *existential_typeref,
+          tr_ts->GetDescriptorFinder());
+
+      if (!pair) {
+        if (log)
+          log->Printf("Runtime failed to get dynamic type of existential");
+        return false;
+      }
+
+      const swift::reflection::TypeRef *typeref;
+      swift::remote::RemoteAddress out_address(nullptr);
+      std::tie(typeref, out_address) = *pair;
+
+      auto ts = tss->GetTypeSystemSwiftTypeRef();
+      if (!ts)
+        return false;
+      swift::Demangle::Demangler dem;
+      swift::Demangle::NodePointer node = typeref->getDemangling(dem);
+      dynamic_type = ts->RemangleAsType(dem, node, flavor);
+      dynamic_address = out_address.getAddressData();
     } else {
-      SymbolContextList sc_list;
-      auto &module_list = GetProcess().GetTarget().GetImages();
-      module_list.FindSymbolsWithNameAndType(
-          ConstString(maybe_addr_or_symbol->getSymbol()), eSymbolTypeAny,
-          sc_list);
-      if (sc_list.GetSize() != 1)
+      // In the embedded Swift case, the existential container just points to
+      // the instance.
+      auto reflection_ctx = GetReflectionContext();
+      if (!reflection_ctx)
+        return false;
+      auto maybe_addr_or_symbol =
+          reflection_ctx->ReadPointer(existential_address);
+      if (!maybe_addr_or_symbol)
         return false;
 
-      SymbolContext sc = sc_list[0];
-      Symbol *symbol = sc.symbol;
-      address = symbol->GetLoadAddress(&GetProcess().GetTarget());
-    }
+      uint64_t address = 0;
+      if (maybe_addr_or_symbol->isResolved()) {
+        address = maybe_addr_or_symbol->getOffset();
+      } else {
+        SymbolContextList sc_list;
+        auto &module_list = GetProcess().GetTarget().GetImages();
+        module_list.FindSymbolsWithNameAndType(
+            ConstString(maybe_addr_or_symbol->getSymbol()), eSymbolTypeAny,
+            sc_list);
+        if (sc_list.GetSize() != 1)
+          return false;
+
+        SymbolContext sc = sc_list[0];
+        Symbol *symbol = sc.symbol;
+        address = symbol->GetLoadAddress(&GetProcess().GetTarget());
+      }
 
-    dynamic_type =
-        GetDynamicTypeAndAddress_EmbeddedClass(address, existential_type);
-    if (!dynamic_type)
-      return false;
-    dynamic_address = maybe_addr_or_symbol->getOffset();
+      dynamic_type =
+          GetDynamicTypeAndAddress_EmbeddedClass(address, existential_type);
+      if (!dynamic_type)
+        return false;
+      dynamic_address = maybe_addr_or_symbol->getOffset();
+    }
+    class_type_or_name.SetCompilerType(dynamic_type);
+    address.SetRawAddress(dynamic_address);
   }
-  if (use_local_buffer)
-    PopLocalBuffer();
-
-  class_type_or_name.SetCompilerType(dynamic_type);
-  address.SetRawAddress(dynamic_address);
 
 #ifndef NDEBUG
   if (ModuleList::GetGlobalModuleListProperties()
@@ -2893,8 +2891,9 @@ Value::ValueType SwiftLanguageRuntime::GetValueType(
         existential_address = in_value.GetAddressOf();
       }
 
+      MemoryReaderLocalBufferHolder holder;
       if (use_local_buffer)
-        PushLocalBuffer(
+        holder = PushLocalBuffer(
             existential_address,
             llvm::expectedToOptional(in_value.GetByteSize()).value_or(0));
 
@@ -2906,8 +2905,6 @@ Value::ValueType SwiftLanguageRuntime::GetValueType(
             reflection_ctx->IsValueInlinedInExistentialContainer(
                 remote_existential);
 
-        if (use_local_buffer)
-          PopLocalBuffer();
 
         // An error has occurred when trying to read value witness table,
         // default to treating it as pointer.

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeRemoteAST.cpp

@@ -246,15 +246,14 @@ SwiftLanguageRuntime::GetDynamicTypeAndAddress_ExistentialRemoteAST(
           .value_or(swift::Type());
   if (!swift_type)
     return {};
+  MemoryReaderLocalBufferHolder holder;
   if (use_local_buffer)
-    PushLocalBuffer(
+    holder = PushLocalBuffer(
         existential_address,
         llvm::expectedToOptional(in_value.GetByteSize()).value_or(0));
 
   auto result = remote_ast.getDynamicTypeAndAddressForExistential(
       remote_existential, swift_type);
-  if (use_local_buffer)
-    PopLocalBuffer();
 
   if (!result.isSuccess())
     return {};

lldb/test/API/lang/swift/orig_defined_payload/Makefile

@@ -0,0 +1,3 @@
+SWIFT_SOURCES := main.swift
+
+include Makefile.rules

lldb/test/API/lang/swift/orig_defined_payload/TestSwiftOriginallyDefinedInPayload.py

@@ -0,0 +1,29 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+import os
+
+
+class TestSwiftOriginallyDefinedInPayload(TestBase):
+    @swiftTest
+    def test(self):
+        self.build()
+
+        # rdar://151579199
+        self.runCmd("setting set symbols.swift-validate-typesystem false")
+
+        lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.swift")
+        )
+
+        self.expect(
+            "frame variable c",
+            substrs=[
+                "c = 1 value",
+                "flexible = {",
+                "0 = (value = 100)",
+                "1 = (value = 200)",
+                " 2 = (value = 300)",
+            ],
+        )