Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

[clayborg]

Summary

Prior to this fix, no shared libraries would be loaded for a core file, even if they exist on the current machine. The issue was the DYLDRendezvous would read a DYLDRendezvous::Rendezvous from memory of the process in DYLDRendezvous::Resolve() which would read some ld.so structures as they existed in the middle of a process' lifetime. In core files we see, the DYLDRendezvous::Rendezvous::state would be set to eAdd for running processes. When ProcessELFCore.cpp would load the core file, it would call DynamicLoaderPOSIXDYLD::DidAttach(), which would call the above Rendezvous functions. The issue came when during the DidAttach function it call DYLDRendezvous::GetAction() which would return eNoAction if the DYLDRendezvous::m_current.state was read from memory as eAdd. This caused no shared libraries to be loaded for any ELF core files. We now detect if we have a core file and after reading the DYLDRendezvous::m_current.state from memory we set it to eConsistent, which causes DYLDRendezvous::GetAction() to return the correct action of eTakeSnapshot and shared libraries get loaded.

We also improve the DynamicLoaderPOSIXDYLD class to not try and set any breakpoints to catch shared library loads/unloads when we have a core file, which saves a bit of time.

[clayborg]

All current post mortem tests pass with this fix. Not sure how to test this stuff since if a core files have hard coded paths in the rendezvous structures in memory and it isn't possible to make a small core file that contains the right paths.

[yinghuitan]

I think @labath or anyone owns the ELF coredump debugging should take a look. I am surprised other major companies did not hit this issue.

Otherwise, LGTM

[labath]

In D134842#3822740, @yinghuitan wrote:

I am surprised other major companies did not hit this issue.

