[Reflection] Update MemoryReader to distinguish related operations (swiftlang#41530)

Until recently, `MemoryReader` had a single function `resovlePointer` which did two things, and has a somewhat vague name. The two things were:

1. Tool-specific mapping between real addresses and tagged addresses (first implemented in `swift-reflection-dump` and then later in lldb)
2. Finding a "symbol" for a given address

Recently, `resolvePointerAsSymbol` was added, which overloaded the term "resolve" and it added another way to deal with symbols for addresses. Symbols themselves were a bit muddled, as `swift-reflection-dump` was dealing with dynamic symbols aka bindings, while lldb was dealing in regular (static) symbols.

This change separates these two parts of functionality, and also divides symbol lookup into two cases. The API surface will now be:

1. `resolvePointer` for mapping/tagging addresses
3. `getSymbol` for looking up a symbol for an address
4. `getDynamicSymbol` for looking up a dyld binding for an address

Note: each of these names could be improved. Some alternative terms: `lookup` instead of `get`, `Binding` or `BindingName` instead of `DynamicSymbol`. Maybe even another term instead of "resolve". Suggestions welcome!

Currently, `swift-reflection-dump` supports `getDynamicSymbol` but not `getSymbol`. For lldb it's the reverse, `getSymbol` is supported but `getDynamicSymbol` needs to be implemented.

For everything but lldb, this change is NFC. For lldb it fixes a bug where `LLDBMemoryReader` returns regular symbols where we should instead be returning dynamic symbols.

Author: kastiglione

include/swift/Remote/MemoryReader.h

@@ -149,19 +149,32 @@ class MemoryReader {
     return RemoteAbsolutePointer("", readValue);
   }
 
-  /// Atempt to resolve the pointer to a symbol for the given remote address.
   virtual llvm::Optional<RemoteAbsolutePointer>
   resolvePointerAsSymbol(RemoteAddress address) {
     return llvm::None;
   }
 
+  /// Lookup a symbol for the given remote address.
+  virtual RemoteAbsolutePointer getSymbol(RemoteAddress address) {
+    if (auto symbol = resolvePointerAsSymbol(address))
+      return *symbol;
+    return RemoteAbsolutePointer("", address.getAddressData());
+  }
+
+  /// Lookup a dynamic symbol name (ie dynamic loader binding) for the given
+  /// remote address. Note: An address can be referenced by both dynamic and
+  /// regular symbols, this function must return a dynamic symbol only.
+  virtual RemoteAbsolutePointer getDynamicSymbol(RemoteAddress address) {
+    return nullptr;
+  }
+
   /// Attempt to read and resolve a pointer value at the given remote address.
   llvm::Optional<RemoteAbsolutePointer> readPointer(RemoteAddress address,
                                                     unsigned pointerSize) {
-    // Try to resolve the pointer as a symbol first, as reading memory
-    // may potentially be expensive.
-    if (auto symbolPointer = resolvePointerAsSymbol(address))
-      return symbolPointer;
+    // First, try to lookup the pointer as a dynamic symbol (binding), as
+    // reading memory may potentially be expensive.
+    if (auto dynamicSymbol = getDynamicSymbol(address))
+      return dynamicSymbol;
 
     auto result = readBytes(address, pointerSize);
     if (!result)

include/swift/Remote/MetadataReader.h

@@ -420,11 +420,7 @@ class MetadataReader {
           return nullptr;
         }
       } else {
-        resolved = Reader->resolvePointer(RemoteAddress(remoteAddress), 0);
-        if (resolved.getSymbol().empty()) {
-          // No symbol found, use the already calculated address.
-          resolved = RemoteAbsolutePointer("", remoteAddress);
-        }
+        resolved = Reader->getSymbol(RemoteAddress(remoteAddress));
       }
 
       switch (kind) {

include/swift/StaticMirror/ObjectFileContext.h

@@ -70,6 +70,8 @@ class Image {
 
   remote::RemoteAbsolutePointer resolvePointer(uint64_t Addr,
                                                uint64_t pointerValue) const;
+
+  remote::RemoteAbsolutePointer getDynamicSymbol(uint64_t Addr) const;
 };
 
 /// MemoryReader that reads from the on-disk representation of an executable
@@ -118,6 +120,9 @@ class ObjectMemoryReader : public reflection::MemoryReader {
 
   remote::RemoteAbsolutePointer resolvePointer(reflection::RemoteAddress Addr,
                                                uint64_t pointerValue) override;
+
+  remote::RemoteAbsolutePointer
+  getDynamicSymbol(reflection::RemoteAddress Addr) override;
 };
 
 using ReflectionContextOwner = std::unique_ptr<void, void (*)(void *)>;

lib/StaticMirror/ObjectFileContext.cpp

@@ -279,22 +279,23 @@ StringRef Image::getContentsAtAddress(uint64_t Addr, uint64_t Size) const {
 
 remote::RemoteAbsolutePointer
 Image::resolvePointer(uint64_t Addr, uint64_t pointerValue) const {
+  // In Mach-O images with ptrauth, the pointer value has an offset from the
+  // base address in the low 32 bits, and ptrauth discriminator info in the top
+  // 32 bits.
+  if (isMachOWithPtrAuth()) {
+    return remote::RemoteAbsolutePointer(
+        "", HeaderAddress + (pointerValue & 0xffffffffull));
+  } else {
+    return remote::RemoteAbsolutePointer("", pointerValue);
+  }
+}
+
+remote::RemoteAbsolutePointer Image::getDynamicSymbol(uint64_t Addr) const {
   auto found = DynamicRelocations.find(Addr);
-  remote::RemoteAbsolutePointer result;
   if (found == DynamicRelocations.end())
-    // In Mach-O images with ptrauth, the pointer value has an offset from
-    // the base address in the low 32 bits, and ptrauth discriminator info
-    // in the top 32 bits.
-    if (isMachOWithPtrAuth()) {
-      result = remote::RemoteAbsolutePointer(
-          "", HeaderAddress + (pointerValue & 0xffffffffull));
-    } else {
-      result = remote::RemoteAbsolutePointer("", pointerValue);
-    }
-  else
-    result = remote::RemoteAbsolutePointer(found->second.Symbol,
-                                           found->second.Offset);
-  return result;
+    return nullptr;
+  return remote::RemoteAbsolutePointer(found->second.Symbol,
+                                       found->second.Offset);
 }
 
 std::pair<const Image *, uint64_t>
@@ -482,16 +483,24 @@ ObjectMemoryReader::resolvePointer(reflection::RemoteAddress Addr,
     return remote::RemoteAbsolutePointer();
 
   auto resolved = image->resolvePointer(imageAddr, pointerValue);
+  // Mix in the image index again to produce a remote address pointing into the
+  // same image.
+  return remote::RemoteAbsolutePointer(
+      "", encodeImageIndexAndAddress(
+              image, resolved.getResolvedAddress().getAddressData()));
+}
 
-  if (resolved && resolved.isResolved()) {
-    // Mix in the image index again to produce a remote address pointing into
-    // the same image.
-    return remote::RemoteAbsolutePointer(
-        "", encodeImageIndexAndAddress(
-                image, resolved.getResolvedAddress().getAddressData()));
-  }
-  // If the pointer is relative to an unresolved relocation, leave it as is.
-  return resolved;
+remote::RemoteAbsolutePointer
+ObjectMemoryReader::getDynamicSymbol(reflection::RemoteAddress Addr) {
+  auto addrValue = Addr.getAddressData();
+  const Image *image;
+  uint64_t imageAddr;
+  std::tie(image, imageAddr) = decodeImageIndexAndAddress(addrValue);
+
+  if (!image)
+    return nullptr;
+
+  return image->getDynamicSymbol(imageAddr);
 }
 
 template <typename Runtime>