[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

Leonard Mosescu via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 18 10:31:21 PDT 2018

lemo marked an inline comment as done.
lemo added a comment.

In https://reviews.llvm.org/D45700#1070491, @amccarth wrote:

> LGTM, but consider highlighting the side effect to `target` when the factory makes a Placeholder module.

Good observation: taking a step back, the factory introduces too much coupling, especially if we want to extend this placeholder module approach to other uses. Because of this, I reverted back to the standalone PlaceholderModule::CreateImageSection() approach. Thanks Adrian!

Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:70
+  // Creates a synthetic module section covering the whole module image
+  void CreateImageSection(const MinidumpModule *module, Target& target) {
+    const ConstString section_name(".module_image");
amccarth wrote:
> I didn't notice before that target is a non-const ref.  Maybe the comment should explain why target is modified (and/or maybe in PlaceholderModule::Create).
Updated the function comment. This is similar to how other places set the section load address (ex. ObjectFileELF::SetLoadAddress)


More information about the lldb-commits mailing list