[PATCH] D41264: Fix faulty assertion for void type in debug info

[David Blaikie via llvm-commits]

This sort of shows a bit of a bug/weakness in LLVM's representation here.

This is a bit tricky to describe...

So, some back story: to ensure LLVM debug info metadata description of a
type is consistent between different translation units (this was important
for early versions of type merging I think - and also useful for DWARF type
units & somewhat for general sanity), any parts of a type that could vary
between translation units was removed from the member list and only listed
the type as their scope.

(this includes things like: nested types (since they could appear as a
declaration or a definition in different translation units), member
function template instantiations (because there could be different
instantiations in different translation units), and implicit special
members (copy ctor/move ctor, etc - depending on whether or not they're
used in a given translation unit))

But the way things are built if they're in the member list versus when
they're just listing the class as their parent scope is different - that's
how this bug exists. The DWARF backend would never see these member types
in the member list - so wasn't tested with these situations, etc.

Other quality bugs exist here because of this approach - member-specific
properties of these not-in-the-member-list entities aren't handled, such as
access specifiers.

Ah well, maybe should be fixed one day, but not sure many consumers care
greatly about access specifiers & not sure there's any other observable
difference.


I'm good with the more localized fix, looks right to me (given the
situation we're in).

On Thu, Dec 14, 2017 at 4:54 PM Adrian McCarthy via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> amccarth created this revision.
> amccarth added reviewers: rnk, russell.gallop.
> Herald added subscribers: JDevlieghere, hiraditya, aprantl.
>
> It appears the code uses nullptr to represent a void type in debug
> metadata, which led to an assertion failure when building
> DeltaAlgorithm.cpp with a self-hosted clang on Windows.
>
> I'm not sure why/if the problem was Windows-specific.
>
> Fixes bug https://bugs.llvm.org/show_bug.cgi?id=35543
>
>
> https://reviews.llvm.org/D41264
>
> Files:
>   llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
>   llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>
>
> Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> ===================================================================
> --- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> +++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> @@ -761,7 +761,8 @@
>
>  void DwarfUnit::addType(DIE &Entity, const DIType *Ty,
>                          dwarf::Attribute Attribute) {
> -  assert(Ty && "Trying to add a type that doesn't exist?");
> +  if (!Ty)
> +    return;
>    addDIEEntry(Entity, Attribute, DIEEntry(*getOrCreateTypeDIE(Ty)));
>  }
>
> Index: llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
> ===================================================================
> --- llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
> +++ llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
> @@ -163,7 +163,8 @@
>
>    DIType *BaseType = DDTy->getBaseType().resolve();
>
> -  assert(BaseType && "Unexpected invalid base type");
> +  if (!BaseType)
> +    return 0;
>
>    // If this is a derived type, go ahead and get the base type, unless
> it's a
>    // reference then it's just the size of the field. Pointer types have
> no need
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171219/78d5776e/attachment.html>