diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -680,10 +680,19 @@ /// shared library load state. /// /// \return - /// The number of shared libraries that were loaded - virtual size_t LoadModules() { return 0; } + /// A status object indicating if the operation was sucessful or not. + virtual llvm::Error LoadModules() { + return llvm::make_error("Not implemented.", + llvm::inconvertibleErrorCode()); + } - virtual size_t LoadModules(LoadedModuleInfoList &) { return 0; } + /// Query remote GDBServer for a detailed loaded library list + /// \return + /// The list of modules currently loaded by the process, or an error. + virtual llvm::Expected GetLoadedModuleList() { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Not implemented"); + } protected: virtual JITLoaderList &GetJITLoaders(); diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h @@ -222,16 +222,20 @@ /// Updates the current set of SOEntries, the set of added entries, and the /// set of removed entries. - bool UpdateSOEntries(bool fromRemote = false); + bool UpdateSOEntries(); + + /// Same as UpdateSOEntries but it gets the list of loaded modules from the + /// remote debug server (faster when supported). + bool UpdateSOEntriesFromRemote(); bool FillSOEntryFromModuleInfo( LoadedModuleInfoList::LoadedModuleInfo const &modInfo, SOEntry &entry); - bool SaveSOEntriesFromRemote(LoadedModuleInfoList &module_list); + bool SaveSOEntriesFromRemote(const LoadedModuleInfoList &module_list); - bool AddSOEntriesFromRemote(LoadedModuleInfoList &module_list); + bool AddSOEntriesFromRemote(const LoadedModuleInfoList &module_list); - bool RemoveSOEntriesFromRemote(LoadedModuleInfoList &module_list); + bool RemoveSOEntriesFromRemote(const LoadedModuleInfoList &module_list); bool AddSOEntries(); @@ -248,6 +252,17 @@ enum PThreadField { eSize, eNElem, eOffset }; bool FindMetadata(const char *name, PThreadField field, uint32_t &value); + + enum RendezvousAction { + eNoAction, + eTakeSnapshot, + eAddModules, + eRemoveModules + }; + + /// Returns the current action to be taken given the current and previous + /// state + RendezvousAction GetAction() const; }; #endif diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp @@ -160,7 +160,10 @@ m_previous = m_current; m_current = info; - if (UpdateSOEntries(true)) + if (m_current.map_addr == 0) + return false; + + if (UpdateSOEntriesFromRemote()) return true; return UpdateSOEntries(); @@ -170,53 +173,89 @@ return m_rendezvous_addr != LLDB_INVALID_ADDRESS; } -bool DYLDRendezvous::UpdateSOEntries(bool fromRemote) { - SOEntry entry; - LoadedModuleInfoList module_list; +DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const { + 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: + return eTakeSnapshot; + // If we are about to add or remove a shared object clear out the current + // state and take a snapshot of the currently loaded images. + case eAdd: + return eAddModules; + case eDelete: + return eRemoveModules; + } + break; - // If we can't get the SO info from the remote, return failure. - if (fromRemote && m_process->LoadModules(module_list) == 0) - return false; + case eAdd: + case eDelete: + // Some versions of the android dynamic linker might send two + // notifications with state == eAdd back to back. Ignore them until we + // get an eConsistent notification. + if (!(m_previous.state == eConsistent || + (m_previous.state == eAdd && m_current.state == eDelete))) + return eNoAction; - if (!fromRemote && m_current.map_addr == 0) + return eTakeSnapshot; + } + + return eNoAction; +} + +bool DYLDRendezvous::UpdateSOEntriesFromRemote() { + auto action = GetAction(); + + if (action == eNoAction) return false; - // 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. - if (m_previous.state == eConsistent && m_current.state == eConsistent) - return fromRemote ? SaveSOEntriesFromRemote(module_list) - : TakeSnapshot(m_soentries); - - // If we are about to add or remove a shared object clear out the current - // state and take a snapshot of the currently loaded images. - if (m_current.state == eAdd || m_current.state == eDelete) { - // Some versions of the android dynamic linker might send two notifications - // with state == eAdd back to back. Ignore them until we get an eConsistent - // notification. - if (!(m_previous.state == eConsistent || - (m_previous.state == eAdd && m_current.state == eDelete))) - return false; + if (action == eTakeSnapshot) { + m_added_soentries.clear(); + m_removed_soentries.clear(); + // We already have the loaded list from the previous update so no need to + // find all the modules again. + if (!m_loaded_modules.m_list.empty()) + return true; + } + + llvm::Expected module_list = + m_process->GetLoadedModuleList(); + if (!module_list) { + llvm::consumeError(module_list.takeError()); + return false; + } + switch (action) { + case eTakeSnapshot: m_soentries.clear(); - if (fromRemote) - return SaveSOEntriesFromRemote(module_list); + return SaveSOEntriesFromRemote(*module_list); + case eAddModules: + return AddSOEntriesFromRemote(*module_list); + case eRemoveModules: + return RemoveSOEntriesFromRemote(*module_list); + case eNoAction: + return false; + } +} +bool DYLDRendezvous::UpdateSOEntries() { + switch (GetAction()) { + case eTakeSnapshot: + m_soentries.clear(); m_added_soentries.clear(); m_removed_soentries.clear(); return TakeSnapshot(m_soentries); + case eAddModules: + return AddSOEntries(); + case eRemoveModules: + return RemoveSOEntries(); + case eNoAction: + return false; } - assert(m_current.state == eConsistent); - - // Otherwise check the previous state to determine what to expect and update - // accordingly. - if (m_previous.state == eAdd) - return fromRemote ? AddSOEntriesFromRemote(module_list) : AddSOEntries(); - else if (m_previous.state == eDelete) - return fromRemote ? RemoveSOEntriesFromRemote(module_list) - : RemoveSOEntries(); - - return false; } bool DYLDRendezvous::FillSOEntryFromModuleInfo( @@ -247,7 +286,7 @@ } bool DYLDRendezvous::SaveSOEntriesFromRemote( - LoadedModuleInfoList &module_list) { + const LoadedModuleInfoList &module_list) { for (auto const &modInfo : module_list.m_list) { SOEntry entry; if (!FillSOEntryFromModuleInfo(modInfo, entry)) @@ -262,7 +301,8 @@ return true; } -bool DYLDRendezvous::AddSOEntriesFromRemote(LoadedModuleInfoList &module_list) { +bool DYLDRendezvous::AddSOEntriesFromRemote( + const LoadedModuleInfoList &module_list) { for (auto const &modInfo : module_list.m_list) { bool found = false; for (auto const &existing : m_loaded_modules.m_list) { @@ -280,8 +320,10 @@ return false; // Only add shared libraries and not the executable. - if (!SOEntryIsMainExecutable(entry)) + if (!SOEntryIsMainExecutable(entry)) { m_soentries.push_back(entry); + m_added_soentries.push_back(entry); + } } m_loaded_modules = module_list; @@ -289,7 +331,7 @@ } bool DYLDRendezvous::RemoveSOEntriesFromRemote( - LoadedModuleInfoList &module_list) { + const LoadedModuleInfoList &module_list) { for (auto const &existing : m_loaded_modules.m_list) { bool found = false; for (auto const &modInfo : module_list.m_list) { @@ -313,6 +355,7 @@ return false; m_soentries.erase(pos); + m_removed_soentries.push_back(entry); } } diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -94,7 +94,8 @@ __FUNCTION__, m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID); // ask the process if it can load any of its own modules - m_process->LoadModules(); + auto error = m_process->LoadModules(); + LLDB_LOG_ERROR(log, std::move(error), "Couldn't load modules: {0}"); ModuleSP executable_sp = GetTargetExecutable(); ResolveExecutableModule(executable_sp); diff --git a/lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp b/lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp --- a/lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp @@ -146,7 +146,8 @@ ModuleList module_list; module_list.Append(executable); m_process->GetTarget().ModulesDidLoad(module_list); - m_process->LoadModules(); + auto error = m_process->LoadModules(); + LLDB_LOG_ERROR(log, std::move(error), "failed to load modules: {0}"); } void DynamicLoaderWindowsDYLD::DidLaunch() { @@ -165,7 +166,8 @@ ModuleList module_list; module_list.Append(executable); m_process->GetTarget().ModulesDidLoad(module_list); - m_process->LoadModules(); + auto error = m_process->LoadModules(); + LLDB_LOG_ERROR(log, std::move(error), "failed to load modules: {0}"); } } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -200,9 +200,9 @@ llvm::VersionTuple GetHostOSVersion() override; - size_t LoadModules(LoadedModuleInfoList &module_list) override; + llvm::Error LoadModules() override; - size_t LoadModules() override; + llvm::Expected GetLoadedModuleList() override; Status GetFileLoadAddress(const FileSpec &file, bool &is_loaded, lldb::addr_t &load_addr) override; @@ -391,9 +391,6 @@ // Query remote GDBServer for register information bool GetGDBServerRegisterInfo(ArchSpec &arch); - // Query remote GDBServer for a detailed loaded library list - Status GetLoadedModuleList(LoadedModuleInfoList &); - lldb::ModuleSP LoadModuleAtAddress(const FileSpec &file, lldb::addr_t link_map, lldb::addr_t base_addr, diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2390,7 +2390,12 @@ ostr.Printf("%" PRIu64 " %" PRIu32, wp_addr, wp_index); description = ostr.GetString(); } else if (key.compare("library") == 0) { - LoadModules(); + auto error = LoadModules(); + if (error) { + Log *log( + ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); + LLDB_LOG_ERROR(log, std::move(error), "Failed to load modules: {0}"); + } } else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) { uint32_t reg = UINT32_MAX; if (!key.getAsInteger(16, reg)) @@ -2742,9 +2747,13 @@ // the loaded module list can also provides a link map address if (addr == LLDB_INVALID_ADDRESS) { - LoadedModuleInfoList list; - if (GetLoadedModuleList(list).Success()) - addr = list.m_link_map; + llvm::Expected list = GetLoadedModuleList(); + if (!list) { + Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); + LLDB_LOG_ERROR(log, list.takeError(), "Failed to read module list: {0}"); + } else { + addr = list->m_link_map; + } } return addr; @@ -4682,39 +4691,43 @@ return m_register_info.GetNumRegisters() > 0; } -Status ProcessGDBRemote::GetLoadedModuleList(LoadedModuleInfoList &list) { +llvm::Expected ProcessGDBRemote::GetLoadedModuleList() { // Make sure LLDB has an XML parser it can use first if (!XMLDocument::XMLEnabled()) - return Status(0, ErrorType::eErrorTypeGeneric); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "XML parsing not available"); Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS); LLDB_LOGF(log, "ProcessGDBRemote::%s", __FUNCTION__); + LoadedModuleInfoList list; GDBRemoteCommunicationClient &comm = m_gdb_comm; bool can_use_svr4 = GetGlobalPluginProperties()->GetUseSVR4(); // check that we have extended feature read support if (can_use_svr4 && comm.GetQXferLibrariesSVR4ReadSupported()) { - list.clear(); - // request the loaded library list std::string raw; lldb_private::Status lldberr; if (!comm.ReadExtFeature(ConstString("libraries-svr4"), ConstString(""), raw, lldberr)) - return Status(0, ErrorType::eErrorTypeGeneric); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Error in libraries-svr4 packet"); // parse the xml file in memory LLDB_LOGF(log, "parsing: %s", raw.c_str()); XMLDocument doc; if (!doc.ParseMemory(raw.c_str(), raw.size(), "noname.xml")) - return Status(0, ErrorType::eErrorTypeGeneric); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Error reading noname.xml"); XMLNode root_element = doc.GetRootElement("library-list-svr4"); if (!root_element) - return Status(); + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Error finding library-list-svr4 xml element"); // main link map structure llvm::StringRef main_lm = root_element.GetAttributeValue("main-lm"); @@ -4778,28 +4791,31 @@ // node }); - LLDB_LOGF(log, "found %" PRId32 " modules in total", - (int)list.m_list.size()); + if (log) + LLDB_LOGF(log, "found %" PRId32 " modules in total", + (int)list.m_list.size()); + return list; } else if (comm.GetQXferLibrariesReadSupported()) { - list.clear(); - // request the loaded library list std::string raw; lldb_private::Status lldberr; if (!comm.ReadExtFeature(ConstString("libraries"), ConstString(""), raw, lldberr)) - return Status(0, ErrorType::eErrorTypeGeneric); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Error in libraries packet"); LLDB_LOGF(log, "parsing: %s", raw.c_str()); XMLDocument doc; if (!doc.ParseMemory(raw.c_str(), raw.size(), "noname.xml")) - return Status(0, ErrorType::eErrorTypeGeneric); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Error reading noname.xml"); XMLNode root_element = doc.GetRootElement("library-list"); if (!root_element) - return Status(); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Error finding library-list xml element"); root_element.ForEachChildElementWithName( "library", [log, &list](const XMLNode &library) -> bool { @@ -4836,13 +4852,14 @@ // node }); - LLDB_LOGF(log, "found %" PRId32 " modules in total", - (int)list.m_list.size()); + if (log) + LLDB_LOGF(log, "found %" PRId32 " modules in total", + (int)list.m_list.size()); + return list; } else { - return Status(0, ErrorType::eErrorTypeGeneric); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Remote libraries not supported"); } - - return Status(); } lldb::ModuleSP ProcessGDBRemote::LoadModuleAtAddress(const FileSpec &file, @@ -4857,17 +4874,18 @@ value_is_offset); } -size_t ProcessGDBRemote::LoadModules(LoadedModuleInfoList &module_list) { +llvm::Error ProcessGDBRemote::LoadModules() { using lldb_private::process_gdb_remote::ProcessGDBRemote; // request a list of loaded libraries from GDBServer - if (GetLoadedModuleList(module_list).Fail()) - return 0; + llvm::Expected module_list = GetLoadedModuleList(); + if (!module_list) + return module_list.takeError(); // get a list of all the modules ModuleList new_modules; - for (LoadedModuleInfoList::LoadedModuleInfo &modInfo : module_list.m_list) { + for (LoadedModuleInfoList::LoadedModuleInfo &modInfo : module_list->m_list) { std::string mod_name; lldb::addr_t mod_base; lldb::addr_t link_map; @@ -4934,12 +4952,7 @@ m_process->GetTarget().ModulesDidLoad(new_modules); } - return new_modules.GetSize(); -} - -size_t ProcessGDBRemote::LoadModules() { - LoadedModuleInfoList module_list; - return LoadModules(module_list); + return llvm::ErrorSuccess(); } Status ProcessGDBRemote::GetFileLoadAddress(const FileSpec &file,