[PATCH] D74560: Introduce DWARFDataExtractor::getInitialLength

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test:195
 
-# RESERVED: warning: parsing line table prologue at offset 0x00000048 unsupported reserved unit length found of value 0xfffffffe
+# RESERVED: warning: Unsupported reserved unit length of value 0xfffffffe at offset 0x48
 
----------------
labath wrote:
> jhenderson wrote:
> > This is a regression in the quality of the error, as it is not clear what section this error comes from. Same goes for many of the other changes in errors.
> When this is run normally on the command line (so both stdout and stderr go to the terminal), this comes out as:
> ```
> debug_line[0x00000048]
> warning: Unsupported reserved unit length of value 0xfffffffe at offset 0x48
> ...
> ```
> So, the section where this comes from should be fairly obvious. Nonetheless, it should be pretty easy to prepend the error message with the section name. Maybe something like "warning: parsing line table prologue at offset 0x00000048: unsupported reserved unit length found of value 0xfffffffe" or "warning: parsing line table prologue: unsupported reserved unit length of value 0xfffffffe at offset 0x48" ?
> 
> The question boils down to whether it should be the DataExtractor who embeds the offset into the message, or the line table code. 
I don't have an answer on what layer adds the offset context to the message, as I don't really have any particular opinion on that. However, it's worth noting that other tools use this code (e.g. LLD), and they aren't printing the `debug_line[0x00000048]` or indeed any of the rest of the dumped output, only the warning/error. As such, without the offset context, it's impossible to see the problem from those tools' output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74560