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

Adrian McCarthy via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Apr 30 15:52:17 PDT 2018

amccarth accepted this revision.
amccarth added a comment.
This revision is now accepted and ready to land.

The big picture here is fine.

I see a couple opportunities in the details, but I won't block on them.

Comment at: include/lldb/Core/Module.h:779
+  //------------------------------------------------------------------
+  void SetUUID(const lldb_private::UUID &uuid);
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?

Comment at: source/Core/Module.cpp:341
+  m_uuid = uuid;
+  m_did_parse_uuid = true;
I wonder if the lazy UUID lookup in Module::GetUUID should call this to avoid code duplication.  I know it's not a lot of code, but it involves a mutex and an atomic flag, so it might be nice to always have this update in just one place.

Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:99
+UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) {
+  auto cv_record =
I wonder if this should return an `Expected<UUID>`.  The function has multiple ways it can fail, in which case the error info is lost and we silently return an empty UUID.


More information about the lldb-commits mailing list