Add hashing of the .text section to ProcessMinidump.

[clayborg]

Summary

Breakpad will always have a UUID for binaries when it creates minidump files. If an ELF files has a GNU build ID, it will use that. If it doesn't, it will create one by hashing up to the first 4096 bytes of the .text section. LLDB was not able to load these binaries even when we had the right binary because the UUID didn't match. LLDB will use the GNU build ID first as the main UUID for a binary and fallback onto a 8 byte CRC if a binary doesn't have one. With this fix, we will check for the Breakpad hash or the Facebook hash (a modified version of the breakpad hash that collides a bit less) and accept binaries when these hashes match.

[jhenderson]

I'm not an LLDB developer, so probably not the right person to review this, sorry.

[wallace]

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

563–564

I don't understand well the first sentence

[labath]

This has been a feature we've been missing for a while. Thanks for implementing it.

Just two quick requests on the implementation.

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

171–174

Running off of the end of data buffer in this way seems dangerous. I get that we actually want to replicate running off of the end of the section, but we should do it in a way that won't light up on any sanitizer. It seems like it should be possible to fetch the desired data via something like this:

size_t size_to_read = std::min(llvm::alignTo(sect_sp->GetFileSize(), kMDGUIDSize), kBreakpadPageSize) /*or something similar*/;
sect_sp->GetObjectFile()->GetData(sect_sp->GetFileOffset(), size_to_read, data);

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml

16

I guess this should include a custom Fill pseudo-section so that we can guarantee the contents of whatever comes after it. Otherwise, yaml2obj might decide to place anything (or nothing) there. Something like this ought to do it:

- Type: Fill
  Pattern: "DEADBEEF"
  Size: 0xsomething

[labath]

Also, +@markmentovai, in case he has any thoughts on this.

[clayborg]

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

174

I will try and make sure the data is there. The main issue is I don't think you can ask for more bytes than a section has, I believe it will cap the data. But I will check into this.

563–564

I will try to make this clearer and rephrase a bit.

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml

16

We don't need to because I selected a multiple of 16 for the contents of the text section! If I added one more byte, then we would need to.

[clayborg]

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

174

Never mind, you are reading from the object file's data directly (not the section contents) so this should work just fine.

[clayborg]

Fixed:

• Use a safer method to read the data we need for the .text section in case we need and extra 15 bytes.
• Rephrase confusing comment

[clayborg]

Hopefully this should be good to go, let me know if anyone has any issues.

[labath]

In D86261#2229341, @clayborg wrote:

Hopefully this should be good to go, let me know if anyone has any issues.

Looks good, just please also add a test case which runs off the end of the text section. If you have problems generating this test with yaml2obj, I would also be fine with a checked-in binary for a special case like this.

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml

16

I see. In that case, it would be good to have an test case which actually exercises the overflow path. :)

[clayborg]

Added a test case with a 1 byte .text section that will overflow into the 15 bytes of the .data section that follows. This will test the overflow case that was requested.

[labath]

thanks

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

public:
 
   lldb::addr_t GetBaseImageAddress() const { return m_base; }
 private:
   ArchSpec m_arch;
   UUID m_uuid;
   lldb::addr_t m_base;
   lldb::addr_t m_size;
 };
