alvinhochun added inline comments.
================
Comment at: lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp:119
+ };
+ for (SectionType section_type : g_sections) {
+ if (SectionSP section_sp =
----------------
mstorsjo wrote:
> alvinhochun wrote:
> > mstorsjo wrote:
> > > I'm curious - this adds new logic (copied from SymbolVendorELF afaik?)
> > > for iterating over sections and finding the ones that contain the dwarf
> > > debug info. Prior to this change, finding debug info within the
> > > executable itself has worked just fine.
> > >
> > > What codepath and where has handled that? Has it fallen back on
> > > SymbolVendorELF so far?
> > >
> > > (If that used to work, why is a specific plugin for PECOFF needed at this
> > > point for debuglink?)
> > The SymbolVendorELF seems to only handle loading the external debug info
> > specified by the `.gnu_debuglink` section. The dwarf debug info already
> > embedded in the main executable should be handled somewhere else, which has
> > worked fine for both ELF and PE/COFF. Though I don't know where
> > specifically this was handled.
> >
> > SymbolVendorELF tries to downcast the object file to `ObjectFileELF *`, so
> > it could not have worked for PE/COFF.
> Ok, so what I misunderstood is that this function doesn't seem to handle
> dwarf debug info in the main executable after all - this only tries to dig up
> dwarf sections from `GetSymbolFileFileSpec()` or `GetDebugLink()`. So the
> existing code that locates dwarf sections in the executables themselves still
> runs as before.
>
> So then this seems reasonable.
>
> So essentially, if `GetDebugLink()` would be a virtual method, both
> SymbolVendorELF and SymbolVendorPECOFF could theoretically be merged into one?
> So essentially, if GetDebugLink() would be a virtual method, both
> SymbolVendorELF and SymbolVendorPECOFF could theoretically be merged into one?
Yes, they may very well be merged if no other platform-specific logic is
needed. There are some small differences now: SymbolVendorELF checks that
`obj_file->GetUUID()` is valid, but this fails the test I added so I removed
this check in SymbolVendorPECOFF. I also removed a few entries (those that
doesn't show up in `ObjectFilePECOFF`) from the `g_sections` list, but this
change is probably not necessary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126367/new/
https://reviews.llvm.org/D126367
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
