[llvm] r210221 - DebugInfo: Reapply r209984 (reverted in r210143), asserting that abstract DbgVariables have DIEs.

[Kuba Břečka]

Unfortunately, I don't have a test case and the existing ones don't trigger
the buggy behavior. I'll see what I can do.

Kuba

On Mon, Jun 23, 2014 at 11:02 AM, David Blaikie <dblaikie at gmail.com> wrote:

> Sure, though you'll need a (void)numAbstractScopes; there somewhere so
> that non-asserts builds don't warn about the unused variable (since
> it's only used in the assert, in a non-asserts build it appears to be
> not used at all and fires the warning).
>
> Did you reduce a good test case once you had this assertion, or did
> this fail on existing test cases once added?
>
> On Mon, Jun 23, 2014 at 10:20 AM, Kuba Břečka <kuba.brecka at gmail.com>
> wrote:
> > Hi David,
> > thanks for helping me figure this out. What do you think of adding this
> > assert permanently? See http://reviews.llvm.org/D4259.
> >
> > Kuba
> >
> >
> > On Mon, Jun 16, 2014 at 1:32 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Mon, Jun 16, 2014 at 1:11 PM, Kuba Břečka <kuba.brecka at gmail.com>
> >> wrote:
> >> > I'm experiencing a heap use-after-free memory issue after this patch.
> Is
> >> > this change (more context provided) intented?
> >> >
> >> > // Construct abstract scopes.
> >> > for (LexicalScope *AScope : LScopes.getAbstractScopesList()) {
> >> >   DISubprogram SP(AScope->getScopeNode());
> >> >   if (!SP.isSubprogram())
> >> >     continue;
> >> >   // Collect info for variables that were optimized out.
> >> >   DIArray Variables = SP.getVariables();
> >> >   for (unsigned i = 0, e = Variables.getNumElements(); i != e; ++i) {
> >> >     DIVariable DV(Variables.getElement(i));
> >> >     assert(DV && DV.isVariable());
> >> >     if (!ProcessedVars.insert(DV))
> >> >       continue;
> >> > -   findAbstractVariable(DV, DV.getContext());
> >> > +   getOrCreateAbstractVariable(DV, DV.getContext());
> >> >   }
> >> >   constructAbstractSubprogramScopeDIE(TheCU, AScope);
> >> > }
> >> >
> >> > It seems like getOrCreateAbstractVariable calls
> >> > LScopes.getOrCreateAbstractScope which can cause the
> AbstractScopesList
> >> > member vector to be resized (via push_back), introducing a
> >> > modify-container-while-iterating-it bug.
> >> >
> >> > Unfortunately, I don't have a test case for this, any ideas how could
> I
> >> > create one?
> >>
> >> If you put an assert after the getOrCreateAbstractVariable that the
> >> size of the abstract scopes list hasn't changed (eg:
> >>
> >> auto numAbstractScopes = LScopes.getAbstractScopesList();
> >> getOrCreateAbstractVariable...
> >> assert(numAbstractScopes == LScopes.getAbstractScopesList());
> >>
> >> Then you should be able to reliably reduce a test case either manually
> >> or using something like creduce, etc.
> >>
> >> If it's failing for the reason you've explained, that should fail with
> >> the assertion in the provided test case
> >> (test/DebugInfo/missing-abstract-variable.ll). Ideally we should just
> >> be iterating over the subprogram abstract scopes (& that code should
> >> never cause one of /those/ to be created at this late stage)... but
> >> I'm not sure if it'll be worth creating a separate list for them or
> >> just to change this loop to handle insertions (by using indexes
> >> instead of a range-for).
> >>
> >> - David
> >>
> >> >
> >> > Kuba
> >> >
> >> >
> >> >>
> >> >> Author: dblaikie
> >> >> Date: Wed Jun  4 18:50:52 2014
> >> >> New Revision: 210221
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=210221&view=rev
> >> >> Log:
> >> >> DebugInfo: Reapply r209984 (reverted in r210143), asserting that
> >> >> abstract
> >> >> DbgVariables have DIEs.
> >> >> Abstract variables within abstract scopes that are entirely optimized
> >> >> away in their first inlining are omitted because their scope is not
> >> >> present so the variable is never created. Instead, we should ensure
> the
> >> >> scope is created so the variable can be added, even if it's been
> >> >> optimized away in its first inlining.
> >> >> This fixes the incorrect debug info in missing-abstract-variable.ll
> >> >> (added in r210143) and passes an asserts self-hosting build, so
> >> >> hopefully there's not more of these issues left behind... *fingers
> >> >> crossed*.
> >> >> Modified:
> >> >>     llvm/trunk/include/llvm/CodeGen/LexicalScopes.h
> >> >>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> >> >>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> >> >>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> >> >>     llvm/trunk/test/DebugInfo/missing-abstract-variable.ll
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140623/018d143b/attachment.html>