[PATCH] Fix crash in DebugInfoFinder when adding a module with forward declared composite type.

[Duncan P. N. Exon Smith]

> On 2015 Apr 13, at 14:45, Anders Waldenborg <anders at 0x63.nu> wrote:
> 
> Hi echristo, dexonsmith,
> 
> The testcase that is included in the patch caused a crash when doing DebugInfoFinder::processModule on the module due to DCT->getElements() returning nullptr in DebugInfoFinder::processType.
> 
> By doing DCT.getElements() instead of DCT->getElements() one gets a DIArray instead of a raw MDTuple. The former has code to handle null as a 0-element array and therefore avoids the crash.
> 
> This appears to go slightly against the direction recent patches tries to take the code (in particular the "FIXME: Fix callers and remove condition on N." comments in include/llvm/IR/Metadata.h:MDTupleTypedArrayWrapper).  Would a better patch be to add a "if (!(DCT.getFlags() & DIDescriptor::FlagFwdDecl))" condition around the for loop maybe?
> 
> (side note: I tried to add a "assert(!(getFlags() & DIDescriptor::FlagFwdDecl) && "no elements for FwdDecl'd DICompositeType")" in DICompositeType::getElements, to catch the cases where one would get a nullptr back from getElements because it is a forward declaration. But this caused many testcases to fail, the ones I looked at seemed to handle the null case by putting the data into a DIArray and thus get the special handling for null in MDTupleTypedArrayWrapper)
> 
> REPOSITORY
>  rL LLVM
> 
> http://reviews.llvm.org/D9008
> 
> Files:
>  lib/IR/DebugInfo.cpp
>  test/DebugInfo/debuginfofinder-forward-declaration.ll

Thanks for the fix and the testcase.  LGTM with a minor change below.

Regarding the direction, I've discovered there are lots of callers
that are simplified by assuming these wrappers treat null as an empty
range.  I'm no longer convinced that the FIXME is correct.  Feel free
to delete it for now.  If I decide to do that after all, this patch is
just one extra caller to update.

> 
> Index: lib/IR/DebugInfo.cpp
> ===================================================================
> --- lib/IR/DebugInfo.cpp
> +++ lib/IR/DebugInfo.cpp
> @@ -234,7 +234,7 @@
>         processType(Ref.resolve(TypeIdentifierMap));
>       return;
>     }
> -    for (Metadata *D : DCT->getElements()->operands()) {
> +    for (Metadata *D : DCT.getElements()) {

Please leave this as `DCT->getElements()`.  I have out-of-tree
patches that delete all the accessors from the `DIDescriptor`
hierarchy (slowly working through committing them as I validate them
against ToT).  If for some reason that doesn't compile let me know.


>       if (DIType T = dyn_cast<MDType>(D))
>         processType(T);
>       else if (DISubprogram SP = dyn_cast<MDSubprogram>(D))
> Index: test/DebugInfo/debuginfofinder-forward-declaration.ll
> ===================================================================
> --- /dev/null
> +++ test/DebugInfo/debuginfofinder-forward-declaration.ll
> @@ -0,0 +1,42 @@
> +; RUN: opt -analyze -module-debuginfo < %s | FileCheck %s
> +
> +
> +; This module is generated from the following c-code:
> +;
> +; > union X;
> +; >
> +; > struct Y {
> +; >     union X *x;
> +; > };
> +; >
> +; > struct Y y;
> +
> +
> +; CHECK: Type: Y from /tmp/minimal.c:3 DW_TAG_structure_type
> +; CHECK: Type: x from /tmp/minimal.c:4 DW_TAG_member
> +; CHECK: Type: DW_TAG_pointer_type
> +; CHECK: Type: X from /tmp/minimal.c:1 DW_TAG_structure_type
> +
> +
> +%struct.Y = type { %struct.X* }
> +%struct.X = type opaque
> +
> + at y = common global %struct.Y zeroinitializer, align 8
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!10, !11}
> +!llvm.ident = !{!12}
> +
> +!0 = !MDCompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.7.0 (http://llvm.org/git/clang.git 247b30a043eb8f39ea3708e7e995089da0a6b00f) (http://llvm.org/git/llvm.git 6ecc7365a89c771fd229bdd9ffcc178684ea1aa5)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, retainedTypes: !2, subprograms: !2, globals: !3, imports: !2)
> +!1 = !MDFile(filename: "minimal.c", directory: "/tmp")
> +!2 = !{}
> +!3 = !{!4}
> +!4 = !MDGlobalVariable(name: "y", scope: !0, file: !1, line: 7, type: !5, isLocal: false, isDefinition: true, variable: %struct.Y* @y)
> +!5 = !MDCompositeType(tag: DW_TAG_structure_type, name: "Y", file: !1, line: 3, size: 64, align: 64, elements: !6)
> +!6 = !{!7}
> +!7 = !MDDerivedType(tag: DW_TAG_member, name: "x", scope: !5, file: !1, line: 4, baseType: !8, size: 64, align: 64)
> +!8 = !MDDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, align: 64)
> +!9 = !MDCompositeType(tag: DW_TAG_structure_type, name: "X", file: !1, line: 1, flags: DIFlagFwdDecl)
> +!10 = !{i32 2, !"Dwarf Version", i32 4}
> +!11 = !{i32 2, !"Debug Info Version", i32 3}
> +!12 = !{!"clang version 3.7.0 (http://llvm.org/git/clang.git 247b30a043eb8f39ea3708e7e995089da0a6b00f) (http://llvm.org/git/llvm.git 6ecc7365a89c771fd229bdd9ffcc178684ea1aa5)"}
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D9008.23698.patch>