That could be because this is something specific to your environment. Just to be clear, is this happening for *all* core files or only for some of them? If only some, is there anything special about the state of the applications that produced those core files (e.g. are they in the middle of loading a shared library?)
Even though this may very well be the right fix for middle-of-dlopen core dumps (we can't really wait for the loading to finish), I suspect this is actually masking some other problem, as the amount of time an application spends in the RT_ADD state is very brief, and it shouldn't be doing anything crash-prone while in there.

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp

174–181

How about we just change GetAction to directly return eTakeSnapshot in this case?

[clayborg]

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp

174–181

that is a good idea

[clayborg]

In D134842#3823497, @labath wrote:

In D134842#3822740, @yinghuitan wrote:

I am surprised other major companies did not hit this issue.

That could be because this is something specific to your environment. Just to be clear, is this happening for *all* core files or only for some of them? If only some, is there anything special about the state of the applications that produced those core files (e.g. are they in the middle of loading a shared library?)

These are produced by the linux kernel. If we produce any core files manually we make Minidumps as they contain more useful information. ELF core files are kind of a joke as they don't include any build IDs for the shared libraries so many people often will try to load a core file on another system and they end up just loading the shared libraries from the current machine (like /usr/lib/libc.so etc) and they will never know. ELF core file are only really useful on the same machine or for looking at the main binary's stack frames.

Even though this may very well be the right fix for middle-of-dlopen core dumps (we can't really wait for the loading to finish), I suspect this is actually masking some other problem, as the amount of time an application spends in the RT_ADD state is very brief, and it shouldn't be doing anything crash-prone while in there.

Thanks for the background on the eAdd state, makes more sense, there are no docs on what these states mean so I didn't know what the normal values were. I will check our small core files that are checked in and see if they end up in these states as well and report back. I will also check out the core file that has this issue to see if one of the threads is doing dlopen.

[clayborg]

Switch DYLDRendezvous::GetAction() to return eTakeSnapshot when we have a core file and don't modify m_current.state.

[clayborg]

Fix typo in comment.

[aadsm]

This comment was removed by aadsm.

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h

protected:
   /// Reads the current list of shared objects according to the link map
   /// supplied by the runtime linker.
   bool TakeSnapshot(SOEntryList &entry_list);
 
   enum PThreadField { eSize, eNElem, eOffset };
 
   bool FindMetadata(const char *name, PThreadField field, uint32_t &value);
 
+  bool IsCoreFile() const;
+
   enum RendezvousAction {
     eNoAction,
     eTakeSnapshot,
     eAddModules,
     eRemoveModules
   };
 
   /// Returns the current action to be taken given the current and previous
   /// state
   RendezvousAction GetAction() const;
 };
 
 #endif

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp

if (!(cursor = ReadPointer(cursor, &info.brk)))
     return false;
 
   if (!(cursor = ReadWord(cursor, &info.state, word_size)))
     return false;
 
   if (!(cursor = ReadPointer(cursor + padding, &info.ldbase)))
     return false;
 
   // The rendezvous was successfully read.  Update our internal state.
   m_rendezvous_addr = info_addr;
   m_previous = m_current;
   m_current = info;
 
   if (m_current.map_addr == 0)
     return false;
 
   if (UpdateSOEntriesFromRemote())
     return true;
 
   return UpdateSOEntries();
 }
 
 bool DYLDRendezvous::IsValid() {
   return m_rendezvous_addr != LLDB_INVALID_ADDRESS;
 }
 
 DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const {
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory into m_current which can be in an inconsistent
+  // state, so we can't rely on its state to determine what we should do. We
+  // always need it to load all of the shared libraries one time when we attach
+  // to a core file.
+  if (IsCoreFile())
+    return eTakeSnapshot;
+
   switch (m_current.state) {
 
   case eConsistent:
     switch (m_previous.state) {
     // When the previous and current states are consistent this is the first
     // time we have been asked to update.  Just take a snapshot of the
     // currently loaded modules.
     case eConsistent:
for (int i = 1; I != E; ++I, ++i) {
     LLDB_LOGF(log, "\n   SOEntry [%d] %s", i, I->file_spec.GetPath().c_str());
     LLDB_LOGF(log, "      Base : %" PRIx64, I->base_addr);
     LLDB_LOGF(log, "      Path : %" PRIx64, I->path_addr);
     LLDB_LOGF(log, "      Dyn  : %" PRIx64, I->dyn_addr);
     LLDB_LOGF(log, "      Next : %" PRIx64, I->next);
     LLDB_LOGF(log, "      Prev : %" PRIx64, I->prev);
   }
 }
+
+bool DYLDRendezvous::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h

protected:
 
   /// Contains the pointer to the interpret module, if loaded.
   std::weak_ptr<lldb_private::Module> m_interpreter_module;
 
   /// Loaded module list. (link map for each module)
   std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
       m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
 
   /// Callback routine which updates the current list of loaded modules based
   /// on the information supplied by the runtime linker.
   static bool RendezvousBreakpointHit(
       void *baton, lldb_private::StoppointCallbackContext *context,

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

void DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP module) {
   m_loaded_modules.erase(module);
 
   UnloadSectionsCommon(module);
 }
 
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+    return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
     LLDB_LOGF(
         log,
         "DynamicLoaderPOSIXDYLD::%s pid %" PRIu64
         " GetEntryPoint() returned no address, not setting entry breakpoint",
         __FUNCTION__, m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID);
     return;
bool DynamicLoaderPOSIXDYLD::EntryBreakpointHit(
 
   dyld_instance->LoadAllCurrentModules();
   dyld_instance->SetRendezvousBreakpoint();
   return false; // Continue running.
 }
 
 bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
+
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+    return false;
+
   if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
     LLDB_LOG(log,
              "Rendezvous breakpoint breakpoint id {0} for pid {1}"
              "is already set.",
              m_dyld_bid,
              m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID);
     return true;
   }
bool DynamicLoaderPOSIXDYLD::AlwaysRelyOnEHUnwindInfo(
   if (!module_sp && sym_ctx.function)
     module_sp =
         sym_ctx.function->GetAddressRange().GetBaseAddress().GetModule();
   if (!module_sp)
     return false;
 
   return module_sp->GetFileSpec().GetPath() == "[vdso]";
 }
+
+bool DynamicLoaderPOSIXDYLD::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}