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

[David Blaikie]

On Wed, Nov 5, 2014 at 1:47 PM, Frédéric Riss <friss at apple.com> wrote:

>
> 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> 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
>> 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
>>
>> ==============================================================================
>> --- 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
>>
>> ==============================================================================
>> --- 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.
>

Right right - thanks!


>
> 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
>> 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/685afc26/attachment.html>