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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This has been a feature we've been missing for a while. Thanks for implementing it.

Just two quick requests on the implementation.



================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:171-174
+  // This code carefully duplicates how the hash was created in Breakpad
+  // sources, including the error where it might has an extra 15 bytes past the
+  // end of the .text section if the .text section is less than a page size in
+  // length.
----------------
Running off of the end of data buffer in this way seems dangerous. I get that we actually want to replicate running off of the end of the section, but we should do it in a way that won't light up on any sanitizer. It seems like it should be possible to fetch the desired data via something like this:
```
size_t size_to_read = std::min(llvm::alignTo(sect_sp->GetFileSize(), kMDGUIDSize), kBreakpadPageSize) /*or something similar*/;
sect_sp->GetObjectFile()->GetData(sect_sp->GetFileOffset(), size_to_read, data);
```


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86261