Show error messages from DebugSymbols DBGShellCommand agent

The DebugSymbols DBGShellsCommand, which can find the symbols
for binaries, has a mechanism to return error messages when
it cannot find a symbol file.  Those errors were not printed
to the user for several corefile use case scenarios; this
patch fixes that.

Also add dyld/process logging for the LC_NOTE metadata parsers
in ObjectFileMachO, to help in seeing what lldb is basing its
searches on.

Differential Revision: https://reviews.llvm.org/D157160

Author: jasonmolenda

lldb/source/Core/DynamicLoader.cpp

@@ -8,6 +8,7 @@
 
 #include "lldb/Target/DynamicLoader.h"
 
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -235,6 +236,9 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
                                            force_symbol_search);
       if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
         module_sp = std::make_shared<Module>(module_spec);
+      } else if (force_symbol_search && error.AsCString("") &&
+                 error.AsCString("")[0] != '\0') {
+        target.GetDebugger().GetErrorStream() << error.AsCString();
       }
     }
 
@@ -267,26 +271,26 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
         if (value != LLDB_INVALID_ADDRESS) {
           LLDB_LOGF(log,
                     "DynamicLoader::LoadBinaryWithUUIDAndAddress Loading "
-                    "binary UUID %s at %s 0x%" PRIx64,
-                    uuid.GetAsString().c_str(),
+                    "binary %s UUID %s at %s 0x%" PRIx64,
+                    name.str().c_str(), 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,
                     "DynamicLoader::LoadBinaryWithUUIDAndAddress Loading "
-                    "binary UUID %s at file address",
-                    uuid.GetAsString().c_str());
+                    "binary %s UUID %s at file address",
+                    name.str().c_str(), 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,
                   "DynamicLoader::LoadBinaryWithUUIDAndAddress Loading binary "
-                  "UUID %s from memory at address 0x%" PRIx64,
-                  uuid.GetAsString().c_str(), value);
+                  "%s UUID %s from memory at address 0x%" PRIx64,
+                  name.str().c_str(), uuid.GetAsString().c_str(), value);
         module_sp->SetLoadAddress(target, 0, true /* value_is_slide */,
                                   changed);
       }
@@ -298,10 +302,26 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
       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);
+    if (force_symbol_search) {
+      Stream &s = target.GetDebugger().GetErrorStream();
+      s.Printf("Unable to find file");
+      if (!name.empty())
+        s.Printf(" %s", name.str().c_str());
+      if (uuid.IsValid())
+        s.Printf(" with UUID %s", uuid.GetAsString().c_str());
+      if (value != LLDB_INVALID_ADDRESS) {
+        if (value_is_offset)
+          s.Printf(" with slide 0x%" PRIx64, value);
+        else
+          s.Printf(" at address 0x%" PRIx64, value);
+      }
+      s.Printf("\n");
+    }
+    LLDB_LOGF(log,
+              "Unable to find binary %s with UUID %s and load it at "
+              "%s 0x%" PRIx64,
+              name.str().c_str(), uuid.GetAsString().c_str(),
+              value_is_offset ? "offset" : "address", value);
   }
 
   return module_sp;

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

