[PATCH] D87656: [llvm-dwarfdump] --show-sources option to show all sources

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:455
+  for (const auto &CU : DICtx.compile_units()) {
+    const auto *LT = DICtx.getLineTableForUnit(CU.get());
+    for (uint32_t I = 1; I <= LT->Prologue.FileNames.size(); ++I) {
----------------
jhenderson wrote:
> Higuoxing wrote:
> > jhenderson wrote:
> > > I think we need testing for multiple CUs. The current test only checks a single one. This might go against the yaml2obj usage suggested above though (@Higuoxing, is there support for multiple tables in .debug_line yet?).
> > > is there support for multiple tables in .debug_line yet?
> > 
> > Yes, `yaml2obj` supports emitting multiple line tables. I'm able to help craft these test cases.
> > 
> > ----------------------------
> > 
> > It looks that `LT` isn't checked. If a compilation unit doesn't have an associated line table, `llvm-dwarfdump --show-sources` will crash.
> > 
> > ```
> > const auto *LT = DICtx.getLineTableForUnit(CU.get()); // Can be a null pointer.
> > for (uint32_t I = 1; I <= LT->Prologue.FileNames.size(); ++I) {
> >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   ...
> > }
> > ```
> > 
> > We can reproduce it using the following test case.
> > 
> > ```
> > $ yaml2obj %s | llvm-dwarfdump --show-sources -
> > ```
> > 
> > ```
> > --- !ELF
> > FileHeader:
> >   Class:   ELFCLASS64
> >   Data:    ELFDATA2LSB
> >   Type:    ET_EXEC
> >   Machine: EM_X86_64
> > DWARF:
> >   debug_info:
> >     - Version: 4
> > ```
> Nice catch! In fact, do we really need to use the CUs at all for this? Could we not just iterate over all line tables? That would allow this to work when there is no .debug_info data too (which the DWARF spec implies is permitted).
I don't know how carefully the spec says it is permitted, but certainly I've heard committee members talk about stripping everything but .debug_line (and with v5, .debug_line_str) from an object file.

In DWARF v4, technically the primary source file & compilation dir could be omitted from the line table, although in practice I think that never happens. In v5 the primary source file & dir are supposed to be explicit in the line table, so I think ignoring .debug_info ought to be okay in general.


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

https://reviews.llvm.org/D87656