[Lldb-commits] [PATCH] D89155: [lldb] Minidump: check for .text hash match with directory

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:533
+      }
       if (module_sp) {
         // We consider the module to be a match if the minidump UUID is a
----------------
This whole "if" statement will need to be tried with both files. We might end up finding a file in the sysroot that is a valid file, but does not actually match the .text hash. Then the search by basename, which will search using the "target.exec-search-paths" or "target.debug-file-search-paths" settings might find another file that you might have downloaded, and it will need to go through all of the .text checks as well.

So as we now have more than 1 file to check, this "if (module_sp)" and its contents should be made into a function. Look at the code above I would create a method of ProcessMinidump that does all this:

```
ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid, ModuleSpec module_spec) {
  ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
  if (!module_sp)
    return module_sp;
  // We consider the module to be a match if the minidump UUID is a
  // prefix of the actual UUID, or if either of the UUIDs are empty.
  const auto dmp_bytes = minidump_uuid.GetBytes();
  const auto mod_bytes = module_sp->GetUUID().GetBytes();
  const bool match = dmp_bytes.empty() || mod_bytes.empty() ||
  mod_bytes.take_front(dmp_bytes.size()) == dmp_bytes;
  if (match) {
    LLDB_LOG(log, "Partial uuid match for {0}.", name);
    return module_sp;
  }

  // Breakpad generates minindump files, and if there is no GNU build
  // ID in the binary, it will calculate a UUID by hashing first 4096
  // bytes of the .text section and using that as the UUID for a module
  // in the minidump. Facebook uses a modified breakpad client that
  // uses a slightly modified this hash to avoid collisions. Check for
  // UUIDs from the minindump that match these cases and accept the
  // module we find if they do match.
  std::vector<uint8_t> breakpad_uuid;
  std::vector<uint8_t> facebook_uuid;
  HashElfTextSection(module_sp, breakpad_uuid, facebook_uuid);
  if (dmp_bytes == llvm::ArrayRef<uint8_t>(breakpad_uuid)) {
    LLDB_LOG(log, "Breakpad .text hash match for {0}.", name);
    return module_sp;
  }
  if (dmp_bytes == llvm::ArrayRef<uint8_t>(facebook_uuid)) {
    LLDB_LOG(log, "Facebook .text hash match for {0}.", name);
    return module_sp;
  }
  // The UUID wasn't a partial match and didn't match the .text hash
  // so remove the module from the target, we will need to create a
  // placeholder object file.
  GetTarget().GetImages().Remove(module_sp);
  module_sp.reset();
  return module_sp;
}
```

Then call this function once with basename_module_spec with a full path, and if no matching module is returned, call the remove the directory and call it again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89155