clang static analyzer crash with CTU Analysis enabled #123093
Labels
clang:frontend
Language frontend issues, e.g. anything involving "Sema"
crash
Prefer [crash-on-valid] or [crash-on-invalid]
Comments
EugeneZelenko
added
clang:frontend
|
@llvm/issue-subscribers-clang-frontend Author: Tom Ritter (tomrittervg)
The following program (creduce-d from real Firefox source code, originally 133,189 lines long) and command produces a segfault in clang static analyzer. I'm more confident it is in the Analyzer, but not 100% sure, but it outputs "CTU loaded AST file" and disabling CTU makes the segfault go away.
> clang version 20.0.0git (https://github.com/llvm/llvm-project.git 9d5299e) VTuneWrapper.cpp: It does require two AST files and an externalDef file that I don't know how to minimize. I've hosted them here: https://ritter.vg/misc/transient/taint-2/ Here is the top of the stack: Here is the bottom: cc @steakhal |
|
Yea, this one looks like an infinite recursion in astimporter. Unfortunately, i dont know of a general solution to containt all these effectively for once and all. That containment wouldnt help with the import, which would then just fail instead. I acknowledge that would be many magnitudes better than crashing. |
|
I came back to this, and realized that you are sharing the raw ast files. I'd suggest sharing the input files, aka. The usually way of reducing CTU crashes is by first reducing the primary TU, then the secondary TU. Usually I try to add a static counter to some of the import entry points to pin down the root cause of the infinite recursion. diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
@@ -3753,6 +3753,12 @@ ASTNodeImporter::importExplicitSpecifier(Error &Err, ExplicitSpecifier ESpec) {
}
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
+ //llvm::errs() << "ASTNodeImporter::VisitFunctionDecl " << D->getQualifiedNameAsString() << "\n";
+ static int counter = 0;
+ if (D->getQualifiedNameAsString() == StringRef("mozilla::UnderlyingValue")) {
+ ++counter;
+ assert(counter < 3 && "UnderlyingValue is imported multiple times");
+ }
SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
auto RedeclIt = Redecls.begin();There I made sure that the same decl is requested for import ( I'm not an expert on the ASTImporter though, so they may have a more sophisticated process for debugging these. Unfortunately, with the input files, nobody can help you with this case. |
|
Thanks, that's really helpful and I will work on it. |
|
My usual way to debug this type of problem if the stack dump is available: |
|
I think I was reducing separately the exact same inf import. From what I've seen in the debugger, the import of the FunctionDecl Test, that fails on current main (382d359): |
|
I just finished minimizing things from my side as well, so hopefully another test case is helpful: Value-full.cpp Pretenuring-full.cpp VtuneWrapper.cpp Command |
template <typename Enum> auto UnderlyingValue(Enum v) {
return static_cast<typename underlying_type<Enum>::type>(v);
}What if the function prototype type refers to a function that has an auto return type that depends on the type of a template parameter? @balazske My question is, how do we usually break this circular dependency between an auto return type and a type template parameter? I suppose, we should follow the same tactic here. |
|
I think I found two other places where we had a similar issue: |
|
#69724 looks related too. |
I was thinking about this for a while, but I couldn't find a solution. |
|
Let me reiterate the events:
I can't see right now where to break this circle. |
|
I came up with a patch like this, that avoids the crash, but it doesn't fix the import of course: diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c9f2f905d213..8451f2063e2b 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3586,7 +3586,7 @@ namespace {
class IsTypeDeclaredInsideVisitor
: public TypeVisitor<IsTypeDeclaredInsideVisitor, std::optional<bool>> {
public:
- IsTypeDeclaredInsideVisitor(const FunctionDecl *ParentDC)
+ IsTypeDeclaredInsideVisitor(const DeclContext *ParentDC)
: ParentDC(ParentDC) {}
bool CheckType(QualType T) {
@@ -3636,6 +3636,13 @@ public:
return {};
}
+ std::optional<bool> VisitFunctionProtoType(const FunctionProtoType *T) {
+ for (QualType Param : T->param_types())
+ if (CheckType(Param))
+ return true;
+ return CheckType(T->getReturnType());
+ }
+
std::optional<bool>
VisitTemplateSpecializationType(const TemplateSpecializationType *T) {
for (const auto &Arg : T->template_arguments())
@@ -7960,6 +7967,18 @@ ASTNodeImporter::ImportCastPath(CastExpr *CE) {
}
ExpectedStmt ASTNodeImporter::VisitImplicitCastExpr(ImplicitCastExpr *E) {
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(E->getSubExpr())) {
+ if (const auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
+ const DeclContext *DC = FD->isTemplateInstantiation()
+ ? FD->getPrimaryTemplate()->getDeclContext()
+ : FD;
+ IsTypeDeclaredInsideVisitor Visitor(DC);
+ if (Visitor.CheckType(E->getType())) {
+ return make_error<ASTImportError>(ASTImportError::UnsupportedConstruct);
+ }
+ }
+ }
+
ExpectedType ToTypeOrErr = import(E->getType());
if (!ToTypeOrErr)
return ToTypeOrErr.takeError(); |
|
This is the return type of the function In this case the "associated declaration" in the The type
The second one is the problematic because there the associated declaration is function |
|
My previous comment is not exactly true. I found out that there is a problem in |
|
These are the changes to fix the crash: diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c27ebbf838ad..db6bc3ebbbcf 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3614,6 +3614,14 @@ public:
return isAncestorDeclContextOf(ParentDC, T->getDecl());
}
+ std::optional<bool> VisitElaboratedType(const ElaboratedType *T) {
+ if (NestedNameSpecifier *NNS = T->getQualifier())
+ if (const Type *T = NNS->getAsType())
+ if (CheckType(QualType(T, 0)))
+ return true;
+ return {};
+ }
+
std::optional<bool> VisitPointerType(const PointerType *T) {
return CheckType(T->getPointeeType());
}
@@ -3625,7 +3633,9 @@ public:
std::optional<bool> VisitTypedefType(const TypedefType *T) {
const TypedefNameDecl *TD = T->getDecl();
assert(TD);
- return isAncestorDeclContextOf(ParentDC, TD);
+ if (isAncestorDeclContextOf(ParentDC, TD))
+ return true;
+ return {};
}
std::optional<bool> VisitUsingType(const UsingType *T) { |
|
I'll have a look, many thanks! <3 @balazske |
So, I looked at the case again, and I'm not completely satisfied. It's better to not crash, but it would also stop the import of I was thinking of a solution, and I come back again and again to the idea of delaying imports of subexpressions of the function body. The next problem of course that imports of functions may be nested, so these "delay lists" must be also handled separately, but I don't think that should be too hard to handle. WDYT? @balazske |
|
This change (my fix) should not stop the import. It only delays import of the function prototype type until the body was imported. If About the new idea, it looks difficult to first find all the statement nodes that should be delayed (a loop over all statements is needed and a recursive visit of each statement and sub-expression). The data is difficult to pass to import functions of expressions (which is necessary if sub-expressions are to be delayed). What looks almost impossible is to import and set the delayed data where it is needed at the final import phase. Additionally there may be other unexpected dependencies between |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following program (creduce-d from real Firefox source code, originally 133,189 lines long) and command produces a segfault in clang static analyzer. I'm more confident it is in the Analyzer, but not 100% sure, but it outputs "CTU loaded AST file" and disabling CTU makes the segfault go away.
VTuneWrapper.cpp:
It does require two AST files and an externalDef file that I don't know how to minimize. I've hosted them here: https://ritter.vg/misc/transient/taint-2/
Here is the top of the stack:
Here is the bottom:
cc @steakhal
The text was updated successfully, but these errors were encountered: