[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[David Blaikie via lldb-commits]

On Tue, Jun 4, 2019 at 4:15 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> labath added a comment.
>
> In D62634#1527575 <https://reviews.llvm.org/D62634#1527575>, @dblaikie
> wrote:
>
> > > 1. We need to figure out whether there's a bug in clang and fix it. +
> @dblaikie for help with that
> >
> > This is intentional behavior in clang (controllable under the
> -f[no-]split-dwarf-inlining flag, and modified by the
> -f[no-]debug-info-for-profiling flag).
> >
> > This extra debug info is used for online symbolication (in the absence
> of .dwo files) - such as for the sanitizers (accurate symbolication is
> necessary for correctness in msan, due to msan's necessary use of
> blacklisting to avoid certain false positives) and some forms of sample
> based profiling collection.
> >
> > In the default mode, clang includes, as you noted, just the subprograms
> that have inlined subroutines in them in this split-dwarf-inlining data.
> >  In -fdebug-info-for-profiling all subprograms are described in the
> skeleton CU with some minimal attributes (they don't need parameters, local
> variables/scopes, etc) necessary to do certain profile things I don't know
> lots about.
> >
> > Chances are it's probably best for a debugger to ignore these entries.
>
>
> Thanks for explaining David. It sounds like we can safely ignore these
> entries if we have successfully loaded the dwo file, as it will contain a
> superset of information. If we cannot find the dwo file for any reason,
> then this information might be useful as a rough approximation of debug
> info. However, in this case, there's no dwo file around, so there isn't a
> possibility for confusion. I'll prepare a patch doing something like that.
>

Yep - safe to ignore. "nice to have" if the .dwo file can't be loaded, but
not a huge motivation/I wouldn't bend over backwards to make that work if
it's a significant burden.


>
> Over email, @clayborg wrote:
>
> > > Actually, there was a subtle change of behavior here. Before this
> change, if we tried to parse a DIE using an abbreviation table from a
> different file, we would most likely fail immediately because of the
> fail-safe in GetAbbreviationDeclarationPtr. Now, we just do a blind read
> and return bogus data.
> >
> > This seems just like a straight up bug we need to fix in LLDB. It
> shouldn't affect or require any changes from any compilers. Seems like a
> lldbassert that maybe verifies that the abbreviation we use is valid for
> the current CU in debug builds would be nice so we can trigger this bug
> when we run the test suite or locally against an example program would be
> good. Depending on that fact that we have a mismatch in the abbrev index
> seems fragile. Most .debug_abbrev tables start out with a
> DW_TAG_compile_unit. So if the abbrev index magically matches, that still
> doesn't mean the we will be decoding the right data even if the index
> matches.
>
>
> Yes, this part is a bug, but in order to figure out how to fix it, I had
> to know whether the compiler behavior is intentional, and whether we need
> to access that data. If it turned out we did, the fix would be more
> involved, because we'd also need to change the UID encoding so that we can
> reference these DIEs correctly, etc. Given that it seems we can just ignore
> these debug info entries, the fix will be relatively simple.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62634/new/
>
> https://reviews.llvm.org/D62634
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190604/680f5bb4/attachment.html>