r221385 - [DebugInfo] Do not record artificial global initializer functions in the DeclCache.

[Frédéric Riss]

> On Nov 5, 2014, at 1:33 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Nov 5, 2014 at 11:19 AM, Frederic Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
> Author: friss
> Date: Wed Nov  5 13:19:04 2014
> New Revision: 221385
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=221385&view=rev <http://llvm.org/viewvc/llvm-project?rev=221385&view=rev>
> Log:
> [DebugInfo] Do not record artificial global initializer functions in the DeclCache.
> 
> When we are generating the global initializer functions, we call
> CGDebugInfo::EmitFunctionStart() with a valid decl which is describing
> the initialized global variable. Do not update the DeclCache with this
> key as it will overwrite the the cached variable DIGlobalVariable with
> the newly created artificial DISubprogram.
> 
> One could wonder if we should put artificial subprograms in the DIE tree
> at all (there are vaild uses for them carrying line information though).
> 
> Modified:
>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>     cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=221385&r1=221384&r2=221385&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=221385&r1=221384&r2=221385&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Nov  5 13:19:04 2014
> @@ -2527,7 +2527,10 @@ void CGDebugInfo::EmitFunctionStart(Glob
>        getOrCreateFunctionType(D, FnType, Unit), Fn->hasInternalLinkage(),
>        true /*definition*/, ScopeLine, Flags, CGM.getLangOpts().Optimize, Fn,
>        TParamsArray, getFunctionDeclaration(D));
> -  if (HasDecl)
> +  // We might get here with a VarDecl in the case we're generating
> +  // code for the initialization of globals. Do not record these decls
> +  // as they will overwrite the actual VarDecl Decl in the cache.
> +  if (HasDecl && isa<FunctionDecl>(D))
>      DeclCache.insert(std::make_pair(D->getCanonicalDecl(), llvm::WeakVH(SP)));
> 
>    // Push the function onto the lexical block stack.
> 
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp?rev=221385&r1=221384&r2=221385&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp?rev=221385&r1=221384&r2=221385&view=diff>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp Wed Nov  5 13:19:04 2014
> @@ -5,8 +5,8 @@
>  namespace A {
>  #line 1 "foo.cpp"
>  namespace B {
> -int i;
> -void f1() { }
> +extern int i;
> +int f1() { return 0; }
>  void f1(int) { }
>  struct foo;
>  struct bar { };
> @@ -19,7 +19,7 @@ using namespace B;
> 
>  using namespace A;
>  namespace E = A;
> -
> +int B::i = f1();
>  int func(bool b) {
>    if (b) {
>      using namespace A::B;
> @@ -51,8 +51,8 @@ using B::i;
>  // CHECK: [[FILE]] {{.*}}debug-info-namespace.cpp"
>  // CHECK: [[BAR:![0-9]*]] {{.*}} ; [ DW_TAG_structure_type ] [bar] [line 6, {{.*}}] [decl] [from ]
>  // CHECK: [[F1:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] [line 4] [def] [f1]
> -// CHECK: [[FUNC:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func]
>  // CHECK: [[FILE2]]} ; [ DW_TAG_file_type ] [{{.*}}foo.cpp]
> +// CHECK: [[FUNC:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func]
> 
> Is there a more specific observable change we should be testing here? I assume if this produced a bug, it would be actually visible in some way not just "metadat order changed slightly".
> 

You misunderstood the change in the testcase. The metadata order change is a side effect from the patch, but it’s not this part that tests the change. (This kinda random order is a pain BTW. While I was investigating various fixes for that issue, I had to dig into quite a few failing test cases just to find out that things had only been reordered)

I changed the global variable B::i to need an initializer function. If you run the version of the testcase I committed through a clang that hasn’t the patch, you’ll see that the import directive will point to the a global init function rather than to the variable. The real test performed wrt the patch is that the import directive really points to the variable and this test was already there and thus doesn’t appear in the patch.

Fred

>  // CHECK: [[I:![0-9]*]] = metadata !{metadata !"0x34\00i\00{{.*}}", metadata [[NS]], {{.*}} ; [ DW_TAG_variable ] [i]
>  // CHECK: [[MODULES]] = metadata !{metadata [[M1:![0-9]*]], metadata [[M2:![0-9]*]], metadata [[M3:![0-9]*]], metadata [[M4:![0-9]*]], metadata [[M5:![0-9]*]], metadata [[M6:![0-9]*]], metadata [[M7:![0-9]*]], metadata [[M8:![0-9]*]], metadata [[M9:![0-9]*]], metadata [[M10:![0-9]*]], metadata [[M11:![0-9]*]], metadata [[M12:![0-9]*]], metadata [[M13:![0-9]*]]}
>  // CHECK: [[M1]] = metadata !{metadata !"0x3a\0011\00", metadata [[CTXT]], metadata [[NS]]} ; [ DW_TAG_imported_module ]
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141105/eff2c4fa/attachment.html>