zequanwu updated this revision to Diff 301694.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90325

Files:
  lldb/include/lldb/Utility/UUID.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Plugins/Process/minidump/MinidumpTypes.h
  lldb/source/Utility/UUID.cpp

Index: lldb/source/Utility/UUID.cpp
===================================================================
--- lldb/source/Utility/UUID.cpp
+++ lldb/source/Utility/UUID.cpp
@@ -35,6 +35,16 @@
   }
 }
 
+UUID UUID::fromCvRecord(UUID::CvRecordPdb70 &debug_info) {
+  llvm::sys::swapByteOrder(debug_info.Uuid.Data1);
+  llvm::sys::swapByteOrder(debug_info.Uuid.Data2);
+  llvm::sys::swapByteOrder(debug_info.Uuid.Data3);
+  llvm::sys::swapByteOrder(debug_info.Age);
+  if (debug_info.Age)
+    return UUID::fromOptionalData(&debug_info, sizeof(debug_info));
+  return UUID::fromOptionalData(&debug_info.Uuid, sizeof(debug_info.Uuid));
+}
+
 std::string UUID::GetAsString(llvm::StringRef separator) const {
   std::string result;
   llvm::raw_string_ostream os(result);
Index: lldb/source/Plugins/Process/minidump/MinidumpTypes.h
===================================================================
--- lldb/source/Plugins/Process/minidump/MinidumpTypes.h
+++ lldb/source/Plugins/Process/minidump/MinidumpTypes.h
@@ -40,21 +40,6 @@
   ElfBuildId = 0x4270454c, // BpEL (Breakpad/Crashpad minidumps)
 };
 
