[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM.



================
Comment at: source/Core/Module.cpp:1286
+    if (SymbolVendor *vendor = GetSymbolVendor())
+      vendor->CreateSections(*GetUnifiedSectionList());
   }
----------------
labath wrote:
> clayborg wrote:
> > should we pass "obj_file" down into the SymbolVendor::CreateSections(...) call for the case where the symbol vendor is using the same ObjectFile that it can just return?
> I don't think an extra parameter is needed to achieve this, as this is something that the symbol vendor should already know about based on how it was constructed.
> 
> And after looking up the implementation, it seems this is already how it works right now: SymbolVendorELF (the only non-trivial implementation of this function) is only constructed if it manages to find debug info in an alternative symbol file (there's an `if (llvm::sys::fs::equivalent(candidate_file_spec.GetPath(), module_file_spec.GetPath()))` check in `Symbols::LocateExecutableSymbolFile`). If we fail to construct a SymbolVendorELF then a default SymbolVendor instance is created in `SymbolVendor::FindPlugin` (and that one has a no-op implementation of this function).
> 
I agree with Pavel's reasoning here.


https://reviews.llvm.org/D42955