mirrors/llvm-project
0
0
Fork 0
mirror of https://github.com/llvm/llvm-project.git synced 2025-04-24 04:16:08 +00:00
Code Issues Projects Releases Wiki Activity

Revert "[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (#117437)"

Browse Source 
This caused assertion failures:

  clang/lib/Analysis/CFG.cpp:822:
  void (anonymous namespace)::CFGBuilder::appendStmt(CFGBlock *, const Stmt *):
  Assertion `!isa<Expr>(S) || cast<Expr>(S)->IgnoreParens() == S' failed.

See comment on the PR.

This reverts commit 44aa618ef67d302f5ab77cc591fb3434fe967a2e.
This commit is contained in:
Hans Wennborg 2025-02-03 15:52:04 +01:00
parent d906da5ead
commit 90e0dd15ff
10 changed files with 60 additions and 167 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang/docs/ReleaseNotes.rst
View File

@ -219,10 +219,6 @@ Code Completion
Static Analyzer
---------------
- Clang currently support extending lifetime of object bound to
reference members of aggregates in CFG and ExprEngine, that are
created from default member initializer.
New features
^^^^^^^^^^^^

17
clang/lib/AST/ParentMap.cpp
View File

@ -13,7 +13,6 @@
#include "clang/AST/ParentMap.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtObjC.h"
#include "llvm/ADT/DenseMap.h"
@ -104,22 +103,6 @@ static void BuildParentMap(MapTy& M, Stmt* S,
BuildParentMap(M, SubStmt, OVMode);
}
break;
case Stmt::CXXDefaultArgExprClass:
if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
if (Arg->hasRewrittenInit()) {
M[Arg->getExpr()] = S;
BuildParentMap(M, Arg->getExpr(), OVMode);
}
}
break;
case Stmt::CXXDefaultInitExprClass:
if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
if (Init->hasRewrittenInit()) {
M[Init->getExpr()] = S;
BuildParentMap(M, Init->getExpr(), OVMode);
}
}
break;
default:
for (Stmt *SubStmt : S->children()) {
if (SubStmt) {

50
clang/lib/Analysis/CFG.cpp
View File

@ -556,10 +556,6 @@ public:
private:
// Visitors to walk an AST and construct the CFG.
CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
AddStmtChoice asc);
CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
AddStmtChoice asc);
CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@ -2265,10 +2261,16 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
case Stmt::CXXDefaultInitExprClass:
return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
// FIXME: The expression inside a CXXDefaultArgExpr is owned by the
// called function's declaration, not by the caller. If we simply add
// this expression to the CFG, we could end up with the same Expr
// appearing multiple times (PR13385).
//
// It's likewise possible for multiple CXXDefaultInitExprs for the same
// expression to be used in the same function (through aggregate
// initialization).
return VisitStmt(S, asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@ -2438,40 +2440,6 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
return B;
}
CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
AddStmtChoice asc) {
if (Arg->hasRewrittenInit()) {
if (asc.alwaysAdd(*this, Arg)) {
autoCreateBlock();
appendStmt(Block, Arg);
}
return VisitStmt(Arg->getExpr(), asc);
}
// We can't add the default argument if it's not rewritten because the
// expression inside a CXXDefaultArgExpr is owned by the called function's
// declaration, not by the caller, we could end up with the same expression
// appearing multiple times.
return VisitStmt(Arg, asc);
}
CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
AddStmtChoice asc) {
if (Init->hasRewrittenInit()) {
if (asc.alwaysAdd(*this, Init)) {
autoCreateBlock();
appendStmt(Block, Init);
}
return VisitStmt(Init->getExpr(), asc);
}
// We can't add the default initializer if it's not rewritten because multiple
// CXXDefaultInitExprs for the same sub-expression to be used in the same
// function (through aggregate initialization). we could end up with the same
// expression appearing multiple times.
return VisitStmt(Init, asc);
}
CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
if (asc.alwaysAdd(*this, ILE)) {
autoCreateBlock();

37
clang/lib/Analysis/ReachableCode.cpp
View File

@ -454,12 +454,11 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
return isDeadRoot;
}
// Check if the given `DeadStmt` is one of target statements or is a sub-stmt of
// them. `Block` is the CFGBlock containing the `DeadStmt`.
template <class... Ts>
static bool isDeadStmtInOneOf(const Stmt *DeadStmt, const CFGBlock *Block) {
// Check if the given `DeadStmt` is a coroutine statement and is a substmt of
// the coroutine statement. `Block` is the CFGBlock containing the `DeadStmt`.
static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
// The coroutine statement, co_return, co_await, or co_yield.
const Stmt *TargetStmt = nullptr;
const Stmt *CoroStmt = nullptr;
// Find the first coroutine statement after the DeadStmt in the block.
bool AfterDeadStmt = false;
for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
@ -468,27 +467,32 @@ static bool isDeadStmtInOneOf(const Stmt *DeadStmt, const CFGBlock *Block) {
const Stmt *S = CS->getStmt();
if (S == DeadStmt)
AfterDeadStmt = true;
if (AfterDeadStmt && llvm::isa<Ts...>(S)) {
TargetStmt = S;
if (AfterDeadStmt &&
// For simplicity, we only check simple coroutine statements.
(llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) {
CoroStmt = S;
break;
}
}
if (!TargetStmt)
if (!CoroStmt)
return false;
struct Checker : DynamicRecursiveASTVisitor {
const Stmt *DeadStmt;
bool IsSubStmtOfTargetStmt = false;
Checker(const Stmt *S) : DeadStmt(S) { ShouldVisitImplicitCode = true; }
bool CoroutineSubStmt = false;
Checker(const Stmt *S) : DeadStmt(S) {
// Statements captured in the CFG can be implicit.
ShouldVisitImplicitCode = true;
}
bool VisitStmt(Stmt *S) override {
if (S == DeadStmt)
IsSubStmtOfTargetStmt = true;
CoroutineSubStmt = true;
return true;
}
};
Checker checker(DeadStmt);
checker.TraverseStmt(const_cast<Stmt *>(TargetStmt));
return checker.IsSubStmtOfTargetStmt;
checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
return checker.CoroutineSubStmt;
}
static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
@ -499,12 +503,7 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
// Coroutine statements are never considered dead statements, because removing
// them may change the function semantic if it is the only coroutine statement
// of the coroutine.
//
// If the dead stmt is a sub-stmt of CXXDefaultInitExpr and CXXDefaultArgExpr,
// we would rather expect to find CXXDefaultInitExpr and CXXDefaultArgExpr as
// a valid dead stmt.
return !isDeadStmtInOneOf<CoreturnStmt, CoroutineSuspendExpr,
CXXDefaultArgExpr, CXXDefaultInitExpr>(S, Block);
return !isInCoroutineStmt(S, Block);
}
const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {

9
clang/lib/Sema/SemaExpr.cpp
View File

@ -5570,10 +5570,8 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
/*SkipImmediateInvocations=*/NestedDefaultChecking))
return ExprError();
Expr *RewrittenExpr = (Init == Param->getDefaultArg() ? nullptr : Init);
return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
RewrittenExpr,
InitializationContext->Context);
Init, InitializationContext->Context);
}
static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
@ -5691,11 +5689,10 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
return ExprError();
}
Init = Res.get();
Expr *RewrittenInit =
(Init == Field->getInClassInitializer() ? nullptr : Init);
return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
Field, InitializationContext->Context,
RewrittenInit);
Init);
}
// DR1351:

54
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
View File

@ -1987,45 +1987,33 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
ExplodedNodeSet Tmp;
StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
bool HasRebuiltInit = false;
const Expr *ArgE = nullptr;
if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
const Expr *ArgE;
if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
ArgE = DefE->getExpr();
HasRebuiltInit = DefE->hasRewrittenInit();
} else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
ArgE = DefE->getExpr();
HasRebuiltInit = DefE->hasRewrittenInit();
} else
else
llvm_unreachable("unknown constant wrapper kind");
if (HasRebuiltInit) {
for (auto *N : PreVisit) {
ProgramStateRef state = N->getState();
const LocationContext *LCtx = N->getLocationContext();
state = state->BindExpr(S, LCtx, state->getSVal(ArgE, LCtx));
Bldr2.generateNode(S, N, state);
}
} else {
// If it's not rewritten, the contents of these expressions are not
// actually part of the current function, so we fall back to constant
// evaluation.
bool IsTemporary = false;
if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
ArgE = MTE->getSubExpr();
IsTemporary = true;
}
bool IsTemporary = false;
if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
ArgE = MTE->getSubExpr();
IsTemporary = true;
}
std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
const LocationContext *LCtx = Pred->getLocationContext();
for (auto *I : PreVisit) {
ProgramStateRef State = I->getState();
State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
if (IsTemporary)
State = createTemporaryRegionIfNeeded(State, LCtx, cast<Expr>(S),
cast<Expr>(S));
std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
if (!ConstantVal)
ConstantVal = UnknownVal();
Bldr2.generateNode(S, I, State);
}
const LocationContext *LCtx = Pred->getLocationContext();
for (const auto I : PreVisit) {
ProgramStateRef State = I->getState();
State = State->BindExpr(S, LCtx, *ConstantVal);
if (IsTemporary)
State = createTemporaryRegionIfNeeded(State, LCtx,
cast<Expr>(S),
cast<Expr>(S));
Bldr2.generateNode(S, I, State);
}
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);

2
clang/test/AST/ast-dump-recovery.cpp
View File

@ -507,7 +507,7 @@ union U {
// CHECK-NEXT: `-DeclStmt {{.*}}
// CHECK-NEXT: `-VarDecl {{.*}} g 'U':'GH112560::U' listinit
// CHECK-NEXT: `-InitListExpr {{.*}} 'U':'GH112560::U' contains-errors field Field {{.*}} 'f' 'int'
// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors has rewritten init
// CHECK-NEXT: `-RecoveryExpr {{.*}} 'int' contains-errors
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
void foo() {

7
clang/test/Analysis/lifetime-extended-regions.cpp
View File

@ -121,10 +121,11 @@ void aggregateWithReferences() {
clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
// The lifetime of object bound to reference members of aggregates,
// that are created from default member initializer was extended.
// FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
// that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
// The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
RefAggregate defaultInitExtended{i};
clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
}
void lambda() {

8
clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
View File

@ -274,16 +274,16 @@ void f() {
// CHECK: ClassTemplateSpecializationDecl {{.*}} struct A definition
// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr A 'void () noexcept'
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 1
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 2
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} 'a' 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 4
// CHECK-NEXT: CompoundStmt {{.*}}

39
clang/test/SemaCXX/warn-unreachable.cpp
View File

@ -414,42 +414,3 @@ void tautological_compare(bool x, int y) {
calledFun();
}
namespace test_rebuilt_default_arg {
struct A {
explicit A(int = __builtin_LINE());
};
int h(int a) {
return 3;
A(); // expected-warning {{will never be executed}}
}
struct Temp {
Temp();
~Temp();
};
struct B {
explicit B(const Temp &t = Temp());
};
int f(int a) {
return 3;
B(); // expected-warning {{will never be executed}}
}
} // namespace test_rebuilt_default_arg
namespace test_rebuilt_default_init {
struct A {
A();
~A();
};
struct B {
const A &t = A();
};
int f(int a) {
return 3;
A{}; // expected-warning {{will never be executed}}
}
} // namespace test_rebuilt_default_init