Fixed debug info generation for function static variables, typedef, and records

[aaboud]

Summary

A Fix for Bug 19238.
Function static variable defined inside a lexical scope, same as typedef and record (class, struct or union), which are declared inside a lexical scope, were all associated to the function as its parent scope, rather than to the lexical scope they are defined or declared inside.

This patch provides a fix for all these three cases in the Clang debug info generation.

This patch is related to "D9758"

[probinson]

The original bug had an example where the inner block has a second static variable, with the same name as the static variable in the outer block. With a different type, to make it more interesting.
It would be nice to have a test like that here.

[aaboud]

I did not get feedback on the fix, yet.
Can you please approve or comment if you disagree on this code change?

[probinson]

lib/CodeGen/CGDebugInfo.cpp

782 ↗

(On Diff #25722)

This could be refactored into a 'getDeclarationLexicalScope' method, as the companion to 'recordDeclarationLexicalScope'.

2228 ↗

(On Diff #25722)

This would become a call to 'getDeclarationLexicalScope'.

[probinson]

In D9760#176193, @aaboud wrote:

I did not get feedback on the fix, yet.

Actually I had a previous comment on testing.

[aaboud]

Made some changes to follow Paul's review:
Added another test for static variables, based on the example from Bug 19238.
Moved some common code to a new helper function.

[aaboud]

Applied changes according to David Blaikie commits.
Now, records and typedef entries that are defined inside a lexical block are added to the retained types list.

[aaboud]

Updated the LLVM version to 240902 (close to top of trunk preparing for commit).

[dblaikie]

Do you need to resolve the Context in the retained types iteration? If it's
a lexical block it shouldn't ever be a scope ref, I think? Not sure.

(mostly I'm just concerned aobut the inconsistency between that case and
the global variable case above)

In DwarfCompileUnit - perhaps:

auto LocalDeclNodeRangeForScope = DD->findLocalDeclNodesForScope(DS);

could be changed to:

iterator_range<LocalDeclMap::const_iterator> LocalDeclNodeRangeForScope;
if (!includeMinimalInlineScopes())
  LocalDeclNodeRangeForScope = DD->findLocalDeclNodesForScope(DS);

Then you can drop the includeMinimalInlineScopes() condition from the
following if and the if around the for loop furtehr down, I think? (with
the caveat that I'm not sure if iterator_range is guaranteed (or can be
guaranteed) to produce an empty range - not sure if two default constructed
iterators are guaranteed/meant to be equal to each other - could check the
spec, etc)

• David

[aaboud]

Hi David,

The reason I resolve the context for the retained type while I do not for the global variable, is because the “getScope” of the DIType returns a DIScopeRef, while the “getScope” of the DIGlobalVariable returns a DIScope.
I have no idea what are the reasons for this difference, but if we decide to unify the return value of the getScope, it is a change for another patch.

I will do the changes you suggested in DwarfCompileUnit and upload a new version.

Regards,
Amjad

From: David Blaikie [mailto:[e-mail]]
Sent: Monday, June 29, 2015 19:45
To: [e-mail]
Cc: Aboud, Amjad; Eric Christopher; Alexey Bataev; Robinson, Paul; llvm cfe
Subject: Re: [PATCH] Fixed debug info generation for function static variables, typedef, and records

Do you need to resolve the Context in the retained types iteration? If it's a lexical block it shouldn't ever be a scope ref, I think? Not sure.

(mostly I'm just concerned aobut the inconsistency between that case and the global variable case above)

In DwarfCompileUnit - perhaps:

auto LocalDeclNodeRangeForScope = DD->findLocalDeclNodesForScope(DS);

could be changed to:

iterator_range<LocalDeclMap::const_iterator> LocalDeclNodeRangeForScope;
if (!includeMinimalInlineScopes())
  LocalDeclNodeRangeForScope = DD->findLocalDeclNodesForScope(DS);

Then you can drop the includeMinimalInlineScopes() condition from the following if and the if around the for loop furtehr down, I think? (with the caveat that I'm not sure if iterator_range is guaranteed (or can be guaranteed) to produce an empty range - not sure if two default constructed iterators are guaranteed/meant to be equal to each other - could check the spec, etc)

• David

cfe/trunk/lib/CodeGen/CGDebugInfo.h

class CGDebugInfo {
 
   /// \brief Cache of replaceable forward declarartions (functions and
   /// variables) to RAUW at the end of compilation.
   std::vector<std::pair<const DeclaratorDecl *, llvm::TrackingMDRef>>
       FwdDeclReplaceMap;
 
   // LexicalBlockStack - Keep track of our current nested lexical block.
   std::vector<llvm::TypedTrackingMDRef<llvm::DIScope>> LexicalBlockStack;
+
+  /// \brief Map of AST declaration to its lexical block scope.
+  llvm::DenseMap<const Decl *, llvm::TypedTrackingMDRef<llvm::DIScope>>
+      LexicalBlockMap;
+
   llvm::DenseMap<const Decl *, llvm::TrackingMDRef> RegionMap;
   // FnBeginRegionCount - Keep track of LexicalBlockStack counter at the
   // beginning of a function. This is used to pop unbalanced regions at
   // the end of a function.
   std::vector<unsigned> FnBeginRegionCount;
 
   /// \brief This is a storage for names that are
   /// constructed on demand. For example, C++ destructors, C++ operators etc..
public:
 
   /// \brief Emit record type's standalone debug info.
   llvm::DIType *getOrCreateRecordType(QualType Ty, SourceLocation L);
 
   /// \brief Emit an objective c interface type standalone
   /// debug info.
   llvm::DIType *getOrCreateInterfaceType(QualType Ty, SourceLocation Loc);
 
+  /// \brief Map AST declaration to its lexical block scope if available.
+  void recordDeclarationLexicalScope(const Decl &D);
+
+  /// \brief Get lexical scope of AST declaration.
+  llvm::DIScope *getDeclarationLexicalScope(const Decl &D, QualType Ty);
+
   void completeType(const EnumDecl *ED);
   void completeType(const RecordDecl *RD);
   void completeRequiredType(const RecordDecl *RD);
   void completeClassData(const RecordDecl *RD);
 
   void completeTemplateDefinition(const ClassTemplateSpecializationDecl &SD);
 
 private:

cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

llvm::DIType *CGDebugInfo::CreateType(const TemplateSpecializationType *Ty,
   SourceLocation Loc = AliasDecl->getLocation();
   return DBuilder.createTypedef(
       Src, internString(OS.str()), getOrCreateFile(Loc), getLineNumber(Loc),
       getContextDescriptor(cast<Decl>(AliasDecl->getDeclContext())));
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const TypedefType *Ty,
                                       llvm::DIFile *Unit) {
+  TypedefNameDecl *TD = Ty->getDecl();
   // We don't set size information, but do specify where the typedef was
   // declared.
-  SourceLocation Loc = Ty->getDecl()->getLocation();
+  SourceLocation Loc = TD->getLocation();
+
+  llvm::DIScope *TDContext = getDeclarationLexicalScope(*TD, QualType(Ty, 0));
 
   // Typedefs are derived from some other type.
   return DBuilder.createTypedef(
       getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit),
       Ty->getDecl()->getName(), getOrCreateFile(Loc), getLineNumber(Loc),
-      getContextDescriptor(cast<Decl>(Ty->getDecl()->getDeclContext())));
+      TDContext);
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const FunctionType *Ty,
                                       llvm::DIFile *Unit) {
   SmallVector<llvm::Metadata *, 16> EltTys;
 
   // Add the result type at least.
   EltTys.push_back(getOrCreateType(Ty->getReturnType(), Unit));

 llvm::DIType *CGDebugInfo::getOrCreateInterfaceType(QualType D,
                                                     SourceLocation Loc) {
   assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
   llvm::DIType *T = getOrCreateType(D, getOrCreateFile(Loc));
   RetainedTypes.push_back(D.getAsOpaquePtr());
   return T;
 }
 
+void CGDebugInfo::recordDeclarationLexicalScope(const Decl &D) {
+  assert(LexicalBlockMap.find(&D) == LexicalBlockMap.end() &&
+         "D is already mapped to lexical block scope");
+  if (!LexicalBlockStack.empty())
+    LexicalBlockMap[&D] = LexicalBlockStack.back();
+}
+
+llvm::DIScope *CGDebugInfo::getDeclarationLexicalScope(const Decl &D,
+                                                       QualType Ty) {
+  auto I = LexicalBlockMap.find(&D);
+  if (I != LexicalBlockMap.end()) {
+    RetainedTypes.push_back(Ty.getAsOpaquePtr());
+    return I->second;
+  } else
+    return getContextDescriptor(cast<Decl>(D.getDeclContext()));
+}
+
 void CGDebugInfo::completeType(const EnumDecl *ED) {
   if (DebugKind <= CodeGenOptions::DebugLineTablesOnly)
     return;
   QualType Ty = CGM.getContext().getEnumType(ED);
   void *TyPtr = Ty.getAsOpaquePtr();
   auto I = TypeCache.find(TyPtr);
   if (I == TypeCache.end() || !cast<llvm::DIType>(I->second)->isForwardDecl())
     return;

 llvm::DICompositeType *CGDebugInfo::CreateLimitedType(const RecordType *Ty) {
   RecordDecl *RD = Ty->getDecl();
 
   // Get overall information about the record type for the debug info.
   llvm::DIFile *DefUnit = getOrCreateFile(RD->getLocation());
   unsigned Line = getLineNumber(RD->getLocation());
   StringRef RDName = getClassName(RD);
 
-  llvm::DIScope *RDContext =
-      getContextDescriptor(cast<Decl>(RD->getDeclContext()));
+  llvm::DIScope *RDContext = getDeclarationLexicalScope(*RD, QualType(Ty, 0));
 
   // If we ended up creating the type during the context chain construction,
   // just return that.
   auto *T = cast_or_null<llvm::DICompositeType>(
       getTypeOrNull(CGM.getContext().getRecordType(RD)));
   if (T && (!T->isForwardDecl() || !RD->getDefinition()))
     return T;
 
void CGDebugInfo::collectVarDeclProps(const VarDecl *VD, llvm::DIFile *&Unit,
   // When a record type contains an in-line initialization of a static data
   // member, and the record type is marked as __declspec(dllexport), an implicit
   // definition of the member will be created in the record context.  DWARF
   // doesn't seem to have a nice way to describe this in a form that consumers
   // are likely to understand, so fake the "normal" situation of a definition
   // outside the class by putting it in the global scope.
   if (DC->isRecord())
     DC = CGM.getContext().getTranslationUnitDecl();
+
+  if (VD->isStaticLocal()) {
+    // Get context for static locals (that are technically globals) the same way
+    // we do for "local" locals -- by using current lexical block.
+    assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+    VDContext = LexicalBlockStack.back();
+  } else
     VDContext = getContextDescriptor(dyn_cast<Decl>(DC));
 }
 
 llvm::DISubprogram *
 CGDebugInfo::getFunctionForwardDeclaration(const FunctionDecl *FD) {
   llvm::DINodeArray TParamsArray;
   StringRef Name, LinkageName;
   unsigned Flags = 0;
   SourceLocation Loc = FD->getLocation();

cfe/trunk/lib/CodeGen/CGDecl.cpp

void CodeGenFunction::EmitDecl(const Decl &D) {
   case Decl::Friend:
   case Decl::FriendTemplate:
   case Decl::Block:
   case Decl::Captured:
   case Decl::ClassScopeFunctionSpecialization:
   case Decl::UsingShadow:
     llvm_unreachable("Declaration should not be in declstmts!");
   case Decl::Function:  // void X();
-  case Decl::Record:    // struct/union/class X;
   case Decl::Enum:      // enum X;
   case Decl::EnumConstant: // enum ? { X = ? }
-  case Decl::CXXRecord: // struct/union/class X; [C++]
   case Decl::StaticAssert: // static_assert(X, ""); [C++0x]
   case Decl::Label:        // __label__ x;
   case Decl::Import:
   case Decl::OMPThreadPrivate:
   case Decl::Empty:
     // None of these decls require codegen support.
     return;
 
+  case Decl::CXXRecord: // struct/union/class X; [C++]
+  case Decl::Record:    // struct/union/class X;
+    if (CGDebugInfo *DI = getDebugInfo())
+      DI->recordDeclarationLexicalScope(D);
+    return;
+
   case Decl::NamespaceAlias:
     if (CGDebugInfo *DI = getDebugInfo())
         DI->EmitNamespaceAlias(cast<NamespaceAliasDecl>(D));
     return;
   case Decl::Using:          // using X; [C++]
     if (CGDebugInfo *DI = getDebugInfo())
         DI->EmitUsingDecl(cast<UsingDecl>(D));
     return;
   case Decl::UsingDirective: // using namespace X; [C++]
     if (CGDebugInfo *DI = getDebugInfo())
       DI->EmitUsingDirective(cast<UsingDirectiveDecl>(D));
     return;
   case Decl::Var: {
     const VarDecl &VD = cast<VarDecl>(D);
     assert(VD.isLocalVarDecl() &&
            "Should not see file-scope variables inside a function!");
     return EmitVarDecl(VD);
   }
 
   case Decl::Typedef:      // typedef int X;
   case Decl::TypeAlias: {  // using X = int; [C++0x]
     const TypedefNameDecl &TD = cast<TypedefNameDecl>(D);
     QualType Ty = TD.getUnderlyingType();
 
+    if (CGDebugInfo *DI = getDebugInfo())
+      DI->recordDeclarationLexicalScope(D);
+
     if (Ty->isVariablyModifiedType())
       EmitVariablyModifiedType(Ty);
   }
   }
 }
 
 /// EmitVarDecl - This method handles emission of any variable declaration
 /// inside a function, including static vars etc.

cfe/trunk/test/CodeGenCXX/debug-info-lb-class.cpp

+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -g %s -o - | FileCheck %s
+
+int foo(int x) {
+  if(x)
+  {
+    class X {
+    public:
+      char z;
+      X(int y) : z(y) {}
+    };
+    {
+      X a(x);
+      return a.z;
+    }
+  }
+  return 0;
+}
+
+// CHECK: !{{[0-9]+}} = !DICompositeType(tag: DW_TAG_class_type, name: "X", scope: [[LBScope:![0-9]+]],
+// CHECK: [[LBScope]] = distinct !DILexicalBlock(scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 5)
+

cfe/trunk/test/CodeGenCXX/debug-info-lb-static.cpp

+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -g %s -o - | FileCheck %s
+
+int foo(int x) {
+  if(x)
+  {
+    static int bar = 0;
+    {
+      int a = bar++;
+      return a;
+    }
+  }
+  return 0;
+}
+
+// CHECK: !{{[0-9]+}} = !DIGlobalVariable(name: "bar", scope: [[LBScope:![0-9]+]],
+// CHECK: [[LBScope]] = distinct !DILexicalBlock(scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 5)
+

cfe/trunk/test/CodeGenCXX/debug-info-lb-static2.cpp

+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -g %s -o - | FileCheck %s
+
+int main() {
+  static int X = 10;
+  {
+    static bool X = false;
+    return (int) X;
+  }
+  return X;
+}
+
+// CHECK: [[FuncScope:![0-9]+]] = !DISubprogram(name: "main",
+// CHECK: !{{[0-9]+}} = !DIGlobalVariable(name: "X", scope: [[FuncScope:![0-9]+]],
+// CHECK: !{{[0-9]+}} = !DIGlobalVariable(name: "X", scope: [[LBScope:![0-9]+]],
+// CHECK: [[LBScope]] = distinct !DILexicalBlock(scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 5)
+

cfe/trunk/test/CodeGenCXX/debug-info-lb-typedef.cpp

+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -g %s -o - | FileCheck %s
+
+int foo(int x) {
+  if(x) 
+  {
+    typedef char X;
+    {
+      X a = x;
+      return a;
+    }
+  }
+  return 0;
+}
+
+// CHECK: !{{[0-9]+}} = !DIDerivedType(tag: DW_TAG_typedef, name: "X", scope: [[LBScope:![0-9]+]],
+// CHECK: [[LBScope]] = distinct !DILexicalBlock(scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 5)
+