r259975 - Fix a crash when emitting dbeug info for forward-declared scoped enums.
Eric Christopher via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 23 11:07:54 PST 2016
Thanks Adrian!
On Tue, Feb 23, 2016 at 9:19 AM Adrian Prantl <aprantl at apple.com> wrote:
> --> r261657.
>
> Author: adrian <adrian at 91177308-0d34-0410-b5e6-96231b3b80d8>
> Date: Tue Feb 23 17:13:47 2016 +0000
>
> Remove an unnecessary workaround introduced in r259975. (NFC)
>
> Now that LLVM r259973 allows replacing a temporary type with another
> temporary we can rely on the original implementation.
>
> It is possible for enums to be created as part of
> their own declcontext. In this case a FwdDecl will be created
> twice. This doesn't cause a problem because both FwdDecls are
> entered into the ReplaceMap: finalize() will replace the first
> FwdDecl with the second and then replace the second with
> complete type.
>
> Thanks to echristo for pointing this out.
>
> git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@261657
>
> -- adrian
>
> > On Feb 22, 2016, at 5:26 PM, Eric Christopher <echristo at gmail.com>
> wrote:
> >
> > Hi Adrian,
> >
> > On Sat, Feb 20, 2016 at 1:18 PM Adrian Prantl <aprantl at apple.com> wrote:
> > This had me puzzled for a second, but then I figured out what happened
> :-)
> > The “crash” I quoted in the commit message really was an assertion
> failure, to be precise, the very assertion I relaxed in LLVM r259973 in
> order to be able to defer the building of the DeclContext implemented by
> this commit. Even with the assertion removed, I still believe that
> implementing it this way preferable, as it avoids creating the enum a
> second time.
> >
> > > On Feb 19, 2016, at 7:05 PM, Eric Christopher <echristo at gmail.com>
> wrote:
> > >
> > > Hi Adrian,
> > >
> > > I'm taking a look at this and can't duplicate using the testcase you
> gave without your patch(es) applied. It's also causing asserts in other
> code as you can have the forward decl left around and the CU isn't a valid
> context.
> >
> > Do you happen to have an example handy? I don’t quite understand the
> problem from the description — shouldn’t the temporary fwddecl be RAUWed
> unconditionally after the DeclContext is created?
> >
> >
> > I'd think so. It appears to get through to code generation unreplaced
> though and...
> >
> > If you’re blocked on this we can definitely revert the change in
> CGDebugInfo (but not the testcase), I just would like to understand what’s
> going on.
> >
> >
> > I am a bit. I don't have a reduction at the moment small enough to debug
> what's wrong with this and the current patch doesn't appear necessary so
> I'm unable to debug the original problem at this point to try to help more.
> >
> > -eric
> >
> > -- adrian
> >
> > > Can you take a look/revert until you've got a different testcase?
> There's not enough information in the commit to construct one up for you.
> > >
> > > Thanks!
> > >
> > > -eric
> > >
> > > On Fri, Feb 5, 2016 at 6:03 PM Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> > > Author: adrian
> > > Date: Fri Feb 5 19:59:09 2016
> > > New Revision: 259975
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=259975&view=rev
> > > Log:
> > > Fix a crash when emitting dbeug info for forward-declared scoped enums.
> > > It is possible for enums to be created as part of their own
> > > declcontext. We need to cache a placeholder to avoid the type being
> > > created twice before hitting the cache.
> > >
> > > <rdar://problem/24493203>
> > >
> > > Added:
> > > cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp
> > > Modified:
> > > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> > >
> > > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> > > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=259975&r1=259974&r2=259975&view=diff
> > >
> ==============================================================================
> > > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> > > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Feb 5 19:59:09 2016
> > > @@ -2051,13 +2051,25 @@ llvm::DIType *CGDebugInfo::CreateEnumTyp
> > > // If this is just a forward declaration, construct an appropriately
> > > // marked node and just return it.
> > > if (isImportedFromModule || !ED->getDefinition()) {
> > > - llvm::DIScope *EDContext = getDeclContextDescriptor(ED);
> > > llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation());
> > > +
> > > + // It is possible for enums to be created as part of their own
> > > + // declcontext. We need to cache a placeholder to avoid the type
> being
> > > + // created twice before hitting the cache.
> > > + llvm::DIScope *EDContext =
> DBuilder.createReplaceableCompositeType(
> > > + llvm::dwarf::DW_TAG_enumeration_type, "", TheCU, DefUnit, 0);
> > > +
> > > unsigned Line = getLineNumber(ED->getLocation());
> > > StringRef EDName = ED->getName();
> > > llvm::DIType *RetTy = DBuilder.createReplaceableCompositeType(
> > > llvm::dwarf::DW_TAG_enumeration_type, EDName, EDContext,
> DefUnit, Line,
> > > 0, Size, Align, llvm::DINode::FlagFwdDecl, FullName);
> > > +
> > > + // Cache the enum type so it is available when building the
> declcontext
> > > + // and replace the declcontect with the real thing.
> > > + TypeCache[Ty].reset(RetTy);
> > > + EDContext->replaceAllUsesWith(getDeclContextDescriptor(ED));
> > > +
> > > ReplaceMap.emplace_back(
> > > std::piecewise_construct, std::make_tuple(Ty),
> > > std::make_tuple(static_cast<llvm::Metadata *>(RetTy)));
> > >
> > > Added: cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp
> > > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp?rev=259975&view=auto
> > >
> ==============================================================================
> > > --- cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp (added)
> > > +++ cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp Fri Feb 5
> 19:59:09 2016
> > > @@ -0,0 +1,15 @@
> > > +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -std=c++11 \
> > > +// RUN: -triple thumbv7-apple-ios %s -o - | FileCheck %s
> > > +
> > > +// This forward-declared scoped enum will be created while building
> its own
> > > +// declcontext. Make sure it is only emitted once.
> > > +
> > > +struct A {
> > > + enum class Return;
> > > + Return f1();
> > > +};
> > > +A::Return* f2() {}
> > > +
> > > +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name:
> "Return",
> > > +// CHECK-SAME: flags: DIFlagFwdDecl,
> > > +// CHECK-NOT: tag: DW_TAG_enumeration_type, name:
> "Return"
> > >
> > >
> > > _______________________________________________
> > > cfe-commits mailing list
> > > cfe-commits at lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160223/e3fdaa7e/attachment-0001.html>
More information about the cfe-commits
mailing list