[PATCH] D44761: Fix PR36793

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: ELF/InputFiles.cpp:134
       continue;
+    LineTables.push_back(LT);
 
----------------
grimar wrote:
> 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());
> ...
> ```
@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).


================
Comment at: ELF/InputFiles.h:20
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugLine.h"
 #include "llvm/IR/Comdat.h"
----------------
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?


================
Comment at: test/ELF/Inputs/multiple-cu.s:18
+.Lbegin0:
+	.short	4                       # DWARF version number
+	.long	.debug_abbrev           # Offset Into Abbrev. Section
----------------
Nit: comment doesn't line up here (also in test/ELF/multiple-cu.s).


https://reviews.llvm.org/D44761