clayborg added a comment.

Let me know what you think of the UUID code and if you want to include that in 
this patch



================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:67
+/// to the endian-specific integer N. Return true on success.
+template <typename T> static bool consume_integer(llvm::StringRef &str, T &N) {
+  llvm::StringRef chunk = str.take_front(hex_digits<T>());
----------------
rename to "consume_hex_integer" or an extra parameter for the base instead of 
hard coding to 16?


================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:111-112
   // match the native uuid format of these platforms.
-  return UUID::fromData(&data, os == llvm::Triple::Win32 ? 20 : 16);
+  return UUID::fromData(&data, os == llvm::Triple::Win32 ? sizeof(data)
+                                                         : sizeof(data.uuid));
 }
----------------
For Apple platforms Breakpad actually incorrectly byte swaps the first 32 bits 
and the next two 16 bit values. I have a patch for this, but since this is 
moving, it would be great to get that fix in here. Also, many linux breakpad 
file have the UUID field present but set to all zeroes which is bad as UUID 
parsing code will cause multiple modules to claim a UUID of all zeroes is valid 
and causes all modules with such a UUID to just use the first one it finds. The 
code I have in my unsubmitted patch is:

```
      if (pdb70_uuid->Age == 0) {
        bool all_zeroes = true;
        for (size_t i=0; all_zeroes && i<sizeof(pdb70_uuid->Uuid); ++i)
          all_zeroes = pdb70_uuid->Uuid[i] == 0;
        // Many times UUIDs are not filled in at all, so avoid claiming that
        // all such libraries have a valid UUID that is all zeroes.
        if (all_zeroes)
          return UUID();
        if (arch.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));
        } else
          return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
      } else
        return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
```


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

https://reviews.llvm.org/D57037



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to