Author: Felipe de Azevedo Piovezan Date: 2023-02-20T14:22:49-05:00 New Revision: 997dc7e00f49663b60a78e18df1dfecdf62a4172
URL: https://github.com/llvm/llvm-project/commit/997dc7e00f49663b60a78e18df1dfecdf62a4172 DIFF: https://github.com/llvm/llvm-project/commit/997dc7e00f49663b60a78e18df1dfecdf62a4172.diff LOG: [debug-info][codegen] Prevent creation of self-referential SP node The function `CGDebugInfo::EmitFunctionDecl` is supposed to create a declaration -- never a _definition_ -- of a subprogram. This is made evident by the fact that the SPFlags never have the "Declaration" bit set by that function. However, when `EmitFunctionDecl` calls `DIBuilder::createFunction`, it still tries to fill the "Declaration" argument by passing it the result of `getFunctionDeclaration(D)`. This will query an internal cache of previously created declarations and, for most code paths, we return nullptr; all is good. However, as reported in [0], there are pathological cases in which we attempt to recreate a declaration, so the cache query succeeds, resulting in a subprogram declaration whose declaration field points to another declaration. Through a series of RAUWs, the declaration field ends up pointing to the SP itself. Self-referential MDNodes can't be `unique`, which causes the verifier to fail (declarations must be `unique`). We can argue that the caller should check the cache first, but this is not a correctness issue (declarations are `unique` anyway). The bug is that `CGDebugInfo::EmitFunctionDecl` should always pass `nullptr` to the declaration argument of `DIBuilder::createFunction`, expressing the fact that declarations don't point to other declarations. AFAICT this is not something for which any reasonable meaning exists. This seems a lot like a copy-paste mistake that has survived for ~10 years, since other places in this file have the exact same call almost token-by-token. I've tested this by compiling LLVMSupport with and without the patch, O2 and O0, and comparing the dwarfdump of the lib. The dumps are identical modulo the attributes decl_file/producer/comp_dir. [0]: https://github.com/llvm/llvm-project/issues/59241 Differential Revision: https://reviews.llvm.org/D143921 Added: llvm/test/Verifier/disubprogram_declaration.ll Modified: clang/lib/CodeGen/CGDebugInfo.cpp llvm/docs/LangRef.rst llvm/lib/IR/Verifier.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index bb91b5116d71b..4dab595ada76b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4225,10 +4225,9 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, SourceLocation Loc, llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D); llvm::DISubroutineType *STy = getOrCreateFunctionType(D, FnType, Unit); - llvm::DISubprogram *SP = - DBuilder.createFunction(FDContext, Name, LinkageName, Unit, LineNo, STy, - ScopeLine, Flags, SPFlags, TParamsArray.get(), - getFunctionDeclaration(D), nullptr, Annotations); + llvm::DISubprogram *SP = DBuilder.createFunction( + FDContext, Name, LinkageName, Unit, LineNo, STy, ScopeLine, Flags, + SPFlags, TParamsArray.get(), nullptr, nullptr, Annotations); // Preserve btf_decl_tag attributes for parameters of extern functions // for BPF target. The parameters created in this loop are attached as diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 4052e58faa7fb..b4c9cff4821cc 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -5776,11 +5776,12 @@ retained, even if their IR counterparts are optimized out of the IR. The .. _DISubprogramDeclaration: -When ``isDefinition: false``, subprograms describe a declaration in the type -tree as opposed to a definition of a function. If the scope is a composite -type with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, -then the subprogram declaration is uniqued based only on its ``linkageName:`` -and ``scope:``. +When ``spFlags: DISPFlagDefinition`` is not present, subprograms describe a +declaration in the type tree as opposed to a definition of a function. In this +case, the ``declaration`` field must be empty. If the scope is a composite type +with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, then +the subprogram declaration is uniqued based only on its ``linkageName:`` and +``scope:``. .. code-block:: text @@ -5789,9 +5790,9 @@ and ``scope:``. } !0 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1, - file: !2, line: 7, type: !3, isLocal: true, - isDefinition: true, scopeLine: 8, - containingType: !4, + file: !2, line: 7, type: !3, + spFlags: DISPFlagDefinition | DISPFlagLocalToUnit, + scopeLine: 8, containingType: !4, virtuality: DW_VIRTUALITY_pure_virtual, virtualIndex: 10, flags: DIFlagPrototyped, isOptimized: true, unit: !5, templateParams: !6, diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 40968a262811c..97ab9d2dfd663 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -1400,6 +1400,8 @@ void Verifier::visitDISubprogram(const DISubprogram &N) { } else { // Subprogram declarations (part of the type hierarchy). CheckDI(!Unit, "subprogram declarations must not have a compile unit", &N); + CheckDI(!N.getRawDeclaration(), + "subprogram declaration must not have a declaration field"); } if (auto *RawThrownTypes = N.getRawThrownTypes()) { diff --git a/llvm/test/Verifier/disubprogram_declaration.ll b/llvm/test/Verifier/disubprogram_declaration.ll new file mode 100644 index 0000000000000..e3402c9970866 --- /dev/null +++ b/llvm/test/Verifier/disubprogram_declaration.ll @@ -0,0 +1,17 @@ +; RUN: llvm-as -disable-output <%s 2>&1| FileCheck %s + +declare !dbg !12 i32 @declared_only() + +!llvm.module.flags = !{!2} +!llvm.dbg.cu = !{!5} + +!2 = !{i32 2, !"Debug Info Version", i32 3} +!5 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !6, producer: "clang", emissionKind: FullDebug) +!6 = !DIFile(filename: "a.cpp", directory: "/") +!7 = !{} +!11 = !DISubroutineType(types: !7) + +!12 = !DISubprogram(name: "declared_only", scope: !6, file: !6, line: 2, type: !11, spFlags: DISPFlagOptimized, retainedNodes: !7, declaration: !13) +!13 = !DISubprogram(name: "declared_only", scope: !6, file: !6, line: 2, type: !11, spFlags: DISPFlagOptimized, retainedNodes: !7) +; CHECK: subprogram declaration must not have a declaration field +; CHECK: warning: ignoring invalid debug info _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits