This is an archive of the discontinued LLVM Phabricator instance.

Fix PR36793
ClosedPublic

Authored by espindola on Mar 21 2018, 4:15 PM.

Details

Summary

With this patch lld will iterate over compile units to file the line tables instead of assuming there is only one at offset 0.

Diff Detail

Event Timeline

espindola created this revision.Mar 21 2018, 4:15 PM
arichardson added inline comments.Mar 21 2018, 4:26 PM
ELF/InputFiles.cpp
138

Maybe use .get() instead of &*?

address review comments.

espindola marked an inline comment as done.Mar 21 2018, 4:34 PM

It turns out this can be simplified slightly. I attached the patch.

ELF/InputFiles.cpp
134

Code above duplicates code from DWARFContext::getLineTableForUnit.
Since we now do not explicitly support case when there is no .debug_info (D44760),
we do not need to parse line tables explicitly here I think, so I would suggest the
different approach:

We could have DWARFContext as a member of ObjectFile. And here it could be just:

template <class ELFT> void ObjFile<ELFT>::initializeDwarf() {
  Dwarf = llvm::make_unique<DWARFContext>(
      llvm::make_unique<LLDDwarfObj<ELFT>>(this));
  for (std::unique_ptr<DWARFCompileUnit> &CU : Dwarf->compile_units()) {
    const DWARFDebugLine::LineTable *LT = Dwarf->getLineTableForUnit(CU.get());
...
203

getFileLineInfoForAddress returns true on success according to specification.
It can be the following I think:

for (const llvm::DWARFDebugLine::LineTable *LT : LineTables)
  if (LT->getFileLineInfoForAddress(
      S->getOffsetInFile() + Offset, nullptr,
      DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, Info))
  return Info;
jhenderson added inline comments.Mar 22 2018, 3:17 AM
ELF/InputFiles.cpp
134

@grimar is probably right, although it looks like it would mean I'd have to propagate the Errors I'm adding to getOrParseLineTableUnit further up the stack than they currently need to be (see D44560).

ELF/InputFiles.h
20

Do we need this header here? All I see that has changed is the use of pointers in a vector, which can be forward declared, can't they?

test/ELF/Inputs/multiple-cu.s
18

Nit: comment doesn't line up here (also in test/ELF/multiple-cu.s).

espindola added inline comments.Mar 22 2018, 10:48 AM
ELF/InputFiles.h
20

I don't think it is possible to forward declare an inner class (llvm::DWARFDebugLine::LineTable).

Use getLineTableForUnit.

ruiu accepted this revision.Mar 22 2018, 3:57 PM

LGTM

This revision is now accepted and ready to land.Mar 22 2018, 3:57 PM
espindola closed this revision.Mar 22 2018, 5:38 PM