Fix UUID decoding from minidump files

This patch fixes:

UUIDs now don't include the age field from a PDB70 when the age is zero. Prior to this they would incorrectly contain the zero age which stopped us from being able to match up the UUID with real files.
UUIDs for Apple targets get the first 32 bit value and next two 16 bit values swapped. Breakpad incorrectly swaps these values when it creates darwin minidump files, so this must be undone so we can match up symbol files with the minidump modules.
UUIDs that are all zeroes are treated as invalid UUIDs. Breakpad will always save out a UUID, even if one wasn't available. This caused all files that have UUID values of zero to be uniqued to the first module that had a zero UUID. We now don't fill in the UUID if it is all zeroes.
Added tests for PDB70 and ELF build ID based CvRecords.

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

llvm-svn: 356573

1 files changed, 35 insertions, 12 deletions

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 0ec8ddcdcfb..884d7c9b0db 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -165,21 +165,44 @@ UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) {
       static_cast<CvSignature>(static_cast<uint32_t>(*signature));
 
   if (cv_signature == CvSignature::Pdb70) {
-    // PDB70 record
     const CvRecordPdb70 *pdb70_uuid = nullptr;
     Status error = consumeObject(cv_record, pdb70_uuid);
-    if (!error.Fail()) {
-      auto arch = GetArchitecture();
-      // For Apple targets we only need a 16 byte UUID so that we can match
-      // the UUID in the Module to actual UUIDs from the built binaries. The
-      // "Age" field is zero in breakpad minidump files for Apple targets, so
-      // we restrict the UUID to the "Uuid" field so we have a UUID we can use
-      // to match.
-      if (arch.GetTriple().getVendor() == llvm::Triple::Apple)
-        return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
-      else
-        return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+    if (error.Fail())
+      return UUID();
+    // If the age field is not zero, then include the entire pdb70_uuid struct
+    if (pdb70_uuid->Age != 0)
+      return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+
+    // Many times UUIDs are all zeroes. This can cause more than one module
+    // to claim it has a valid UUID of all zeroes and causes the files to all
+    // merge into the first module that claims this valid zero UUID.
+    bool all_zeroes = true;
+    for (size_t i = 0; all_zeroes && i < sizeof(pdb70_uuid->Uuid); ++i)
+      all_zeroes = pdb70_uuid->Uuid[i] == 0;
+    if (all_zeroes)
+      return UUID();
+    
+    if (GetArchitecture().GetTriple().getVendor() == llvm::Triple::Apple) {
+      // Breakpad incorrectly byte swaps the first 32 bit and next 2 16 bit
+      // values in the UUID field. Undo this so we can match things up
+      // with our symbol files
+      uint8_t apple_uuid[16];
+      // Byte swap the first 32 bits
+      apple_uuid[0] = pdb70_uuid->Uuid[3];
+      apple_uuid[1] = pdb70_uuid->Uuid[2];
+      apple_uuid[2] = pdb70_uuid->Uuid[1];
+      apple_uuid[3] = pdb70_uuid->Uuid[0];
+      // Byte swap the next 16 bit value
+      apple_uuid[4] = pdb70_uuid->Uuid[5];
+      apple_uuid[5] = pdb70_uuid->Uuid[4];
+      // Byte swap the next 16 bit value
+      apple_uuid[6] = pdb70_uuid->Uuid[7];
+      apple_uuid[7] = pdb70_uuid->Uuid[6];
+      for (size_t i = 8; i < sizeof(pdb70_uuid->Uuid); ++i)
+        apple_uuid[i] = pdb70_uuid->Uuid[i];
+      return UUID::fromData(apple_uuid, sizeof(apple_uuid));
     }
+    return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
   } else if (cv_signature == CvSignature::ElfBuildId)
     return UUID::fromData(cv_record);