+
+/// Duplicate the HashElfTextSection() from the breakpad sources.
+///
+/// Breakpad, a Google crash log reporting tool suite, creates minidump files
+/// for many different architectures. When using Breakpad to create ELF
+/// minidumps, it will check for a GNU build ID when creating a minidump file
+/// and if one doesn't exist in the file, it will say the UUID of the file is a
+/// checksum of up to the first 4096 bytes of the .text section. Facebook also
+/// uses breakpad and modified this hash to avoid collisions so we can
+/// calculate and check for this as well.
+///
+/// The breakpad code might end up hashing up to 15 bytes that immediately
+/// follow the .text section in the file, so this code must do exactly what it
+/// does so we can get an exact match for the UUID.
+///
+/// \param[in] module_sp The module to grab the .text section from.
+///
+/// \param[in/out] breakpad_uuid A vector that will receive the calculated
+///                breakpad .text hash.
+///
+/// \param[in/out] facebook_uuid A vector that will receive the calculated
+///                facebook .text hash.
+///
+void HashElfTextSection(ModuleSP module_sp, std::vector<uint8_t> &breakpad_uuid,
+                        std::vector<uint8_t> &facebook_uuid) {
+  SectionList *sect_list = module_sp->GetSectionList();
+  if (sect_list == nullptr)
+    return;
+  SectionSP sect_sp = sect_list->FindSectionByName(ConstString(".text"));
+  if (!sect_sp)
+    return;
+  constexpr size_t kMDGUIDSize = 16;
+  constexpr size_t kBreakpadPageSize = 4096;
+  // The breakpad code has a bug where it might access beyond the end of a
+  // .text section by up to 15 bytes, so we must ensure we round up to the
+  // next kMDGUIDSize byte boundary.
+  DataExtractor data;
+  const size_t text_size = sect_sp->GetFileSize();
+  const size_t read_size = std::min<size_t>(
+      llvm::alignTo(text_size, kMDGUIDSize), kBreakpadPageSize);
+  sect_sp->GetObjectFile()->GetData(sect_sp->GetFileOffset(), read_size, data);
+
+  breakpad_uuid.assign(kMDGUIDSize, 0);
+  facebook_uuid.assign(kMDGUIDSize, 0);
+
+  // The only difference between the breakpad hash and the facebook hash is the
+  // hashing of the text section size into the hash prior to hashing the .text
+  // contents.
+  for (size_t i = 0; i < kMDGUIDSize; i++)
+    facebook_uuid[i] ^= text_size % 255;
+
+  // This code carefully duplicates how the hash was created in Breakpad
+  // sources, including the error where it might has an extra 15 bytes past the
+  // end of the .text section if the .text section is less than a page size in
+  // length.
+  const uint8_t *ptr = data.GetDataStart();
+  const uint8_t *ptr_end = data.GetDataEnd();
+  while (ptr < ptr_end) {
+    for (unsigned i = 0; i < kMDGUIDSize; i++) {
+      breakpad_uuid[i] ^= ptr[i];
+      facebook_uuid[i] ^= ptr[i];
+    }
+    ptr += kMDGUIDSize;
+  }
+}
+
 } // namespace
 
 ConstString ProcessMinidump::GetPluginNameStatic() {
   static ConstString g_name("minidump");
   return g_name;
 }
 
 const char *ProcessMinidump::GetPluginDescriptionStatic() {
if (!module_sp) {
       if (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 = 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) {
+          // 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);
+          } else if (dmp_bytes == llvm::ArrayRef<uint8_t>(facebook_uuid)) {
+            LLDB_LOG(log, "Facebook .text hash match for {0}.", name);
+          } else {
+            // 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();
           }
+        } else {
+          LLDB_LOG(log, "Partial uuid match for {0}.", name);
         }
       }
