[PATCH] D95232: Symbolizer - Teach symbolizer to work directly on object file.

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:70-73
 
   std::unique_ptr<DIContext> Context = DWARFContext::create(Obj);
   Expected<SymbolizableModule *> InfoOrErr =
                      createModuleInfo(&Obj, std::move(Context), ModuleName);
----------------
See below, it seems like this is just a bug currently?


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:603-626
   std::unique_ptr<DIContext> Context;
   // If this is a COFF object containing PDB info, use a PDBContext to
   // symbolize. Otherwise, use DWARF.
   if (auto CoffObject = dyn_cast<COFFObjectFile>(Objects.first)) {
     const codeview::DebugInfo *DebugInfo;
     StringRef PDBFileName;
     auto EC = CoffObject->getDebugPDBInfo(DebugInfo, PDBFileName);
----------------
The logic for `CoffObject` is not repeated elsewhere, which doesn't seem intended?

It seems like this method should also be overloaded, i.e. rather than repeat the "try to find it, if not create it" logic in each overload above, just implement `LLVMSymbolizer::getOrCreateModuleInfo(const ObjectFile &)` too, and the bodies elsewhere should become identical. I'd also consider just making the private `*Common` methods templates over the type of the first argument, and have the overloads be one-line wrappers:

```
template <typename T>
Expected<DILineInfo> symbolizeCommonCode(T ModuleSpecifier, object::SectionedAddress ModuleOffset) {
    SymbolizableModule *Info;
    if (auto InfoOrErr = getOrCreateModuleInfo(ModuleSpecifier))
      Info = InfoOrErr.get();
    else
      return InfoOrErr.takeError();
    // ... the existing body in terms of Info and ModuleOffset
}
Expected<DILineInfo> LLVMSymbolizer::symbolizeCode(const std::string &ModuleName, object::SectionedAddress ModuleOffset) { return symbolizeCodeCommon(ModuleName, ModuleOffset); }
Expected<DILineInfo> LLVMSymbolizer::symbolizeCode(const ObjectFile &ModuleObjectFile, object::SectionedAddress ModuleOffset) { return symbolizeCodeCommon(ModuleObjectFile, ModuleOffset); }
```

I think this minimizes the amount of boilerplate, short of using the preprocessor, while still presenting the nice overloaded interface rather than the templated one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95232/new/

https://reviews.llvm.org/D95232