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

[David Blaikie]

On Wed, Feb 25, 2015 at 8:33 AM, Saleem Abdulrasool <abdulras at fb.com> wrote:

> 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,15 @@
>    // 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();
> +  // When a record type contains an in-line initialization of a static
> data
> +  // member, and the record type is marked as __declspec(dllexport),


I'd continue this with "an implicit definition of the member will be
created in the record context" instead of the rest of the sentence below.
The creation of the implicit definition (& where it's scoped) seems to be
the interesting bit. Could also say "DWARF doesn't seem to have a nice way
to describe this that consumers are likely to understand, so fake the
'normal' situation of a definition outside the class by putting the
definition in the global scope"


> the member
> +  // debug DeclContext will be a type, which will cause an assertion.
> Hoist the
> +  // context to the global scope.
> +  if (DC->isRecord())
> +    DC = CGM.getContext().getTranslationUnitDecl();
> +  VDContext = getContextDescriptor(dyn_cast<Decl>(DC));
>  }
>
>  llvm::DISubprogram
> @@ -3171,6 +3177,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,11 @@
> +// 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: !5 = !{!"{{.*}}inline-dllexport-member.cpp",
>

^ Probably not important? (what's this intended to check?)


> +// CHECK: !8, {{.*}}, i32* @_ZN1s2uiE, {{.*}}} ; [ DW_TAG_variable ] [ui]
> [line 5] [def]
> +// CHECK: !8 = !{!"0x29", !5} ; [ DW_TAG_file_type ]
> [{{.*}}inline-dllexport-member.cpp]
>

Oh, I see, you're checking the specific file name. I don't think that's
important. Simply checking that the scope of the DW_TAG_variable is a
DW_TAG_file_type seems sufficient.

If possible, avoid using explicit metadata numbers - they can
fluctuate/change for unrelated reasons (if we change other bits of debug
info). So try to match the number using a FileCheck match ([[SCOPE:![^,]*]]
- providing enough context in the DW_TAG_variable match line to ensure you
get the right metadata reference (the one for the scope, not the one for
the type, etc)) and then verify that:

[[SCOPE]] = {{.*}} ; [ DW_TAG_file_type ]

would be sufficient, I think.


> +
>
> 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/20150225/27087280/attachment.html>