Revert "Allow firmware binaries to be specified only by load address"

This reverts commit d8879fb.

Debian bot failure; I included <uuid/uuid.h> to get uuid_is_null() but
don't get it there.  Will memcmp or whatever & recommit.

Author: jasonmolenda

lldb/docs/lldb-gdb-remote.txt

@@ -1073,12 +1073,6 @@ main-binary-uuid: is the UUID of a firmware type binary that the gdb stub knows
 main-binary-address: is the load address of the firmware type binary
 main-binary-slide: is the slide of the firmware type binary, if address isn't known
 
-binary-addresses: A comma-separated list of binary load addresses base16.  
-                  lldb will parse the binaries in memory to get UUIDs, then
-                  try to find the binaries & debug info by UUID.  Intended for
-                  use with a small number of firmware type binaries where the 
-                  search for binary/debug info may be expensive.
-
 //----------------------------------------------------------------------
 // "qShlibInfoAddr"
 //

lldb/include/lldb/Target/DynamicLoader.h

@@ -210,52 +210,6 @@ class DynamicLoader : public PluginInterface {
                                              lldb::addr_t base_addr,
                                              bool base_addr_is_offset);
 
-  /// Find/load a binary into lldb given a UUID and the address where it is
-  /// loaded in memory, or a slide to be applied to the file address.
-  /// May force an expensive search on the computer to find the binary by
-  /// UUID, should not be used for a large number of binaries - intended for
-  /// an environment where there may be one, or a few, binaries resident in
-  /// memory.
-  ///
-  /// Given a UUID, search for a binary and load it at the address provided,
-  /// or with the slide applied, or at the file address unslid.
-  ///
-  /// Given an address, try to read the binary out of memory, get the UUID,
-  /// find the file if possible and load it unslid, or add the memory module.
-  ///
-  /// \param[in] process
-  ///     The process to add this binary to.
-  ///
-  /// \param[in] uuid
-  ///     UUID of the binary to be loaded.  UUID may be empty, and if a
-  ///     load address is supplied, will read the binary from memory, get
-  ///     a UUID and try to find a local binary.  There is a performance
-  ///     cost to doing this, it is not preferable.
-  ///
-  /// \param[in] value
-  ///     Address where the binary should be loaded, or read out of memory.
-  ///     Or a slide value, to be applied to the file addresses of the binary.
-  ///
-  /// \param[in] value_is_offset
-  ///     A flag indicating that \p value is an address, or an offset to
-  ///     be applied to the file addresses.
-  ///
-  /// \param[in] force_symbol_search
-  ///     Allow the search to do a possibly expensive external search for
-  ///     the ObjectFile and/or SymbolFile.
-  ///
-  /// \param[in] notify
-  ///     Whether ModulesDidLoad should be called when a binary has been added
-  ///     to the Target.  The caller may prefer to batch up these when loading
-  ///     multiple binaries.
-  ///
-  /// \return
-  ///     Returns a shared pointer for the Module that has been added.
-  static lldb::ModuleSP
-  LoadBinaryWithUUIDAndAddress(Process *process, UUID uuid, lldb::addr_t value,
-                               bool value_is_offset, bool force_symbol_search,
-                               bool notify);
-
   /// Get information about the shared cache for a process, if possible.
   ///
   /// On some systems (e.g. Darwin based systems), a set of libraries that are

lldb/source/Core/DynamicLoader.cpp

@@ -13,15 +13,11 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
-#include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/ConstString.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "lldb/Utility/Log.h"
 #include "lldb/lldb-private-interfaces.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -175,100 +171,6 @@ ModuleSP DynamicLoader::LoadModuleAtAddress(const FileSpec &file,
   return nullptr;
 }
 
