clayborg marked 2 inline comments as done.
clayborg added inline comments.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:394
+ if (error.Fail()) {
+ GetTarget().GetImages().Remove(module_sp);
+ module_sp.reset();
----------------
labath wrote:
> Under what circumstances can `GetSharedModule` fail, and still produce a
> valid module_sp ?
Sometimes you can create a module with a specification, but it won't have a
valid object file is my understanding.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:397
+ }
+ if (!module_sp) {
+ // Try and find a module without specifying the UUID and only looking for
----------------
labath wrote:
> I am wondering if there's any way to narrow down the scope of this so we
> don't do this second check for every module out there. If I understood the
> description correctly, it should be sufficient to do this for linux minidumps
> with PDB70 uuid records. Since PDBs aren't a thing on linux, and later
> breakpad versions used used ElfBuildId records, this should be a pretty exact
> match for the situation we want to capture. WDYT?
>
> (Getting the platform of the minidump should be easy, for the UUID record
> type, we might need a small refactor of the MinidumpParser class.)
We really do want this. Why? Because many times with minidumps you have to go
out and find the symbol files in some alternate location, put them in a
directory, then we want lldb to find them automatically. If you have a module
spec with a full path and no UUID, the path will need to match. If you just
specify a basename with no UUID, it allows us to use that file. Also with our
UUID story being a bit weak with ELF 20 bytes build IDs versus 4 bytes GNU
build IDs,. we might need to pull some tricks and strip out a UUID from a
binary so we can load it in LLDB. So this basename only search allows us to use
a symbol file with no UUID or if the minidump didn't contain a UUID and somehow
we knew where the right symbol file was. So this does catch my case, but will
also find any files in the exec search paths.
================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:435
+ }
+ }
+ }
----------------
I can change this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60001/new/
https://reviews.llvm.org/D60001
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
