>
> 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.
>
I agree. Sorry for mixing the minidump UUID case with the Breakpad symbol
files UUIDs.
On Wed, Jan 23, 2019 at 1:23 PM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> 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
