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

[Kuba Břečka]

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/34037ea3/attachment.html>