Skip to content

Commit 86295dc

Browse files
bgra8Bogdan Graur
and
Bogdan Graur
authored
Revert "[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (#91879)" (#94597)
This depends on #92527 which needs to be reverted due to #92527 (comment). This reverts commit 905b402. Co-authored-by: Bogdan Graur <bgraur@google.com>
1 parent 8516f54 commit 86295dc

File tree

6 files changed

+42
-103
lines changed

6 files changed

+42
-103
lines changed

clang/lib/AST/ParentMap.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,22 +97,6 @@ static void BuildParentMap(MapTy& M, Stmt* S,
9797
BuildParentMap(M, SubStmt, OVMode);
9898
}
9999
break;
100-
case Stmt::CXXDefaultArgExprClass:
101-
if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
102-
if (Arg->hasRewrittenInit()) {
103-
M[Arg->getExpr()] = S;
104-
BuildParentMap(M, Arg->getExpr(), OVMode);
105-
}
106-
}
107-
break;
108-
case Stmt::CXXDefaultInitExprClass:
109-
if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
110-
if (Init->hasRewrittenInit()) {
111-
M[Init->getExpr()] = S;
112-
BuildParentMap(M, Init->getExpr(), OVMode);
113-
}
114-
}
115-
break;
116100
default:
117101
for (Stmt *SubStmt : S->children()) {
118102
if (SubStmt) {

clang/lib/Analysis/CFG.cpp

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -556,10 +556,6 @@ class CFGBuilder {
556556

557557
private:
558558
// Visitors to walk an AST and construct the CFG.
559-
CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
560-
AddStmtChoice asc);
561-
CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
562-
AddStmtChoice asc);
563559
CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
564560
CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
565561
CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@@ -2258,10 +2254,16 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
22582254
asc, ExternallyDestructed);
22592255

22602256
case Stmt::CXXDefaultArgExprClass:
2261-
return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
2262-
22632257
case Stmt::CXXDefaultInitExprClass:
2264-
return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
2258+
// FIXME: The expression inside a CXXDefaultArgExpr is owned by the
2259+
// called function's declaration, not by the caller. If we simply add
2260+
// this expression to the CFG, we could end up with the same Expr
2261+
// appearing multiple times (PR13385).
2262+
//
2263+
// It's likewise possible for multiple CXXDefaultInitExprs for the same
2264+
// expression to be used in the same function (through aggregate
2265+
// initialization).
2266+
return VisitStmt(S, asc);
22652267

22662268
case Stmt::CXXBindTemporaryExprClass:
22672269
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2431,40 +2433,6 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
24312433
return B;
24322434
}
24332435

2434-
CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
2435-
AddStmtChoice asc) {
2436-
if (Arg->hasRewrittenInit()) {
2437-
if (asc.alwaysAdd(*this, Arg)) {
2438-
autoCreateBlock();
2439-
appendStmt(Block, Arg);
2440-
}
2441-
return VisitStmt(Arg->getExpr(), asc);
2442-
}
2443-
2444-
// We can't add the default argument if it's not rewritten because the
2445-
// expression inside a CXXDefaultArgExpr is owned by the called function's
2446-
// declaration, not by the caller, we could end up with the same expression
2447-
// appearing multiple times.
2448-
return VisitStmt(Arg, asc);
2449-
}
2450-
2451-
CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
2452-
AddStmtChoice asc) {
2453-
if (Init->hasRewrittenInit()) {
2454-
if (asc.alwaysAdd(*this, Init)) {
2455-
autoCreateBlock();
2456-
appendStmt(Block, Init);
2457-
}
2458-
return VisitStmt(Init->getExpr(), asc);
2459-
}
2460-
2461-
// We can't add the default initializer if it's not rewritten because multiple
2462-
// CXXDefaultInitExprs for the same sub-expression to be used in the same
2463-
// function (through aggregate initialization). we could end up with the same
2464-
// expression appearing multiple times.
2465-
return VisitStmt(Init, asc);
2466-
}
2467-
24682436
CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
24692437
if (asc.alwaysAdd(*this, ILE)) {
24702438
autoCreateBlock();

clang/lib/Sema/SemaExpr.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5608,7 +5608,6 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
56085608
InitializationContext.emplace(Loc, Field, CurContext);
56095609

56105610
Expr *Init = nullptr;
5611-
bool HasRewrittenInit = false;
56125611

56135612
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
56145613
bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
@@ -5658,7 +5657,6 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
56585657
isa_and_present<ExprWithCleanups>(Field->getInClassInitializer());
56595658
if (V.HasImmediateCalls || InLifetimeExtendingContext ||
56605659
ContainsAnyTemporaries) {
5661-
HasRewrittenInit = true;
56625660
ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
56635661
CurContext};
56645662
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
@@ -5702,7 +5700,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
57025700

57035701
return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
57045702
Field, InitializationContext->Context,
5705-
HasRewrittenInit ? Init : nullptr);
5703+
Init);
57065704
}
57075705

