diff options
author | Felipe de Azevedo Piovezan <fpiovezan@apple.com> | 2023-02-10 15:21:46 -0500 |
---|---|---|
committer | Felipe de Azevedo Piovezan <fpiovezan@apple.com> | 2023-02-20 14:22:49 -0500 |
commit | 997dc7e00f49663b60a78e18df1dfecdf62a4172 (patch) | |
tree | 0b8e0deae58c3a0d144319b252865e13c265656e /clang/lib/CodeGen/CGDebugInfo.cpp | |
parent | 37e6a4f9496c8e35efc654d7619a79d6dbb72f99 (diff) | |
download | llvm-997dc7e00f49663b60a78e18df1dfecdf62a4172.zip llvm-997dc7e00f49663b60a78e18df1dfecdf62a4172.tar.gz llvm-997dc7e00f49663b60a78e18df1dfecdf62a4172.tar.bz2 |
[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
Diffstat (limited to 'clang/lib/CodeGen/CGDebugInfo.cpp')
-rw-r--r-- | clang/lib/CodeGen/CGDebugInfo.cpp | 7 |
1 files changed, 3 insertions, 4 deletions
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index bb91b51..4dab595 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 |