@@ -767,9 +767,10 @@ bool DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule(
       // to do anything useful. This will force a call to dsymForUUID if it
       // exists, instead of depending on the DebugSymbols preferences being
       // set.
+      Status kernel_search_error;
       if (IsKernel()) {
-        Status error;
-        if (Symbols::DownloadObjectAndSymbolFile(module_spec, error, true)) {
+        if (Symbols::DownloadObjectAndSymbolFile(module_spec,
+                                                 kernel_search_error, true)) {
           if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
             m_module_sp = std::make_shared<Module>(module_spec.GetFileSpec(),
                                                    target.GetArchitecture());
@@ -807,9 +808,13 @@ bool DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule(
       }
 
       if (IsKernel() && !m_module_sp) {
-        Stream &s = target.GetDebugger().GetOutputStream();
+        Stream &s = target.GetDebugger().GetErrorStream();
         s.Printf("WARNING: Unable to locate kernel binary on the debugger "
                  "system.\n");
+        if (kernel_search_error.Fail() && kernel_search_error.AsCString("") &&
+            kernel_search_error.AsCString("")[0] != '\0') {
+          s << kernel_search_error.AsCString();
+        }
       }
     }
 

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

@@ -5414,6 +5414,8 @@ uint32_t ObjectFileMachO::GetNumThreadContexts() {
 
 std::string ObjectFileMachO::GetIdentifierString() {
   std::string result;
+  Log *log(
+      GetLog(LLDBLog::Symbols | LLDBLog::Process | LLDBLog::DynamicLoader));
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
@@ -5449,6 +5451,8 @@ std::string ObjectFileMachO::GetIdentifierString() {
                 result = buf;
                 if (buf)
                   free(buf);
+                LLDB_LOGF(log, "LC_NOTE 'kern ver str' found with text '%s'",
+                          result.c_str());
                 return result;
               }
             }
@@ -5472,6 +5476,7 @@ std::string ObjectFileMachO::GetIdentifierString() {
                                               buf) == ident_command.cmdsize) {
           buf[ident_command.cmdsize - 1] = '\0';
           result = buf;
+          LLDB_LOGF(log, "LC_IDENT found with text '%s'", result.c_str());
         }
         if (buf)
           free(buf);
@@ -5484,6 +5489,7 @@ std::string ObjectFileMachO::GetIdentifierString() {
 
 addr_t ObjectFileMachO::GetAddressMask() {
   addr_t mask = 0;
+  Log *log(GetLog(LLDBLog::Process));
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
@@ -5511,6 +5517,10 @@ addr_t ObjectFileMachO::GetAddressMask() {
               if (num_addr_bits != 0) {
                 mask = ~((1ULL << num_addr_bits) - 1);
               }
+              LLDB_LOGF(log,
+                        "LC_NOTE 'addrable bits' found, value %d bits, "
+                        "mask 0x%" PRIx64,
+                        num_addr_bits, mask);
               break;
             }
           }
@@ -5526,6 +5536,8 @@ bool ObjectFileMachO::GetCorefileMainBinaryInfo(addr_t &value,
                                                 bool &value_is_offset,
                                                 UUID &uuid,
                                                 ObjectFile::BinaryType &type) {
+  Log *log(
+      GetLog(LLDBLog::Symbols | LLDBLog::Process | LLDBLog::DynamicLoader));
   value = LLDB_INVALID_ADDRESS;
   value_is_offset = false;
   uuid.Clear();
@@ -5605,20 +5617,31 @@ bool ObjectFileMachO::GetCorefileMainBinaryInfo(addr_t &value,
               uuid = UUID(raw_uuid, sizeof(uuid_t));
               // convert the "main bin spec" type into our
               // ObjectFile::BinaryType enum
+              const char *typestr = "unrecognized type";
               switch (binspec_type) {
               case 0:
                 type = eBinaryTypeUnknown;
+                typestr = "uknown";
                 break;
               case 1:
                 type = eBinaryTypeKernel;
+                typestr = "xnu kernel";
                 break;
               case 2:
                 type = eBinaryTypeUser;
+                typestr = "userland dyld";
                 break;
               case 3:
                 type = eBinaryTypeStandalone;
+                typestr = "standalone";
                 break;
               }
+              LLDB_LOGF(log,
+                        "LC_NOTE 'main bin spec' found, version %d type %d "
+                        "(%s), value 0x%" PRIx64 " value-is-slide==%s uuid %s",
+                        version, type, typestr, value,
+                        value_is_offset ? "true" : "false",
+                        uuid.GetAsString().c_str());
               if (!m_data.GetU32(&offset, &log2_pagesize, 1))
                 return false;
               if (version > 1 && !m_data.GetU32(&offset, &platform, 1))
@@ -6811,6 +6834,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
 ObjectFileMachO::MachOCorefileAllImageInfos
 ObjectFileMachO::GetCorefileAllImageInfos() {
   MachOCorefileAllImageInfos image_infos;
+  Log *log(
+      GetLog(LLDBLog::Symbols | LLDBLog::Process | LLDBLog::DynamicLoader));
 
   // Look for an "all image infos" LC_NOTE.
   lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
@@ -6840,6 +6865,9 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
         //  offset += 4; // uint32_t entries_size;
         //  offset += 4; // uint32_t unused;
 
+        LLDB_LOGF(log,
+                  "LC_NOTE 'all image infos' found version %d with %d images",
+                  version, imgcount);
         offset = entries_fileoff;
         for (uint32_t i = 0; i < imgcount; i++) {
           // Read the struct image_entry.
@@ -6871,6 +6899,12 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
                                                     vmaddr};
             image_entry.segment_load_addresses.push_back(new_seg);
           }
+          LLDB_LOGF(
+              log, "  image entry: %s %s 0x%" PRIx64 " %s",
+              image_entry.filename.c_str(),
+              image_entry.uuid.GetAsString().c_str(), image_entry.load_address,
+              image_entry.currently_executing ? "currently executing"
+                                              : "not currently executing");
           image_infos.all_image_infos.push_back(image_entry);
         }
       } else if (strcmp("load binary", data_owner) == 0) {
@@ -6890,6 +6924,14 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
           image_entry.slide = slide;
           image_entry.currently_executing = true;
           image_infos.all_image_infos.push_back(image_entry);
+          LLDB_LOGF(log,
+                    "LC_NOTE 'load binary' found, filename %s uuid %s load "
+                    "address 0x%" PRIx64 " slide 0x%" PRIx64,
+                    filename.c_str(),
+                    image_entry.uuid.IsValid()
+                        ? image_entry.uuid.GetAsString().c_str()
+                        : "00000000-0000-0000-0000-000000000000",
+                    load_address, slide);
         }
       }
     }

lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

@@ -330,7 +330,8 @@ FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
 
 static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
                                                 ModuleSpec &module_spec,
-                                                Status &error) {
+                                                Status &error,
+                                                const std::string &command) {
   Log *log = GetLog(LLDBLog::Host);
   bool success = false;
   if (uuid_dict != NULL && CFGetTypeID(uuid_dict) == CFDictionaryGetTypeID()) {
@@ -342,7 +343,10 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
                                                CFSTR("DBGError"));
     if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
       if (CFCString::FileSystemRepresentation(cf_str, str)) {
-        error.SetErrorString(str);
+        std::string errorstr = command;
+        errorstr += ":\n";
+        errorstr += str;
+        error.SetErrorString(errorstr);
       }
     }
 
@@ -652,7 +656,8 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
     CFCString uuid_cfstr(uuid_str.c_str());
     CFDictionaryRef uuid_dict =
         (CFDictionaryRef)CFDictionaryGetValue(plist.get(), uuid_cfstr.get());
-    return GetModuleSpecInfoFromUUIDDictionary(uuid_dict, module_spec, error);
+    return GetModuleSpecInfoFromUUIDDictionary(uuid_dict, module_spec, error,
+                                               command.GetData());
   }
 
   if (const CFIndex num_values = ::CFDictionaryGetCount(plist.get())) {
@@ -661,13 +666,14 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
     ::CFDictionaryGetKeysAndValues(plist.get(), NULL,
                                    (const void **)&values[0]);
     if (num_values == 1) {
-      return GetModuleSpecInfoFromUUIDDictionary(values[0], module_spec, error);
+      return GetModuleSpecInfoFromUUIDDictionary(values[0], module_spec, error,
+                                                 command.GetData());
     }
 
     for (CFIndex i = 0; i < num_values; ++i) {
       ModuleSpec curr_module_spec;
       if (GetModuleSpecInfoFromUUIDDictionary(values[i], curr_module_spec,
-                                              error)) {
+                                              error, command.GetData())) {
         if (module_spec.GetArchitecture().IsCompatibleMatch(
                 curr_module_spec.GetArchitecture())) {
           module_spec = curr_module_spec;