[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: include/lldb/Core/Module.h:779
+  //------------------------------------------------------------------
+  void SetUUID(const lldb_private::UUID &uuid);
+
----------------
amccarth wrote:
> Was there no SetUUID before because the UUID was considered immutable?  I wonder if that's worth keeping.  Is the UUID always known at construction time?
The UUID is not always know at construction time -- sometimes it can be, if you set it on the FileSpec you use to create the module, but you don't have to do that.

However, it is true that the UUID was constant during the lifetime of the Module, and this changes that, which is not ideal IMO.

It seems that in the only place you call this function, you already know the UUID. Would it be possible to do this work in the PlaceholderModule constructor (thereby avoiding any changes to the Module API) ?


================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:118-128
+  } else if (cv_signature == CvSignature::ElfBuildId) {
+    // ELF BuildID (found in Breakpad/Crashpad generated minidumps)
+    //
+    // This is variable-length, but usually 20 bytes
+    // as the binutils ld default is a SHA-1 hash.
+    // (We'll handle only 16 and 20 bytes signatures,
+    // matching LLDB support for UUIDs)
----------------
Looks like the elf part of this is not tested. Do any of our linux minidumps have a build-id we could use?

Since the parser->SB path is already tested by the windows test, this doesn't have to be a full-blown test and a minidump parser unittest (unittests/Process/minidump) should be sufficient.


https://reviews.llvm.org/D46292