-static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr) {
-  char namebuf[80];
-  snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr);
-  return process->ReadModuleFromMemory(FileSpec(namebuf), addr);
-}
-
-ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(Process *process,
-                                                     UUID uuid, addr_t value,
-                                                     bool value_is_offset,
-                                                     bool force_symbol_search,
-                                                     bool notify) {
-  ModuleSP memory_module_sp;
-  ModuleSP module_sp;
-  PlatformSP platform_sp = process->GetTarget().GetPlatform();
-  Target &target = process->GetTarget();
-  Status error;
-  ModuleSpec module_spec;
-  module_spec.GetUUID() = uuid;
-
-  if (!uuid.IsValid() && !value_is_offset) {
-    memory_module_sp = ReadUnnamedMemoryModule(process, value);
-
-    if (memory_module_sp)
-      uuid = memory_module_sp->GetUUID();
-  }
-
-  if (uuid.IsValid()) {
-    ModuleSpec module_spec;
-    module_spec.GetUUID() = uuid;
-
-    if (!module_sp)
-      module_sp = target.GetOrCreateModule(module_spec, false, &error);
-
-    // If we haven't found a binary, or we don't have a SymbolFile, see
-    // if there is an external search tool that can find it.
-    if (force_symbol_search &&
-        (!module_sp || !module_sp->GetSymbolFileFileSpec())) {
-      Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
-      if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-        module_sp = std::make_shared<Module>(module_spec);
-      }
-    }
-  }
-
-  // If we couldn't find the binary anywhere else, as a last resort,
-  // read it out of memory.
-  if (!module_sp.get() && value != LLDB_INVALID_ADDRESS && !value_is_offset) {
-    if (!memory_module_sp)
-      memory_module_sp = ReadUnnamedMemoryModule(process, value);
-    if (memory_module_sp)
-      module_sp = memory_module_sp;
-  }
-
-  Log *log = GetLog(LLDBLog::DynamicLoader);
-  if (module_sp.get()) {
-    target.GetImages().AppendIfNeeded(module_sp, false);
-
-    bool changed = false;
-    if (module_sp->GetObjectFile()) {
-      if (value != LLDB_INVALID_ADDRESS) {
-        LLDB_LOGF(log, "Loading binary UUID %s at %s 0x%" PRIx64,
-                  uuid.GetAsString().c_str(),
-                  value_is_offset ? "offset" : "address", value);
-        module_sp->SetLoadAddress(target, value, value_is_offset, changed);
-      } else {
-        // No address/offset/slide, load the binary at file address,
-        // offset 0.
-        LLDB_LOGF(log, "Loading binary UUID %s at file address",
-                  uuid.GetAsString().c_str());
-        module_sp->SetLoadAddress(target, 0, true /* value_is_slide */,
-                                  changed);
-      }
-    } else {
-      // In-memory image, load at its true address, offset 0.
-      LLDB_LOGF(log, "Loading binary UUID %s from memory at address 0x%" PRIx64,
-                uuid.GetAsString().c_str(), value);
-      module_sp->SetLoadAddress(target, 0, true /* value_is_slide */, changed);
-    }
-
-    if (notify) {
-      ModuleList added_module;
-      added_module.Append(module_sp, false);
-      target.ModulesDidLoad(added_module);
-    }
-  } else {
-    LLDB_LOGF(log, "Unable to find binary with UUID %s and load it at "
-                  "%s 0x%" PRIx64,
-                  uuid.GetAsString().c_str(),
-                  value_is_offset ? "offset" : "address", value);
-  }
-
-  return module_sp;
-}
-
 int64_t DynamicLoader::ReadUnsignedIntWithSizeInBytes(addr_t addr,
                                                       int size_in_bytes) {
   Status error;

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -5600,8 +5600,7 @@ bool ObjectFileMachO::GetCorefileMainBinaryInfo(addr_t &value,
             }
 
             if (m_data.CopyData(offset, sizeof(uuid_t), raw_uuid) != 0) {
-              if (!uuid_is_null(raw_uuid))
-                uuid = UUID::fromOptionalData(raw_uuid, sizeof(uuid_t));
+              uuid = UUID::fromOptionalData(raw_uuid, sizeof(uuid_t));
               // convert the "main bin spec" type into our
               // ObjectFile::BinaryType enum
               switch (binspec_type) {
@@ -6902,8 +6901,7 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
 
           MachOCorefileImageEntry image_entry;
           image_entry.filename = (const char *)m_data.GetCStr(&filepath_offset);
-          if (!uuid_is_null(uuid))
-            image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
+          image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
           image_entry.load_address = load_address;
           image_entry.currently_executing = currently_executing;
 
@@ -6934,11 +6932,9 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
 
           MachOCorefileImageEntry image_entry;
           image_entry.filename = filename;
-          if (!uuid_is_null(uuid))
-            image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
+          image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
           image_entry.load_address = load_address;
           image_entry.slide = slide;
-          image_entry.currently_executing = true;
           image_infos.all_image_infos.push_back(image_entry);
         }
       }
@@ -6955,41 +6951,42 @@ bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
 
   ModuleList added_modules;
   for (const MachOCorefileImageEntry &image : image_infos.all_image_infos) {
-    ModuleSP module_sp;
-
-    if (!image.filename.empty()) {
-      Status error;
-      ModuleSpec module_spec;
-      module_spec.GetUUID() = image.uuid;
+    ModuleSpec module_spec;
+    module_spec.GetUUID() = image.uuid;
+    if (image.filename.empty()) {
+      char namebuf[80];
+      if (image.load_address != LLDB_INVALID_ADDRESS)
+        snprintf(namebuf, sizeof(namebuf), "mem-image-0x%" PRIx64,
+                 image.load_address);
+      else
+        snprintf(namebuf, sizeof(namebuf), "mem-image+0x%" PRIx64, image.slide);
+      module_spec.GetFileSpec() = FileSpec(namebuf);
+    } else {
       module_spec.GetFileSpec() = FileSpec(image.filename.c_str());
-      if (image.currently_executing) {
-        Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
-        if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-          process.GetTarget().GetOrCreateModule(module_spec, false);
-        }
+    }
+    if (image.currently_executing) {
+      Status error;
+      Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
+      if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+        process.GetTarget().GetOrCreateModule(module_spec, false);
       }
-      module_sp =
-          process.GetTarget().GetOrCreateModule(module_spec, false, &error);
-      process.GetTarget().GetImages().AppendIfNeeded(module_sp,
-                                                     false /* notify */);
-    } else {
+    }
+    Status error;
+    ModuleSP module_sp =
+        process.GetTarget().GetOrCreateModule(module_spec, false, &error);
+    if (!module_sp.get() || !module_sp->GetObjectFile()) {
       if (image.load_address != LLDB_INVALID_ADDRESS) {
-        module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
-            &process, image.uuid, image.load_address,
-            false /* value_is_offset */, image.currently_executing,
-            false /* notify */);
-      } else if (image.slide != LLDB_INVALID_ADDRESS) {
-        module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
-            &process, image.uuid, image.slide, true /* value_is_offset */,
-            image.currently_executing, false /* notify */);
+        module_sp = process.ReadModuleFromMemory(module_spec.GetFileSpec(),
+                                                 image.load_address);
       }
     }
