[Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

[Greg Clayton via lldb-commits]

> On Dec 10, 2018, at 3:11 PM, Leonard Mosescu <mosescu at google.com> wrote:
> 
> I can see how this works for the PDB, no-module-binary case. What about the PDB & module-binary case?

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.

> 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.

Yeah, my case should handle the following cases:
- Minidumps Placeholder modules only, no real binaries, add symbols by downloading them and using "target symbols add ..."
- 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 (GetTarget().GetSharedModule(...) in the current code) and they will use real object files, not placeholder files. "target symbols add ..." still works in this case

But we can easily layer on top of your PlaceholderModule and the placeholder object file if needed. 

Greg

> 
> 
> On Mon, Dec 10, 2018 at 2:48 PM Greg Clayton via Phabricator <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
> clayborg added a comment.
> 
> 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:
> 
>   auto placeholder_module =
>       std::make_shared<PlaceholderModule>(module_spec, GetTarget());
> 
> Here is the copy of the PlaceholderModule that does what was discussed above:
> 
>   //------------------------------------------------------------------
>   /// A placeholder module used for minidumps, where the original
>   /// object files may not be available (so we can't parse the object
>   /// files to extract the set of sections/segments)
>   ///
>   /// This placeholder module has a single synthetic section (.module_image)
>   /// which represents the module memory range covering the whole module.
>   //------------------------------------------------------------------
>   class PlaceholderModule : public Module {
>   public:
>     PlaceholderModule(const ModuleSpec &module_spec, Target& target) :
>       Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()),
>       m_target_wp(target.shared_from_this()),
>       m_base_of_image(LLDB_INVALID_ADDRESS), m_size_of_image(0) {
>       if (module_spec.GetUUID().IsValid())
>         SetUUID(module_spec.GetUUID());
>     }
> 
>     // Creates a synthetic module section covering the whole module image (and
>     // sets the section load address as well)
>     void CreateImageSection(const MinidumpModule *module) {
>       m_base_of_image = module->base_of_image;
>       m_size_of_image = module->size_of_image;
>       TargetSP target_sp = m_target_wp.lock();
>       if (!target_sp)
>         return;
>       const ConstString section_name(".module_image");
>       lldb::SectionSP section_sp(new Section(
>           shared_from_this(),     // Module to which this section belongs.
>           nullptr,                // ObjectFile
>           0,                      // Section ID.
>           section_name,           // Section name.
>           eSectionTypeContainer,  // Section type.
>           module->base_of_image,  // VM address.
>           module->size_of_image,  // VM size in bytes of this section.
>           0,                      // Offset of this section in the file.
>           module->size_of_image,  // Size of the section as found in the file.
>           12,                     // Alignment of the section (log2)
>           0,                      // Flags for this section.
>           1));                    // Number of host bytes per target byte
>       section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
>       GetSectionList()->AddSection(section_sp);
>       target_sp->GetSectionLoadList().SetSectionLoadAddress(
>           section_sp, module->base_of_image);
>     }
> 
>     ObjectFile *GetObjectFile() override {
>       // Since there is no object file for these place holder modules, check
>       // if the symbol file spec has been set, and if so, then transfer it over
>       // to the file spec so the module can make a real object file out of it.
>       if (m_symfile_spec) {
>         // We need to load the sections once. We check of m_objfile_sp is valid.
>         // If it is, we already have loaded the sections. If it isn't, we will
>         // load the sections.
>         const bool load_sections = !m_objfile_sp;
>         if (load_sections) {
>           m_platform_file = m_file;
>           m_file = m_symfile_spec;
>         }
>         ObjectFile *obj_file = Module::GetObjectFile();
>         if (load_sections && obj_file) {
>           TargetSP target_sp = m_target_wp.lock();
>           // The first time we create the object file from the external symbol
>           // file, we must load its sections and unload the ".module_image"
>           // section
>           bool changed;
>           SetLoadAddress(*target_sp, m_base_of_image, false, changed);
>         }
>         return obj_file;
>       }
>       return nullptr;
>     }
> 
>     SectionList *GetSectionList() override {
>       return Module::GetUnifiedSectionList();
>     }
>   protected:
>     // In case we need to load the sections in PlaceholderModule::GetObjectFile()
>     // after a symbol file has been specified, we might need the target.
>     lldb::TargetWP m_target_wp;
>     lldb::addr_t m_base_of_image;
>     lldb::addr_t m_size_of_image;
>   };
> 
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55142/new/ <https://reviews.llvm.org/D55142/new/>
> 
> https://reviews.llvm.org/D55142 <https://reviews.llvm.org/D55142>
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181210/a45a3b95/attachment.html>