jasonmolenda updated this revision to Diff 548015.
jasonmolenda added a comment.

Update patch to send these error messages to the ErrorStream not OutputStream.  
Also change LocateSymbolFileMacOSX.cpp so the full command that was used to 
find the binary is included in the error message, to make it clearer exactly 
where the error message originated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157160

Files:
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===================================================================
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -330,7 +330,8 @@
 
 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 @@
                                                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 @@
     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 @@
     ::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;
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5414,6 +5414,8 @@
 
 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 @@
                 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 @@
                                               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 @@
 
 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 @@
               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 &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 @@
               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 @@
 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 @@
         //  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 @@
                                                     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 @@
           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);
         }
       }
     }
Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -767,9 +767,10 @@
       // 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 @@
       }
 
       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();
+        }
       }
     }
 
Index: lldb/source/Core/DynamicLoader.cpp
===================================================================
--- lldb/source/Core/DynamicLoader.cpp
+++ 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,10 @@
                                            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') {
+        Stream &s = target.GetDebugger().GetErrorStream();
+        s << error.AsCString();
       }
     }
 
@@ -267,8 +272,8 @@
         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 {
@@ -276,8 +281,8 @@
           // 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);
         }
@@ -285,8 +290,8 @@
         // 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 +303,26 @@
       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-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to