<div dir="ltr">I can see how this works for the PDB, no-module-binary case. What about the PDB & module-binary case?<div><br></div><div>Maybe I missed the rationale, but I think that modeling the general case: module and symbols are separate files, is a better foundation for the particular case where the symbols are embedded in the binary file.</div><div><br><div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 10, 2018 at 2:48 PM Greg Clayton via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">clayborg added a comment.<br>
<br>
FYI: my approach to getting symbols to load was a bit different. I always let a simple PlaceholderModule be created, but I played some tricks in the GetObjectFile() method if someone had setting symbols for the module with "target symbols add ..". I will attach my PlaceholderModule so you can see what I had done. Since these modules always must be associated with a target, I keep a weak pointer to the target in the constructor. Then later, if someone does "target symbols add ..." the module will have Module:: m_symfile_spec filled in, so I set the m_platform_file to be m_file, and then move m_symfile_spec into m_file and then call Module::GetObjectFile(). This is nice and clean because you don't have to make up fake symbols to fake sections. When you create the placeholder module it needs the target:<br>
<br>
auto placeholder_module =<br>
std::make_shared<PlaceholderModule>(module_spec, GetTarget());<br>
<br>
Here is the copy of the PlaceholderModule that does what was discussed above:<br>
<br>
//------------------------------------------------------------------<br>
/// A placeholder module used for minidumps, where the original<br>
/// object files may not be available (so we can't parse the object<br>
/// files to extract the set of sections/segments)<br>
///<br>
/// This placeholder module has a single synthetic section (.module_image)<br>
/// which represents the module memory range covering the whole module.<br>
//------------------------------------------------------------------<br>
class PlaceholderModule : public Module {<br>
public:<br>
PlaceholderModule(const ModuleSpec &module_spec, Target& target) :<br>
Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()),<br>
m_target_wp(target.shared_from_this()),<br>
m_base_of_image(LLDB_INVALID_ADDRESS), m_size_of_image(0) {<br>
if (module_spec.GetUUID().IsValid())<br>
SetUUID(module_spec.GetUUID());<br>
}<br>
<br>
// Creates a synthetic module section covering the whole module image (and<br>
// sets the section load address as well)<br>
void CreateImageSection(const MinidumpModule *module) {<br>
m_base_of_image = module->base_of_image;<br>
m_size_of_image = module->size_of_image;<br>
TargetSP target_sp = m_target_wp.lock();<br>
if (!target_sp)<br>
return;<br>
const ConstString section_name(".module_image");<br>
lldb::SectionSP section_sp(new Section(<br>
shared_from_this(), // Module to which this section belongs.<br>
nullptr, // ObjectFile<br>
0, // Section ID.<br>
section_name, // Section name.<br>
eSectionTypeContainer, // Section type.<br>
module->base_of_image, // VM address.<br>
module->size_of_image, // VM size in bytes of this section.<br>
0, // Offset of this section in the file.<br>
module->size_of_image, // Size of the section as found in the file.<br>
12, // Alignment of the section (log2)<br>
0, // Flags for this section.<br>
1)); // Number of host bytes per target byte<br>
section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);<br>
GetSectionList()->AddSection(section_sp);<br>
target_sp->GetSectionLoadList().SetSectionLoadAddress(<br>
section_sp, module->base_of_image);<br>
}<br>
<br>
ObjectFile *GetObjectFile() override {<br>
// Since there is no object file for these place holder modules, check<br>
// if the symbol file spec has been set, and if so, then transfer it over<br>
// to the file spec so the module can make a real object file out of it.<br>
if (m_symfile_spec) {<br>
// We need to load the sections once. We check of m_objfile_sp is valid.<br>
// If it is, we already have loaded the sections. If it isn't, we will<br>
// load the sections.<br>
const bool load_sections = !m_objfile_sp;<br>
if (load_sections) {<br>
m_platform_file = m_file;<br>
m_file = m_symfile_spec;<br>
}<br>
ObjectFile *obj_file = Module::GetObjectFile();<br>
if (load_sections && obj_file) {<br>
TargetSP target_sp = m_target_wp.lock();<br>
// The first time we create the object file from the external symbol<br>
// file, we must load its sections and unload the ".module_image"<br>
// section<br>
bool changed;<br>
SetLoadAddress(*target_sp, m_base_of_image, false, changed);<br>
}<br>
return obj_file;<br>
}<br>
return nullptr;<br>
}<br>
<br>
SectionList *GetSectionList() override {<br>
return Module::GetUnifiedSectionList();<br>
}<br>
protected:<br>
// In case we need to load the sections in PlaceholderModule::GetObjectFile()<br>
// after a symbol file has been specified, we might need the target.<br>
lldb::TargetWP m_target_wp;<br>
lldb::addr_t m_base_of_image;<br>
lldb::addr_t m_size_of_image;<br>
};<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D55142/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D55142/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D55142" rel="noreferrer" target="_blank">https://reviews.llvm.org/D55142</a><br>
<br>
<br>
<br>
</blockquote></div>