[Lldb-commits] [PATCH] D86261: Add hashing of the .text section to ProcessMinidump.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:174
+  // end of the .text section if the .text section is less than a page size in
+  // length.
+  const uint8_t *ptr = text_data.GetDataStart();
----------------
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.


================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:559-560
         if (!match) {
+          // Breakpad might put a hash of the up to the first page if .text
+          // bytes into the UUID of a minidump. Facebook also has slightly
+          // modified this hash to avoid collisions. Check for UUIDs from the
----------------
wallace wrote:
> I don't understand well the first sentence
I will try to make this clearer and rephrase a bit.


================
Comment at: lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml:15
+    AddressAlign:    0x0000000000000004
+    Content:         040000001400000003000000474E5500
----------------
labath wrote:
> 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
> ```
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86261