From e3d8ee35e4adca664a9149536e0f0b3b0ceaeaeb Mon Sep 17 00:00:00 2001 From: Yonghong Song Date: Fri, 22 Nov 2019 08:45:37 -0800 Subject: [PATCH] reland "[DebugInfo] Support to emit debugInfo for extern variables" Commit d77ae1552fc21a9f3877f3ed7e13d631f517c825 ("[DebugInfo] Support to emit debugInfo for extern variables") added deebugInfo for extern variables for BPF target. The commit is reverted by 891e25b02d760d0de18c7d46947913b3166047e7 as the committed tests using %clang instead of %clang_cc1 causing test failed in certain scenarios as reported by Reid Kleckner. This patch fixed the tests by using %clang_cc1. Differential Revision: https://reviews.llvm.org/D71818 --- clang/include/clang/AST/ASTConsumer.h | 5 +++++ clang/include/clang/Basic/TargetInfo.h | 3 +++ clang/include/clang/Sema/Sema.h | 3 +++ clang/lib/Basic/Targets/BPF.h | 2 ++ clang/lib/CodeGen/CGDebugInfo.cpp | 23 +++++++++++++++++++-- clang/lib/CodeGen/CGDebugInfo.h | 3 +++ clang/lib/CodeGen/CodeGenAction.cpp | 4 ++++ clang/lib/CodeGen/CodeGenModule.cpp | 17 ++++++++++++++++ clang/lib/CodeGen/CodeGenModule.h | 3 +++ clang/lib/CodeGen/ModuleBuilder.cpp | 4 ++++ clang/lib/Sema/Sema.cpp | 7 +++++++ clang/lib/Sema/SemaDecl.cpp | 4 ++++ clang/test/CodeGen/debug-info-extern-basic.c | 26 ++++++++++++++++++++++++ clang/test/CodeGen/debug-info-extern-duplicate.c | 10 +++++++++ clang/test/CodeGen/debug-info-extern-multi.c | 22 ++++++++++++++++++++ clang/test/CodeGen/debug-info-extern-unused.c | 26 ++++++++++++++++++++++++ llvm/include/llvm/IR/DIBuilder.h | 2 +- llvm/lib/IR/DIBuilder.cpp | 5 +++-- llvm/lib/IR/DebugInfo.cpp | 2 +- llvm/unittests/Transforms/Utils/CloningTest.cpp | 2 +- 20 files changed, 166 insertions(+), 7 deletions(-) create mode 100644 clang/test/CodeGen/debug-info-extern-basic.c create mode 100644 clang/test/CodeGen/debug-info-extern-duplicate.c create mode 100644 clang/test/CodeGen/debug-info-extern-multi.c create mode 100644 clang/test/CodeGen/debug-info-extern-unused.c diff --git a/clang/include/clang/AST/ASTConsumer.h b/clang/include/clang/AST/ASTConsumer.h index dc216a8..ecdd8e8 100644 --- a/clang/include/clang/AST/ASTConsumer.h +++ b/clang/include/clang/AST/ASTConsumer.h @@ -102,6 +102,11 @@ public: /// modified by the introduction of an implicit zero initializer. virtual void CompleteTentativeDefinition(VarDecl *D) {} + /// CompleteExternalDeclaration - Callback invoked at the end of a translation + /// unit to notify the consumer that the given external declaration should be + /// completed. + virtual void CompleteExternalDeclaration(VarDecl *D) {} + /// Callback invoked when an MSInheritanceAttr has been attached to a /// CXXRecordDecl. virtual void AssignInheritanceModel(CXXRecordDecl *RD) {} diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 33cecda..bc06f59 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1389,6 +1389,9 @@ public: virtual void setAuxTarget(const TargetInfo *Aux) {} + /// Whether target allows debuginfo types for decl only variables. + virtual bool allowDebugInfoForExternalVar() const { return false; } + protected: /// Copy type and layout related info. void copyAuxTarget(const TargetInfo *Aux); diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8cce6fd..41c0e14 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -668,6 +668,9 @@ public: /// All the tentative definitions encountered in the TU. TentativeDefinitionsType TentativeDefinitions; + /// All the external declarations encoutered and used in the TU. + SmallVector ExternalDeclarations; + typedef LazyVector UnusedFileScopedDeclsType; diff --git a/clang/lib/Basic/Targets/BPF.h b/clang/lib/Basic/Targets/BPF.h index 117f814..b2f1831 100644 --- a/clang/lib/Basic/Targets/BPF.h +++ b/clang/lib/Basic/Targets/BPF.h @@ -76,6 +76,8 @@ public: return None; } + bool allowDebugInfoForExternalVar() const override { return true; } + CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { switch (CC) { default: diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 8858e08..675df30 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4485,7 +4485,7 @@ void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var, GVE = DBuilder.createGlobalVariableExpression( DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit), - Var->hasLocalLinkage(), + Var->hasLocalLinkage(), true, Expr.empty() ? nullptr : DBuilder.createExpression(Expr), getOrCreateStaticDataMemberDeclarationOrNull(D), TemplateParameters, Align); @@ -4588,10 +4588,29 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) { GV.reset(DBuilder.createGlobalVariableExpression( DContext, Name, StringRef(), Unit, getLineNumber(VD->getLocation()), Ty, - true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD), + true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD), TemplateParameters, Align)); } +void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var, + const VarDecl *D) { + assert(DebugKind >= codegenoptions::LimitedDebugInfo); + if (D->hasAttr()) + return; + + auto Align = getDeclAlignIfRequired(D, CGM.getContext()); + llvm::DIFile *Unit = getOrCreateFile(D->getLocation()); + StringRef Name = D->getName(); + llvm::DIType *Ty = getOrCreateType(D->getType(), Unit); + + llvm::DIScope *DContext = getDeclContextDescriptor(D); + llvm::DIGlobalVariableExpression *GVE = + DBuilder.createGlobalVariableExpression( + DContext, Name, StringRef(), Unit, getLineNumber(D->getLocation()), + Ty, false, false, nullptr, nullptr, nullptr, Align); + Var->addDebugInfo(GVE); +} + llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) { if (!LexicalBlockStack.empty()) return LexicalBlockStack.back(); diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index fed79f0..90e9a61 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -478,6 +478,9 @@ public: /// Emit a constant global variable's debug info. void EmitGlobalVariable(const ValueDecl *VD, const APValue &Init); + /// Emit information about an external variable. + void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl); + /// Emit C++ using directive. void EmitUsingDirective(const UsingDirectiveDecl &UD); diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 9552149..7f3f358 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -336,6 +336,10 @@ namespace clang { Gen->CompleteTentativeDefinition(D); } + void CompleteExternalDeclaration(VarDecl *D) override { + Gen->CompleteExternalDeclaration(D); + } + void AssignInheritanceModel(CXXRecordDecl *RD) override { Gen->AssignInheritanceModel(RD); } diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 02b2dd3..1fc2beb 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -3730,6 +3730,10 @@ void CodeGenModule::EmitTentativeDefinition(const VarDecl *D) { EmitGlobalVarDefinition(D); } +void CodeGenModule::EmitExternalDeclaration(const VarDecl *D) { + EmitExternalVarDeclaration(D); +} + CharUnits CodeGenModule::GetTargetTypeStoreSize(llvm::Type *Ty) const { return Context.toCharUnitsFromBits( getDataLayout().getTypeStoreSizeInBits(Ty)); @@ -4113,6 +4117,19 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, DI->EmitGlobalVariable(GV, D); } +void CodeGenModule::EmitExternalVarDeclaration(const VarDecl *D) { + if (CGDebugInfo *DI = getModuleDebugInfo()) + if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo) { + QualType ASTTy = D->getType(); + llvm::Type *Ty = getTypes().ConvertTypeForMem(D->getType()); + llvm::PointerType *PTy = + llvm::PointerType::get(Ty, getContext().getTargetAddressSpace(ASTTy)); + llvm::Constant *GV = GetOrCreateLLVMGlobal(D->getName(), PTy, D); + DI->EmitExternalVariable( + cast(GV->stripPointerCasts()), D); + } +} + static bool isVarDeclStrongDefinition(const ASTContext &Context, CodeGenModule &CGM, const VarDecl *D, bool NoCommon) { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 673eda3..9bf1c5e 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1170,6 +1170,8 @@ public: void EmitTentativeDefinition(const VarDecl *D); + void EmitExternalDeclaration(const VarDecl *D); + void EmitVTable(CXXRecordDecl *Class); void RefreshTypeCacheForClass(const CXXRecordDecl *Class); @@ -1405,6 +1407,7 @@ private: void EmitMultiVersionFunctionDefinition(GlobalDecl GD, llvm::GlobalValue *GV); void EmitGlobalVarDefinition(const VarDecl *D, bool IsTentative = false); + void EmitExternalVarDeclaration(const VarDecl *D); void EmitAliasDefinition(GlobalDecl GD); void emitIFuncDefinition(GlobalDecl GD); void emitCPUDispatchDefinition(GlobalDecl GD); diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp index 4154f6e..01093cf 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -290,6 +290,10 @@ namespace { Builder->EmitTentativeDefinition(D); } + void CompleteExternalDeclaration(VarDecl *D) override { + Builder->EmitExternalDeclaration(D); + } + void HandleVTable(CXXRecordDecl *RD) override { if (Diags.hasErrorOccurred()) return; diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 85548cb..2cd158a 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1136,6 +1136,13 @@ void Sema::ActOnEndOfTranslationUnit() { Consumer.CompleteTentativeDefinition(VD); } + for (auto D : ExternalDeclarations) { + if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed()) + continue; + + Consumer.CompleteExternalDeclaration(D); + } + // If there were errors, disable 'unused' warnings since they will mostly be // noise. Don't warn for a use from a module: either we should warn on all // file-scope declarations in modules or not at all, but whether the diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f05a920..8f68be7 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -12220,6 +12220,10 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) { Diag(Var->getLocation(), diag::note_private_extern); } + if (Context.getTargetInfo().allowDebugInfoForExternalVar() && + !Var->isInvalidDecl() && !getLangOpts().CPlusPlus) + ExternalDeclarations.push_back(Var); + return; case VarDecl::TentativeDefinition: diff --git a/clang/test/CodeGen/debug-info-extern-basic.c b/clang/test/CodeGen/debug-info-extern-basic.c new file mode 100644 index 0000000..e2c9084 --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-basic.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s + +extern char ch; +int test() { + return ch; +} + +int test2() { + extern char ch2; + return ch2; +} + +extern int (*foo)(int); +int test3() { + return foo(0); +} + +// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[CHART:[0-9]+]], isLocal: false, isDefinition: false +// CHECK: distinct !DIGlobalVariable(name: "ch2",{{.*}} type: ![[CHART]], isLocal: false, isDefinition: false +// CHECK: ![[CHART]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) + +// CHECK: distinct !DIGlobalVariable(name: "foo",{{.*}} type: ![[FUNC:[0-9]+]], isLocal: false, isDefinition: false) +// CHECK: ![[FUNC]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[SUB:[0-9]+]], size: 64) +// CHECK: ![[SUB]] = !DISubroutineType(types: ![[TYPES:[0-9]+]]) +// CHECK: ![[TYPES]] = !{![[BASET:[0-9]+]], ![[BASET]]} +// CHECK: ![[BASET]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) diff --git a/clang/test/CodeGen/debug-info-extern-duplicate.c b/clang/test/CodeGen/debug-info-extern-duplicate.c new file mode 100644 index 0000000..1c6d86f --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-duplicate.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s + +extern char ch; +extern char ch; +int test() { + return ch; +} + +// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[T:[0-9]+]], isLocal: false, isDefinition: false +// CHECK-NOT: distinct !DIGlobalVariable(name: "ch" diff --git a/clang/test/CodeGen/debug-info-extern-multi.c b/clang/test/CodeGen/debug-info-extern-multi.c new file mode 100644 index 0000000..6a9021d --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-multi.c @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s + +extern char ch; +int test() { + extern short sh; + return ch + sh; +} + +extern char (*foo)(char); +int test2() { + return foo(0) + ch; +} + +// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[Tch:[0-9]+]], isLocal: false, isDefinition: false +// CHECK: distinct !DIGlobalVariable(name: "sh",{{.*}} type: ![[Tsh:[0-9]+]], isLocal: false, isDefinition: false +// CHECK: ![[Tsh]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed) + +// CHECK: distinct !DIGlobalVariable(name: "foo",{{.*}} type: ![[Tptr:[0-9]+]], isLocal: false, isDefinition: false +// ![[Tptr]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[Tsub:[0-9]+]], size: 64) +// ![[Tsub]] = !DISubroutineType(types: ![[Tproto:[0-9]+]]) +// ![[Tproto]] = !{![[Tch]], ![[Tch]]} +// CHECK: ![[Tch]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) diff --git a/clang/test/CodeGen/debug-info-extern-unused.c b/clang/test/CodeGen/debug-info-extern-unused.c new file mode 100644 index 0000000..8b89eaa --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-unused.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s + +extern char ch; +int test() { + return 0; +} + +int test2() { + extern char ch2; + return 0; +} + +extern int (*foo)(int); +int test3() { + return 0; +} + +int test4() { + extern int (*foo2)(int); + return 0; +} + +// CHECK-NOT: distinct !DIGlobalVariable(name: "ch" +// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2" +// CHECK-NOT: distinct !DIGlobalVariable(name: "foo" +// CHECK-NOT: distinct !DIGlobalVariable(name: "foo2" diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h index 48c0007..7c0c5f5 100644 --- a/llvm/include/llvm/IR/DIBuilder.h +++ b/llvm/include/llvm/IR/DIBuilder.h @@ -583,7 +583,7 @@ namespace llvm { /// specified) DIGlobalVariableExpression *createGlobalVariableExpression( DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File, - unsigned LineNo, DIType *Ty, bool isLocalToUnit, + unsigned LineNo, DIType *Ty, bool isLocalToUnit, bool isDefined = true, DIExpression *Expr = nullptr, MDNode *Decl = nullptr, MDTuple *templateParams = nullptr, uint32_t AlignInBits = 0); diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp index d918551..9f5811d 100644 --- a/llvm/lib/IR/DIBuilder.cpp +++ b/llvm/lib/IR/DIBuilder.cpp @@ -640,13 +640,14 @@ static void checkGlobalVariableScope(DIScope *Context) { DIGlobalVariableExpression *DIBuilder::createGlobalVariableExpression( DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *F, - unsigned LineNumber, DIType *Ty, bool isLocalToUnit, DIExpression *Expr, + unsigned LineNumber, DIType *Ty, bool isLocalToUnit, + bool isDefined, DIExpression *Expr, MDNode *Decl, MDTuple *templateParams, uint32_t AlignInBits) { checkGlobalVariableScope(Context); auto *GV = DIGlobalVariable::getDistinct( VMContext, cast_or_null(Context), Name, LinkageName, F, - LineNumber, Ty, isLocalToUnit, true, cast_or_null(Decl), + LineNumber, Ty, isLocalToUnit, isDefined, cast_or_null(Decl), templateParams, AlignInBits); if (!Expr) Expr = createExpression(); diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp index 5bbd292..fe83119 100644 --- a/llvm/lib/IR/DebugInfo.cpp +++ b/llvm/lib/IR/DebugInfo.cpp @@ -1289,7 +1289,7 @@ LLVMMetadataRef LLVMDIBuilderCreateGlobalVariableExpression( return wrap(unwrap(Builder)->createGlobalVariableExpression( unwrapDI(Scope), {Name, NameLen}, {Linkage, LinkLen}, unwrapDI(File), LineNo, unwrapDI(Ty), LocalToUnit, - unwrap(Expr), unwrapDI(Decl), + true, unwrap(Expr), unwrapDI(Decl), nullptr, AlignInBits)); } diff --git a/llvm/unittests/Transforms/Utils/CloningTest.cpp b/llvm/unittests/Transforms/Utils/CloningTest.cpp index 3d0bd10..28ad4bc 100644 --- a/llvm/unittests/Transforms/Utils/CloningTest.cpp +++ b/llvm/unittests/Transforms/Utils/CloningTest.cpp @@ -764,7 +764,7 @@ protected: DBuilder.createGlobalVariableExpression( Subprogram, "unattached", "unattached", File, 1, - DBuilder.createNullPtrType(), false, Expr); + DBuilder.createNullPtrType(), false, true, Expr); auto *Entry = BasicBlock::Create(C, "", F); IBuilder.SetInsertPoint(Entry); -- 2.7.4