+    } else {
+      LLDB_LOG(log, "Full uuid match for {0}.", name);
+    }
     if (module_sp) {
       // Watch out for place holder modules that have different paths, but the
       // same UUID. If the base address is different, create a new module. If
       // we don't then we will end up setting the load address of a different
       // PlaceholderObjectFile and an assertion will fire.
       auto *objfile = module_sp->GetObjectFile();
       if (objfile && objfile->GetPluginName() ==
           PlaceholderObjectFile::GetStaticPluginName()) {

lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

def test_partial_uuid_mismatch(self):
         cmd = 'settings set target.exec-search-paths "%s"' % (os.path.dirname(so_path))
         self.dbg.HandleCommand(cmd)
         modules = self.get_minidump_modules("linux-arm-partial-uuids-mismatch.yaml")
         self.assertEqual(1, len(modules))
         self.verify_module(modules[0],
                            "/invalid/path/on/current/system/libuuidmismatch.so",
                            "7295E17C-6668-9E05-CBB5-DEE5003865D5")
 
+    def test_breakpad_hash_match(self):
+        """
+            Breakpad creates minidump files using CvRecord in each module whose
+            signature is set to PDB70 where the UUID is a hash generated by
+            breakpad of the .text section. This is only done when the
+            executable has no ELF build ID.
+
+            This test verifies that if we have a minidump with a 16 byte UUID,
+            that we are able to associate a symbol file with no ELF build ID
+            and match it up by hashing the .text section.
+        """
+        so_path = self.getBuildArtifact("libbreakpad.so")
+        self.yaml2obj("libbreakpad.yaml", so_path)
+        cmd = 'settings set target.exec-search-paths "%s"' % (os.path.dirname(so_path))
+        self.dbg.HandleCommand(cmd)
+        modules = self.get_minidump_modules("linux-arm-breakpad-uuid-match.yaml")
+        self.assertEqual(1, len(modules))
+        # LLDB makes up it own UUID as well when there is no build ID so we
+        # will check that this matches.
+        self.verify_module(modules[0], so_path, "D9C480E8")
+
+    def test_breakpad_overflow_hash_match(self):
+        """
+            This is a similar to test_breakpad_hash_match, but it verifies that
+            if the .text section does not end on a 16 byte boundary, then it
+            will overflow into the next section's data by up to 15 bytes. This
+            verifies that we are able to match what breakpad does as it will do
+            this.
+        """
+        so_path = self.getBuildArtifact("libbreakpad.so")
+        self.yaml2obj("libbreakpad-overflow.yaml", so_path)
+        cmd = 'settings set target.exec-search-paths "%s"' % (os.path.dirname(so_path))
+        self.dbg.HandleCommand(cmd)
+        modules = self.get_minidump_modules("linux-arm-breakpad-uuid-match.yaml")
+        self.assertEqual(1, len(modules))
+        # LLDB makes up it own UUID as well when there is no build ID so we
+        # will check that this matches.
+        self.verify_module(modules[0], so_path, "48EB9FD7")
+
+
+    def test_facebook_hash_match(self):
+        """
+            Breakpad creates minidump files using CvRecord in each module whose
+            signature is set to PDB70 where the UUID is a hash generated by
+            breakpad of the .text section and Facebook modified this hash to
+            avoid collisions. This is only done when the executable has no ELF
+            build ID.
+
+            This test verifies that if we have a minidump with a 16 byte UUID,
+            that we are able to associate a symbol file with no ELF build ID
+            and match it up by hashing the .text section like Facebook does.
+        """
+        so_path = self.getBuildArtifact("libbreakpad.so")
+        self.yaml2obj("libbreakpad.yaml", so_path)
+        cmd = 'settings set target.exec-search-paths "%s"' % (os.path.dirname(so_path))
+        self.dbg.HandleCommand(cmd)
+        modules = self.get_minidump_modules("linux-arm-facebook-uuid-match.yaml")
+        self.assertEqual(1, len(modules))
+        # LLDB makes up it own UUID as well when there is no build ID so we
+        # will check that this matches.
+        self.verify_module(modules[0], so_path, "D9C480E8")
+
+
     def test_relative_module_name(self):
         old_cwd = os.getcwd()
         self.addTearDownHook(lambda: os.chdir(old_cwd))
         os.chdir(self.getBuildDir())
         name = "file-with-a-name-unlikely-to-exist-in-the-current-directory.so"
         open(name, "a").close()
         modules = self.get_minidump_modules(
                 self.getSourcePath("relative_module_name.yaml"))

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-overflow.yaml

+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000010000
+    AddressAlign:    0x0000000000000001
+    Content:         04
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_WRITE ]
+    Address:         0x0000000000010001
+    AddressAlign:    0x0000000000000001
+    Content:         0000001400000003000000474E5500

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml

+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000010000
+    AddressAlign:    0x0000000000000004
+    Content:         040000001400000003000000474E5500

lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-breakpad-uuid-match.yaml

+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  ARM
+    Platform ID:     Linux
+    CSD Version:     '15E216'
+    CPU:
+      CPUID:           0x00000000
+  - Type:            ModuleList
+    Modules:
+      - Base of Image:   0x0000000000001000
+        Size of Image:   0x00001000
+        Module Name:     '/invalid/path/on/current/system/libbreakpad.so'
+        CodeView Record: 52534453040000001400000003000000474e55000000000000
+...

lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-facebook-uuid-match.yaml

+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  ARM
+    Platform ID:     Linux
+    CSD Version:     '15E216'
+    CPU:
+      CPUID:           0x00000000
+  - Type:            ModuleList
+    Modules:
+      - Base of Image:   0x0000000000001000
+        Size of Image:   0x00001000
+        Module Name:     '/invalid/path/on/current/system/libbreakpad.so'
+        CodeView Record: 52534453141010100410101013101010575e45100000000000
+...