This revision was automatically updated to reflect the committed changes.
Closed by commit rL356573: Fix UUID decoding from minidump files (authored by 
gclayton, committed by ).
Herald added subscribers: llvm-commits, jdoerfert.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D59433?vs=190901&id=191516#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59433

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-16.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-20.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-no-age.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-with-age.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-zero-uuids.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/macos-arm-uuids-no-age.dmp
  lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp

Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -165,21 +165,44 @@
       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);
 
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -0,0 +1,121 @@
+"""
+Test basics of Minidump debugging.
+"""
+
+from __future__ import print_function
+from six import iteritems
+
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MiniDumpUUIDTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def setUp(self):
+        super(MiniDumpUUIDTestCase, self).setUp()
+        self._initial_platform = lldb.DBG.GetSelectedPlatform()
+
+    def tearDown(self):
+        lldb.DBG.SetSelectedPlatform(self._initial_platform)
+        super(MiniDumpUUIDTestCase, self).tearDown()
+
+    def verify_module(self, module, verify_path, verify_uuid):
+        uuid = module.GetUUIDString()
+        self.assertEqual(verify_path, module.GetFileSpec().fullpath)
+        self.assertEqual(verify_uuid, uuid)
+
+    def test_zero_uuid_modules(self):
+        """
+            Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
+            but contains a PDB70 value whose age is zero and whose UUID values are 
+            all zero. Prior to a fix all such modules would be duplicated to the
+            first one since the UUIDs claimed to be valid and all zeroes. Now we
+            ensure that the UUID is not valid for each module and that we have
+            each of the modules in the target after loading the core
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-zero-uuids.dmp")
+        modules = self.target.modules
+        self.assertEqual(2, len(modules))
+        self.verify_module(modules[0], "/file/does/not/exist/a", None)
+        self.verify_module(modules[1], "/file/does/not/exist/b", None)
+
+    def test_uuid_modules_no_age(self):
+        """
+            Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
+            and contains a PDB70 value whose age is zero and whose UUID values are 
+            valid. Ensure we decode the UUID and don't include the age field in the UUID.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-uuids-no-age.dmp")
+        modules = self.target.modules
+        self.assertEqual(2, len(modules))
+        self.verify_module(modules[0], "/tmp/a", "01020304-0506-0708-090A-0B0C0D0E0F10")
+        self.verify_module(modules[1], "/tmp/b", "0A141E28-323C-4650-5A64-6E78828C96A0")
+
+    def test_uuid_modules_no_age_apple(self):
+        """
+            Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
+            and contains a PDB70 value whose age is zero and whose UUID values are 
+            valid. Ensure we decode the UUID and don't include the age field in the UUID.
+            Also ensure that the first uint32_t is byte swapped, along with the next
+            two uint16_t values. Breakpad incorrectly byte swaps these values when it
+            saves Darwin minidump files.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("macos-arm-uuids-no-age.dmp")
+        modules = self.target.modules
+        self.assertEqual(2, len(modules))
+        self.verify_module(modules[0], "/tmp/a", "04030201-0605-0807-090A-0B0C0D0E0F10")
+        self.verify_module(modules[1], "/tmp/b", "281E140A-3C32-5046-5A64-6E78828C96A0")
+
+    def test_uuid_modules_with_age(self):
+        """
+            Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
+            and contains a PDB70 value whose age is valid and whose UUID values are 
+            valid. Ensure we decode the UUID and include the age field in the UUID.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-uuids-with-age.dmp")
+        modules = self.target.modules
+        self.assertEqual(2, len(modules))
+        self.verify_module(modules[0], "/tmp/a", "01020304-0506-0708-090A-0B0C0D0E0F10-10101010")
+        self.verify_module(modules[1], "/tmp/b", "0A141E28-323C-4650-5A64-6E78828C96A0-20202020")
+
+    def test_uuid_modules_elf_build_id_16(self):
+        """
+            Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
+            and contains a ELF build ID whose value is valid and is 16 bytes long.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-uuids-elf-build-id-16.dmp")
+        modules = self.target.modules
+        self.assertEqual(2, len(modules))
+        self.verify_module(modules[0], "/tmp/a", "01020304-0506-0708-090A-0B0C0D0E0F10")
+        self.verify_module(modules[1], "/tmp/b", "0A141E28-323C-4650-5A64-6E78828C96A0")
+
+    def test_uuid_modules_elf_build_id_20(self):
+        """
+            Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
+            and contains a ELF build ID whose value is valid and is 20 bytes long.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-uuids-elf-build-id-20.dmp")
+        modules = self.target.modules
+        self.assertEqual(2, len(modules))
+        self.verify_module(modules[0], "/tmp/a", "01020304-0506-0708-090A-0B0C0D0E0F10-11121314")
+        self.verify_module(modules[1], "/tmp/b", "0A141E28-323C-4650-5A64-6E78828C96A0-AAB4BEC8")
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to