[PATCH] DebugInfo: hoist definition into global context when needed

[David Blaikie]

On Tue, Feb 24, 2015 at 3:12 PM, Saleem Abdulrasool <abdulras at fb.com> wrote:

> Hi dblaikie,
>
> When generating debug info for a static inline member which is initialized
> for
> the DLLExport storage class, hoist the definition into a non-composite type
> context.  Otherwise, we would trigger an assertion when generating the DIE
> for
> the associated global value as the debug context has a type association.
> This
> addresses PR22669.
>
> Thanks to David Blakie for help in coming up with a solution to this!
>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D7872
>
> Files:
>   lib/CodeGen/CGDebugInfo.cpp
>   test/CodeGenCXX/inline-dllexport-member.cpp
>
> Index: lib/CodeGen/CGDebugInfo.cpp
> ===================================================================
> --- lib/CodeGen/CGDebugInfo.cpp
> +++ lib/CodeGen/CGDebugInfo.cpp
> @@ -2376,9 +2376,11 @@
>    // FIXME: Generalize this for even non-member global variables where the
>    // declaration and definition may have different lexical decl contexts,
> once
>    // we have support for emitting declarations of (non-member) global
> variables.
> -  VDContext = getContextDescriptor(
> -      dyn_cast<Decl>(VD->isStaticDataMember() ?
> VD->getLexicalDeclContext()
> -                                              : VD->getDeclContext()));
> +  const DeclContext *DC = VD->isStaticDataMember() ?
> VD->getLexicalDeclContext()
> +                                                   : VD->getDeclContext();
> +  while (DC->isRecord())
> +    DC = DC->getParent();
>

I'd be OK with just putting this straight in the global decl context,
rather than the nearest enclosing namespace - but if you're going to do it
this way, it might be worth a test (eg: a namespace with a class with
another class inside the first class, and the static variable inside the
inner class) that demonstrates that this loop walks out to the right place.


> +  VDContext = getContextDescriptor(dyn_cast<Decl>(DC));
>  }
>
>  llvm::DISubprogram
> @@ -3171,6 +3173,7 @@
>  CGDebugInfo::getOrCreateStaticDataMemberDeclarationOrNull(const VarDecl
> *D) {
>    if (!D->isStaticDataMember())
>      return llvm::DIDerivedType();
> +
>    auto MI = StaticDataMemberCache.find(D->getCanonicalDecl());
>    if (MI != StaticDataMemberCache.end()) {
>      assert(MI->second && "Static data member declaration should still
> exist");
> Index: test/CodeGenCXX/inline-dllexport-member.cpp
> ===================================================================
> --- /dev/null
> +++ test/CodeGenCXX/inline-dllexport-member.cpp
> @@ -0,0 +1,12 @@
> +// RUN: %clang_cc1 -triple i686-windows-gnu -fms-compatibility -g
> -emit-llvm %s -o - \
> +// RUN:    | FileCheck %s
> +
> +struct __declspec(dllexport) s {
> +  static const unsigned int ui = 0;
> +};
> +
> +// CHECK: ; [ DW_TAG_variable ] [ui] [line 5] [def]
>

We should check that the variable is scoped to the right place (either null
or the CU or the nearest non-class (namespace) scope, etc).


> +// CHECK: ; [ DW_TAG_const_type ] [line 0, size 0, align 0, offset 0]
> [from unsigned int]
> +// CHECK: ; [ DW_TAG_base_type ] [unsigned int] [line 0, size 32, align
> 32, offset 0, enc DW_ATE_unsigned]
>

We don't need to test the const/base type - we haven't changed the code for
that in this change.


> +// CHECH: ; [ DW_TAG_member ] [ui] [line 5, size 0, align 0, offset 0]
> [static] [from ]
>

& then we probably don't need to check the member at all (again, we haven't
really touched that code) - but if you want to we could test that the
variable references the member as its declaration.


> +
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150224/85857143/attachment.html>