-// Reference:
-// https://crashpad.chromium.org/doxygen/structcrashpad_1_1CodeViewRecordPDB70.html
-struct CvRecordPdb70 {
-  struct {
-    llvm::support::ulittle32_t Data1;
-    llvm::support::ulittle16_t Data2;
-    llvm::support::ulittle16_t Data3;
-    uint8_t Data4[8];
-  } Uuid;
-  llvm::support::ulittle32_t Age;
-  // char PDBFileName[];
-};
-static_assert(sizeof(CvRecordPdb70) == 20,
-              "sizeof CvRecordPdb70 is not correct!");
-
 enum class MinidumpMiscInfoFlags : uint32_t {
   ProcessID = (1 << 0),
   ProcessTimes = (1 << 1),
Index: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -62,27 +62,18 @@
       static_cast<CvSignature>(static_cast<uint32_t>(*signature));
 
   if (cv_signature == CvSignature::Pdb70) {
-    const CvRecordPdb70 *pdb70_uuid = nullptr;
+    const UUID::CvRecordPdb70 *pdb70_uuid = nullptr;
     Status error = consumeObject(cv_record, pdb70_uuid);
     if (error.Fail())
       return UUID();
-
-    CvRecordPdb70 swapped;
-    if (!GetArchitecture().GetTriple().isOSBinFormatELF()) {
-      // LLDB's UUID class treats the data as a sequence of bytes, but breakpad
-      // interprets it as a sequence of little-endian fields, which it converts
-      // to big-endian when converting to text. Swap the bytes to big endian so
-      // that the string representation comes out right.
-      swapped = *pdb70_uuid;
-      llvm::sys::swapByteOrder(swapped.Uuid.Data1);
-      llvm::sys::swapByteOrder(swapped.Uuid.Data2);
-      llvm::sys::swapByteOrder(swapped.Uuid.Data3);
-      llvm::sys::swapByteOrder(swapped.Age);
-      pdb70_uuid = &swapped;
+    if (GetArchitecture().GetTriple().isOSBinFormatELF()) {
+      if (pdb70_uuid->Age != 0)
+        return UUID::fromOptionalData(pdb70_uuid, sizeof(*pdb70_uuid));
+      return UUID::fromOptionalData(&pdb70_uuid->Uuid,
+                                    sizeof(pdb70_uuid->Uuid));
     }
-    if (pdb70_uuid->Age != 0)
-      return UUID::fromOptionalData(pdb70_uuid, sizeof(*pdb70_uuid));
-    return UUID::fromOptionalData(&pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
+    UUID::CvRecordPdb70 debug_info = *pdb70_uuid;
+    return UUID::fromCvRecord(debug_info);
   } else if (cv_signature == CvSignature::ElfBuildId)
     return UUID::fromOptionalData(cv_record);
 
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -43,45 +43,16 @@
 
 LLDB_PLUGIN_DEFINE(ObjectFilePECOFF)
 
-struct CVInfoPdb70 {
-  // 16-byte GUID
-  struct _Guid {
-    llvm::support::ulittle32_t Data1;
-    llvm::support::ulittle16_t Data2;
-    llvm::support::ulittle16_t Data3;
-    uint8_t Data4[8];
-  } Guid;
-
-  llvm::support::ulittle32_t Age;
-};
-
 static UUID GetCoffUUID(llvm::object::COFFObjectFile &coff_obj) {
   const llvm::codeview::DebugInfo *pdb_info = nullptr;
   llvm::StringRef pdb_file;
 
-  // This part is similar with what has done in minidump parser.
   if (!coff_obj.getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) {
     if (pdb_info->PDB70.CVSignature == llvm::OMF::Signature::PDB70) {
-      using llvm::support::endian::read16be;
-      using llvm::support::endian::read32be;
-
-      const uint8_t *sig = pdb_info->PDB70.Signature;
-      struct CVInfoPdb70 info;
-      info.Guid.Data1 = read32be(sig);
-      sig += 4;
-      info.Guid.Data2 = read16be(sig);
-      sig += 2;
-      info.Guid.Data3 = read16be(sig);
-      sig += 2;
-      memcpy(info.Guid.Data4, sig, 8);
-
-      // Return 20-byte UUID if the Age is not zero
-      if (pdb_info->PDB70.Age) {
-        info.Age = read32be(&pdb_info->PDB70.Age);
-        return UUID::fromOptionalData(&info, sizeof(info));
-      }
-      // Otherwise return 16-byte GUID
-      return UUID::fromOptionalData(&info.Guid, sizeof(info.Guid));
+      UUID::CvRecordPdb70 info;
+      memcpy(&info.Uuid, pdb_info->PDB70.Signature, sizeof(info.Uuid));
+      info.Age = pdb_info->PDB70.Age;
+      return UUID::fromCvRecord(info);
     }
   }
 
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -27,43 +27,11 @@
 
 LLDB_PLUGIN_DEFINE(ObjectFilePDB)
 
-struct CVInfoPdb70 {
-  // 16-byte GUID
-  struct _Guid {
-    llvm::support::ulittle32_t Data1;
-    llvm::support::ulittle16_t Data2;
-    llvm::support::ulittle16_t Data3;
-    uint8_t Data4[8];
-  } Guid;
-
-  llvm::support::ulittle32_t Age;
-};
-
 static UUID GetPDBUUID(InfoStream &IS) {
-  // This part is similar with what has done in ObjectFilePECOFF.
-  using llvm::support::endian::read16be;
-  using llvm::support::endian::read32;
-  using llvm::support::endian::read32be;
-
-  GUID guid = IS.getGuid();
-  const uint8_t *guid_p = guid.Guid;
-  struct CVInfoPdb70 info;
-  info.Guid.Data1 = read32be(guid_p);
-  guid_p += 4;
-  info.Guid.Data2 = read16be(guid_p);
-  guid_p += 2;
-  info.Guid.Data3 = read16be(guid_p);
-  guid_p += 2;
-  memcpy(info.Guid.Data4, guid_p, 8);
-
-  // Return 20-byte UUID if the Age is not zero
-  uint32_t age = IS.getAge();
-  if (age) {
-    info.Age = read32(&age, llvm::support::big);
-    return UUID::fromOptionalData(&info, sizeof(info));
-  }
-  // Otherwise return 16-byte GUID
-  return UUID::fromOptionalData(&info.Guid, sizeof(info.Guid));
+  UUID::CvRecordPdb70 debug_info;
+  memcpy(&debug_info.Uuid, IS.getGuid().Guid, sizeof(debug_info.Uuid));
+  debug_info.Age = IS.getAge();
+  return UUID::fromCvRecord(debug_info);
 }
 
 char ObjectFilePDB::ID;
Index: lldb/include/lldb/Utility/UUID.h
===================================================================
--- lldb/include/lldb/Utility/UUID.h
+++ lldb/include/lldb/Utility/UUID.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Endian.h"
 #include <stddef.h>
 #include <stdint.h>
 #include <string>
@@ -23,6 +24,22 @@
 public:
   UUID() = default;
 
+  // Reference:
+  // https://crashpad.chromium.org/doxygen/structcrashpad_1_1CodeViewRecordPDB70.html
+  struct CvRecordPdb70 {
+    struct {
+      llvm::support::ulittle32_t Data1;
+      llvm::support::ulittle16_t Data2;
+      llvm::support::ulittle16_t Data3;
+      uint8_t Data4[8];
+    } Uuid;
+    llvm::support::ulittle32_t Age;
+    // char PDBFileName[];
+  };
+
+  /// Create a UUID from CvRecordPdb70.
+  static UUID fromCvRecord(CvRecordPdb70 &debug_info);
+
   /// Creates a UUID from the data pointed to by the bytes argument. No special
   /// significance is attached to any of the values.
   static UUID fromData(const void *bytes, uint32_t num_bytes) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to