labath marked 5 inline comments as done.
labath added a comment.

After some internal discussion, it seems that the situation with the all-zero 
UUIDs is as follows:

- breakpad symbol files do not attach a special meaning to a zero UUID - if a 
file does not have a build-id, the dump_syms tool will use a hash of the first 
page of the text section (or something equally silly)
- minidump files may treat the missing build-id by replacing it with zeroes 
depending on the tool used to produce the minidump: breakpad doesn't do that 
(it does the same hash as above), crashpad does.

So it seems like there is nothing to do here. Maybe the UUID reading code in 
ProcessMinidump needs revising though.



================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:69
+  llvm::StringRef chunk = str.take_front(hex_digits<T>());
+  uintmax_t t;
+  if (!to_integer(chunk, t, 16))
----------------
lemo wrote:
> = 0; ?
That is not necessary, as to_integer initializes it. Perhaps more importantly, 
not initializing this allows tools like msan and valgrind to actually detect 
the cases when you end up using an uninitialized value.


================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:48
 
 class ModuleRecord : public Record {
 public:
----------------
lemo wrote:
> coding-convention-wise: should these definitions use struct instead of class?
I don't think we have a strict rule about this case, but personally, when 
something starts using inheritance, I tend to think of it as a class.


================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:59
 
-bool operator==(const ModuleRecord &L, const ModuleRecord &R);
+bool operator==(const ModuleRecord &L, const ModuleRecord &R) {
+  return L.OS == R.OS && L.Arch == R.Arch && L.ID == R.ID;
----------------
lemo wrote:
> const method qualifier?
This is a free function, not a method. :)


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

https://reviews.llvm.org/D57037



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

Reply via email to