57085706
// DR1351:

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,45 +1971,33 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
19711971
ExplodedNodeSet Tmp;
19721972
StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
19731973

1974-
bool HasRewrittenInit = false;
1975-
const Expr *ArgE = nullptr;
1976-
if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
1974+
const Expr *ArgE;
1975+
if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
19771976
ArgE = DefE->getExpr();
1978-
HasRewrittenInit = DefE->hasRewrittenInit();
1979-
} else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
1977+
else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
19801978
ArgE = DefE->getExpr();
1981-
HasRewrittenInit = DefE->hasRewrittenInit();
1982-
} else
1979+
else
19831980
llvm_unreachable("unknown constant wrapper kind");
19841981

1985-
if (HasRewrittenInit) {
1986-
for (auto *N : PreVisit) {
1987-
ProgramStateRef state = N->getState();
1988-
const LocationContext *LCtx = N->getLocationContext();
1989-
state = state->BindExpr(S, LCtx, state->getSVal(ArgE, LCtx));
1990-
Bldr2.generateNode(S, N, state);
1991-
}
1992-
} else {
1993-
// If it's not rewritten, the contents of these expressions are not
1994-
// actually part of the current function, so we fall back to constant
1995-
// evaluation.
1996-
bool IsTemporary = false;
1997-
if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
1998-
ArgE = MTE->getSubExpr();
1999-
IsTemporary = true;
2000-
}
2001-
2002-
std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
2003-
const LocationContext *LCtx = Pred->getLocationContext();
2004-
for (auto *I : PreVisit) {
2005-
ProgramStateRef State = I->getState();
2006-
State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
2007-
if (IsTemporary)
2008-
State = createTemporaryRegionIfNeeded(State, LCtx, cast<Expr>(S),
2009-
cast<Expr>(S));
1982+
bool IsTemporary = false;
1983+
if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
1984+
ArgE = MTE->getSubExpr();
1985+
IsTemporary = true;
1986+
}
20101987

2011-
Bldr2.generateNode(S, I, State);
2012-
}
1988+
std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
1989+
if (!ConstantVal)
1990+
ConstantVal = UnknownVal();
1991+
1992+
const LocationContext *LCtx = Pred->getLocationContext();
1993+
for (const auto I : PreVisit) {
1994+
ProgramStateRef State = I->getState();
1995+
State = State->BindExpr(S, LCtx, *ConstantVal);
1996+
if (IsTemporary)
1997+
State = createTemporaryRegionIfNeeded(State, LCtx,
1998+
cast<Expr>(S),
1999+
cast<Expr>(S));
2000+
Bldr2.generateNode(S, I, State);
20132001
}
20142002

20152003
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);

clang/test/Analysis/cxx-uninitialized-object.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,27 +1114,27 @@ void fCXX11MemberInitTest1() {
11141114
CXX11MemberInitTest1();
11151115
}
11161116

1117-
#ifdef PEDANTIC
11181117
struct CXX11MemberInitTest2 {
11191118
struct RecordType {
1120-
int a; // expected-note {{uninitialized field 'this->a'}}
1121-
int b; // expected-note {{uninitialized field 'this->b'}}
1119+
// TODO: we'd expect the note: {{uninitialized field 'this->rec.a'}}
1120+
int a; // no-note
1121+
// TODO: we'd expect the note: {{uninitialized field 'this->rec.b'}}
1122+
int b; // no-note
11221123

11231124
RecordType(int) {}
11241125
};
11251126

1126-
RecordType rec = RecordType(int()); // expected-warning {{2 uninitialized fields}}
1127+
RecordType rec = RecordType(int());
11271128
int dontGetFilteredByNonPedanticMode = 0;
11281129

11291130
CXX11MemberInitTest2() {}
11301131
};
11311132

11321133
void fCXX11MemberInitTest2() {
1134+
// TODO: we'd expect the warning: {{2 uninitializeds field}}
11331135
CXX11MemberInitTest2(); // no-warning
11341136
}
11351137

1136-
#endif // PEDANTIC
1137-
11381138
//===----------------------------------------------------------------------===//
11391139
// "Esoteric" primitive type tests.
11401140
//===----------------------------------------------------------------------===//

clang/test/Analysis/lifetime-extended-regions.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,11 @@ void aggregateWithReferences() {
121121
clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
122122
clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
123123

124-
// The lifetime lifetime of object bound to reference members of aggregates,
125-
// that are created from default member initializer was extended.
124+
// FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
125+
// that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
126+
// The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
126127
RefAggregate defaultInitExtended{i};
127-
clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
128+
clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
128129
}
129130

130131
void lambda() {

0 commit comments

Comments
 (0)