[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Thanks for getting to this. I had a diff going a few weeks ago, but never got things fully working!



================
Comment at: lldb/include/lldb/Symbol/ObjectFile.h:465-479
+  /// Extracts the rendezvous structure address from r_debug.
+  ///
+  /// Especially in the case of running a program through the interpreter, the
+  /// GetImageInfoAddress is not able to extract the address. However, the
+  /// rendezvous address in that case can be extracted from the load address of
+  /// _r_debug symbol. This method returns the address of such a structure if
+  /// the information can be resolved via entries in the object file.
----------------
No need to add this to the ObjectFile class, just search for the symbol using the object file you already have in DYLDRendezvous.cpp. See comments down below.

We try to keep ObjectFile as an abstraction that sits on top of any object file type (ELF, mach-p, COFF), and this one is very specific to ELF. I would rather keep this out of the ObjectFile API, and we have an easy work around where we just search for the symbol by name in the module anyway. 


================
Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:59-60
       } else {
-        LLDB_LOGF(log,
-                  "%s FAILED - direct object file approach did not yield a "
-                  "valid address",
-                  __FUNCTION__);
+        Address rendezvous_addr =
+            obj_file->GetRendezvousStructureAddress(target);
+        if (rendezvous_addr.IsValid())
----------------
Inline contents of ObjectFileELF::GetRendezvousStructureAddress(...) to here. We already have the object file we need, so we can just extract the symbol from here. 

I had started to try and fix this as well a few weeks ago, and I used Module::FindFirstSymbolWithNameAndType(...) to avoid having to get multiple results back:

```
        const Symbol* _r_debug = target->GetExecutableModule()->FindFirstSymbolWithNameAndType(ConstString("_r_debug"));
        if (_r_debug) {
          info_addr = _r_debug->GetAddress().GetLoadAddress(target);
          if (info_addr != LLDB_INVALID_ADDRESS) {
            LLDB_LOGF(log,
                      "%s resolved by finding symbol '_r_debug' whose value is 0x%" PRIx64,
                    __FUNCTION__, info_addr);
            return info_addr;
          }
        }
```


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:885-899
+Address ObjectFileELF::GetRendezvousStructureAddress(Target *target) {
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+    SymbolContextList contexts;
+    SymbolContext context;
+    module_sp->FindSymbolsWithNameAndType(ConstString("_r_debug"),
+                                          eSymbolTypeAny, contexts);
----------------
Revert and move contents to where this was called in DYLDRendezvous.cpp. It is very easy to have the DYLDRendezvous just search for the symbol itself.


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h:134-136
+  lldb_private::Address
+  GetRendezvousStructureAddress(lldb_private::Target *target) override;
+
----------------
Revert


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108061/new/

https://reviews.llvm.org/D108061