<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Dec 10, 2018, at 3:11 PM, Leonard Mosescu <<a href="mailto:mosescu@google.com" class="">mosescu@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">I can see how this works for the PDB, no-module-binary case. What about the PDB & module-binary case?</div></div></blockquote><div><br class=""></div>That would work fine because the symbol vendor will make an object file from the m_symfile_spec and use both the original binary + the symbol file to make symbols.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">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></div></blockquote><div><br class=""></div><div>Yeah, my case should handle the following cases:</div><div>- Minidumps Placeholder modules only, no real binaries, add symbols by downloading them and using "target symbols add ..."</div><div>- Download real binaries up front and load them into LLDB, then load the core file. For any binaries we do have, we should grab them from the global module cache (<span style="color: rgb(36, 82, 86); background-color: rgb(255, 255, 255);" class="">GetTarget</span><span style="background-color: rgb(255, 255, 255);" class="">().</span><span style="background-color: rgb(255, 255, 255);" class=""><font color="#245256" class="">GetSharedModule(...) in the current code) and they will use real object files, not placeholder files. "target symbols add ..." still <span style="caret-color: rgb(36, 82, 86);" class="">works</span> in this case</font></span></div><div><br class=""></div>But we can easily layer on top of your PlaceholderModule and the placeholder object file if needed. </div><div><br class=""></div><div>Greg</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""><div class=""></div></div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, Dec 10, 2018 at 2:48 PM Greg Clayton via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">clayborg added a comment.<br class="">
<br class="">
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 class="">
<br class="">
  auto placeholder_module =<br class="">
      std::make_shared<PlaceholderModule>(module_spec, GetTarget());<br class="">
<br class="">
Here is the copy of the PlaceholderModule that does what was discussed above:<br class="">
<br class="">
  //------------------------------------------------------------------<br class="">
  /// A placeholder module used for minidumps, where the original<br class="">
  /// object files may not be available (so we can't parse the object<br class="">
  /// files to extract the set of sections/segments)<br class="">
  ///<br class="">
  /// This placeholder module has a single synthetic section (.module_image)<br class="">
  /// which represents the module memory range covering the whole module.<br class="">
  //------------------------------------------------------------------<br class="">
  class PlaceholderModule : public Module {<br class="">
  public:<br class="">
    PlaceholderModule(const ModuleSpec &module_spec, Target& target) :<br class="">
      Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()),<br class="">
      m_target_wp(target.shared_from_this()),<br class="">
      m_base_of_image(LLDB_INVALID_ADDRESS), m_size_of_image(0) {<br class="">
      if (module_spec.GetUUID().IsValid())<br class="">
        SetUUID(module_spec.GetUUID());<br class="">
    }<br class="">
<br class="">
    // Creates a synthetic module section covering the whole module image (and<br class="">
    // sets the section load address as well)<br class="">
    void CreateImageSection(const MinidumpModule *module) {<br class="">
      m_base_of_image = module->base_of_image;<br class="">
      m_size_of_image = module->size_of_image;<br class="">
      TargetSP target_sp = m_target_wp.lock();<br class="">
      if (!target_sp)<br class="">
        return;<br class="">
      const ConstString section_name(".module_image");<br class="">
      lldb::SectionSP section_sp(new Section(<br class="">
          shared_from_this(),     // Module to which this section belongs.<br class="">
          nullptr,                // ObjectFile<br class="">
          0,                      // Section ID.<br class="">
          section_name,           // Section name.<br class="">
          eSectionTypeContainer,  // Section type.<br class="">
          module->base_of_image,  // VM address.<br class="">
          module->size_of_image,  // VM size in bytes of this section.<br class="">
          0,                      // Offset of this section in the file.<br class="">
          module->size_of_image,  // Size of the section as found in the file.<br class="">
          12,                     // Alignment of the section (log2)<br class="">
          0,                      // Flags for this section.<br class="">
          1));                    // Number of host bytes per target byte<br class="">
      section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);<br class="">
      GetSectionList()->AddSection(section_sp);<br class="">
      target_sp->GetSectionLoadList().SetSectionLoadAddress(<br class="">
          section_sp, module->base_of_image);<br class="">
    }<br class="">
<br class="">
    ObjectFile *GetObjectFile() override {<br class="">
      // Since there is no object file for these place holder modules, check<br class="">
      // if the symbol file spec has been set, and if so, then transfer it over<br class="">
      // to the file spec so the module can make a real object file out of it.<br class="">
      if (m_symfile_spec) {<br class="">
        // We need to load the sections once. We check of m_objfile_sp is valid.<br class="">
        // If it is, we already have loaded the sections. If it isn't, we will<br class="">
        // load the sections.<br class="">
        const bool load_sections = !m_objfile_sp;<br class="">
        if (load_sections) {<br class="">
          m_platform_file = m_file;<br class="">
          m_file = m_symfile_spec;<br class="">
        }<br class="">
        ObjectFile *obj_file = Module::GetObjectFile();<br class="">
        if (load_sections && obj_file) {<br class="">
          TargetSP target_sp = m_target_wp.lock();<br class="">
          // The first time we create the object file from the external symbol<br class="">
          // file, we must load its sections and unload the ".module_image"<br class="">
          // section<br class="">
          bool changed;<br class="">
          SetLoadAddress(*target_sp, m_base_of_image, false, changed);<br class="">
        }<br class="">
        return obj_file;<br class="">
      }<br class="">
      return nullptr;<br class="">
    }<br class="">
<br class="">
    SectionList *GetSectionList() override {<br class="">
      return Module::GetUnifiedSectionList();<br class="">
    }<br class="">
  protected:<br class="">
    // In case we need to load the sections in PlaceholderModule::GetObjectFile()<br class="">
    // after a symbol file has been specified, we might need the target.<br class="">
    lldb::TargetWP m_target_wp;<br class="">
    lldb::addr_t m_base_of_image;<br class="">
    lldb::addr_t m_size_of_image;<br class="">
  };<br class="">
<br class="">
<br class="">
CHANGES SINCE LAST ACTION<br class="">
  <a href="https://reviews.llvm.org/D55142/new/" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D55142/new/</a><br class="">
<br class="">
<a href="https://reviews.llvm.org/D55142" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D55142</a><br class="">
<br class="">
<br class="">
<br class="">
</blockquote></div>
</div></blockquote></div><br class=""></body></html>