From d77ae1552fc21a9f3877f3ed7e13d631f517c825 Mon Sep 17 00:00:00 2001 From: Yonghong Song Date: Fri, 22 Nov 2019 08:45:37 -0800 Subject: [PATCH] [DebugInfo] Support to emit debugInfo for extern variables Extern variable usage in BPF is different from traditional pure user space application. Recent discussion in linux bpf mailing list has two use cases where debug info types are required to use extern variables: - extern types are required to have a suitable interface in libbpf (bpf loader) to provide kernel config parameters to bpf programs. https://lore.kernel.org/bpf/CAEf4BzYCNo5GeVGMhp3fhysQ=_axAf=23PtwaZs-yAyafmXC9g@mail.gmail.com/T/#t - extern types are required so kernel bpf verifier can verify program which uses external functions more precisely. This will make later link with actual external function no need to reverify. https://lore.kernel.org/bpf/87eez4odqp.fsf@toke.dk/T/#m8d5c3e87ffe7f2764e02d722cb0d8cbc136880ed This patch added clang support to emit debuginfo for extern variables with a TargetInfo hook to enable it. The debuginfo for the extern variable is emitted only if that extern variable is referenced in the current compilation unit. Currently, only BPF target enables to generate debug info for extern variables. The emission of such debuginfo is disabled for C++ at this moment since BPF only supports a subset of C language. Emission with C++ can be enabled later if an appropriate use case is identified. -fstandalone-debug permits us to see more debuginfo with the cost of bloated binary size. This patch did not add emission of extern variable debug info with -fstandalone-debug. This can be re-evaluated if there is a real need. Differential Revision: https://reviews.llvm.org/D70696 --- 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 | 27 ++++++++++++++++++++++++ clang/test/CodeGen/debug-info-extern-duplicate.c | 11 ++++++++++ clang/test/CodeGen/debug-info-extern-multi.c | 23 ++++++++++++++++++++ clang/test/CodeGen/debug-info-extern-unused.c | 27 ++++++++++++++++++++++++ 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, 170 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 5633f95..1cdaab3 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -667,6 +667,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 e08945f..06204a8 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4482,7 +4482,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); @@ -4585,10 +4585,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 867a893..1fa1f62 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 d7146b1..d3d3df8 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -3715,6 +3715,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)); @@ -4098,6 +4102,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 d0832fb..8fd8cbc 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1165,6 +1165,8 @@ public: void EmitTentativeDefinition(const VarDecl *D); + void EmitExternalDeclaration(const VarDecl *D); + void EmitVTable(CXXRecordDecl *Class); void RefreshTypeCacheForClass(const CXXRecordDecl *Class); @@ -1400,6 +1402,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 5ade6bb..8d59729 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1137,6 +1137,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 6fb11cf..76bf7b0 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -12200,6 +12200,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..fab177d --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-basic.c @@ -0,0 +1,27 @@ +// REQUIRES: bpf-registered-target +// RUN: %clang -target bpf -emit-llvm -S -g %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..e3560af --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-duplicate.c @@ -0,0 +1,11 @@ +// REQUIRES: bpf-registered-target +// RUN: %clang -target bpf -emit-llvm -S -g %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..4f9afca --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-multi.c @@ -0,0 +1,23 @@ +// REQUIRES: bpf-registered-target +// RUN: %clang -target bpf -emit-llvm -S -g %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..11de955 --- /dev/null +++ b/clang/test/CodeGen/debug-info-extern-unused.c @@ -0,0 +1,27 @@ +// REQUIRES: bpf-registered-target +// RUN: %clang -target bpf -emit-llvm -S -g %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 7ea0c95..9df1ec9 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 bdd9f6b..e167554 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 62bfeb5..4707535 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