[PATCH] D63521: Teach the symbolizer lib symbolize objects directly.

[Yuanfang Chen via Phabricator via llvm-commits]

ychen marked an inline comment as done.
ychen added inline comments.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:82
+
+  // TODO: codeview is not supported yet.
+  Expected<SymbolizableModule *> InfoOrErr = createModuleInfo({&Obj, &Obj},
----------------
jhenderson wrote:
> This TODO seems somewhat arbitrary in this location. Is CodeView supported for other parts of LLVM symbolizer? What would happen if somebody tried to use this bit of code with CodeView, compared with old behaviour?
I'm not entirely sure. I know PDB and CodeView are not exactly the same thing. Looks like the name is used interchangeably in this context? `COFFObjectFile::getDebugPDBInfo ` said it is `COFF::IMAGE_DEBUG_TYPE_CODEVIEW`. If somebody uses this code with CodeView, `loadDataForEXE` would fail because of `Objects.first->getFileName()`, `symbolizeCode` would return error. Old behavior of `symbolizeCode(const ObjectFile &Obj, ..)` would always create a DWARFContext.

```
  // 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);
    if (!EC && DebugInfo != nullptr && !PDBFileName.empty()) {
      using namespace pdb;
      std::unique_ptr<IPDBSession> Session;
      if (auto Err = loadDataForEXE(PDB_ReaderType::DIA,
                                    Objects.first->getFileName(), Session)) {
        Modules.insert(
            std::make_pair(ModuleName, std::unique_ptr<SymbolizableModule>()));
        // Return along the PDB filename to provide more context
        return createFileError(PDBFileName, std::move(Err));
      }
      Context.reset(new PDBContext(*CoffObject, std::move(Session)));
    }
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63521