[llvm-dev] DWARF debug line error handling changes

[James Henderson via llvm-dev]

On Mon, 27 Jan 2020 at 17:05, Adrian Prantl <aprantl at apple.com> wrote:

>
>
> On Jan 27, 2020, at 5:51 AM, James Henderson <jh7370.2008 at my.bristol.ac.uk>
> wrote:
>
> Hi all,
>
> Since December, I've made several changes to the DWARF debug line parser
> to improve its error handling. Some examples include removing redundant
> error checks[1] and making address size mismatches non-fatal to parsing[2],
> with several more about to land or being planned.
>
> David Blaikie recommended I post something to give a bit more context to
> these changes. I am a member of Sony's Linker and Binutils team, and as
> part of the linking process, we make updates to the debug line section in
> order to handle GC-section processing and similar modifications. In order
> for these operations to be safe, we need the input line program to be in a
> good state. As a result, in addition to the existing LLVM error checks, we
> have added several additional downstream checks. These were added
> downstream at the time for speed, but we've always had the intention of
> bringing these to the wider community. We now have the time to do this.
>
> There are broadly-speaking two kinds of changes we are making:
>
> 1) Adding additional new checks to make sure the table is valid. Where
> possible, these are made as late as reasonable. That way, we don't emit
> errors or warnings until it is necessary, and potentially therefore not at
> all in some cases.
>
> 2) Making existing error checks less fatal, i.e. changing the code to
> allow the parser to continue even though something doesn't look right in
> the code.
>
>
> Thanks for all you work. I'm sure that the additional error handling and
> error resilience benefits other clients such as LLDB which also have to
> deal with inputs from a variety of producers.
>
> This allows consumers to gather more information, whether for displaying
> or otherwise. In several cases, the change is to assume that the length
> recorded in a field is correct, even if it doesn't match what either the
> standard says should be the length. This won't always be the right thing to
> do (it might be that just the length field is correct), but providing more
> information, together with a warning that should indicate that something
> may not be quite right, is surely more useful than refusing to give the
> extra information. I picked the recorded length because other areas of the
> code already assume it to be correct, and use it to iterate onto other
> parts of the line table section.
>
>
> This is something we'll need to decide on a case-by-case basis. If there
> are producers out there with known bugs that we want to support, this may
> be the right call. Generally though, I'd always prefer to err on the side
> of correctness over feeding potentially wrong or misleading information to
> the user. But let's discuss this separately for each case.
>
> -- adrian
>

I agree, a case-by-case basis is perhaps correct. My thinking is that
clients that care about correctness over providing a best guess can change
the error callbacks to ignore data from a line table/section where
something has gone wrong. Consequently, the parser should provide as much
info as it could possibly get (some of which might be wrong), as long as it
uses one of the callbacks appropriately when something looks suspicious.


>
>
> I hope this provides greater context for these changes!
>
> Regards,
>
> James
>
> [1]
> https://github.com/llvm/llvm-project/commit/418cd8216b41f4c08e0e1b22feda381d9b2345da
> [2]
> https://github.com/llvm/llvm-project/commit/07804f75a6cc506fada40c474f1e60840ce737d8
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200128/8f06b9e7/attachment.html>