-
     if (module_sp.get()) {
       // Will call ModulesDidLoad with all modules once they've all
       // been added to the Target with load addresses.  Don't notify
       // here, before the load address is set.
-      added_modules.Append(module_sp, false /* notify */);
+      const bool notify = false;
+      process.GetTarget().GetImages().AppendIfNeeded(module_sp, notify);
+      added_modules.Append(module_sp, notify);
       if (image.segment_load_addresses.size() > 0) {
         if (log) {
           std::string uuidstr = image.uuid.GetAsString();

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

@@ -1033,13 +1033,6 @@ bool GDBRemoteCommunicationClient::GetProcessStandaloneBinary(
   return true;
 }
 
-std::vector<addr_t>
-GDBRemoteCommunicationClient::GetProcessStandaloneBinaries() {
-  if (m_qProcessInfo_is_valid == eLazyBoolCalculate)
-    GetCurrentProcessInfo();
-  return m_binary_addresses;
-}
-
 bool GDBRemoteCommunicationClient::GetGDBServerVersion() {
   if (m_qGDBServerVersion_is_valid == eLazyBoolCalculate) {
     m_gdb_server_name.clear();
@@ -2199,14 +2192,6 @@ bool GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
             m_process_standalone_value_is_offset = false;
             ++num_keys_decoded;
           }
-        } else if (name.equals("binary-addresses")) {
-          addr_t addr;
-          while (!value.empty()) {
-            llvm::StringRef addr_str;
-            std::tie(addr_str, value) = value.split(',');
-            if (!addr_str.getAsInteger(16, addr))
-              m_binary_addresses.push_back(addr);
-          }
         }
       }
       if (num_keys_decoded > 0)

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

@@ -220,8 +220,6 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   bool GetProcessStandaloneBinary(UUID &uuid, lldb::addr_t &value,
                                   bool &value_is_offset);
 
-  std::vector<lldb::addr_t> GetProcessStandaloneBinaries();
-
   void GetRemoteQSupported();
 
   bool GetVContSupported(char flavor);
@@ -595,7 +593,6 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   UUID m_process_standalone_uuid;
   lldb::addr_t m_process_standalone_value = LLDB_INVALID_ADDRESS;
   bool m_process_standalone_value_is_offset = false;
-  std::vector<lldb::addr_t> m_binary_addresses;
   llvm::VersionTuple m_os_version;
   llvm::VersionTuple m_maccatalyst_version;
   std::string m_os_build;