[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression #91879
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
zygoloid
reviewed
May 12, 2024
| } | ||
| return VisitStmt(Arg->getExpr(), asc); | ||
| } | ||
| return VisitStmt(Arg, asc); |
There was a problem hiding this comment.
I think it'd be useful to keep some of the old comment here: we can't add the default argument if it's not rewritten because we could end up with the same expression appearing multiple times.
There was a problem hiding this comment.
Actually, are we safe from that even if the default argument is rewritten? Do we guarantee to recreate all subexpressions in that case?
There was a problem hiding this comment.
Thanks a lot for your review!
I think it'd be useful to keep some of the old comment here: we can't add the default argument if it's not rewritten because we could end up with the same expression appearing multiple times.
Agree 100%, keep the old comment here can make things more clear.
Actually, are we safe from that even if the default argument is rewritten? Do we guarantee to recreate all subexpressions in that case?
Not exactly, AFAIK, in this case https://godbolt.org/z/ev9KnG7j9 , CXXDefaultInitExpr::hasRewrittenInit() returns true, but the sub-expression was not actually rebuilt. I'm not sure whether it's a bug.
There was a problem hiding this comment.
I've explicit specific HasRewrittenInit in BuildCXXDefaultInitExpr when construct CXXDefaultInitExpr. This solution is temporarily feasible, but it may not be a good way.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang-static-analyzer-1 Author: None (yronglin) ChangesDepends on #87933 Clang now support the following:
But CFG and ExprEngine need to be updated to address this change. This PR add Full diff: https://github.com/llvm/llvm-project/pull/91879.diff 6 Files Affected:
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index 3d6a1cc84c7b1..534793b837bbb 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -97,6 +97,22 @@ 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) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 64e6155de090c..02317257c2740 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -556,6 +556,10 @@ class CFGBuilder {
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);
@@ -2254,16 +2258,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
+ return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
case Stmt::CXXDefaultInitExprClass:
- // 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);
+ return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2433,6 +2431,40 @@ 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();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index bb4b116fd73ca..eed328c716382 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5813,6 +5813,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
InitializationContext.emplace(Loc, Field, CurContext);
Expr *Init = nullptr;
+ bool HasRewrittenInit = false;
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
@@ -5862,6 +5863,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
isa_and_present<ExprWithCleanups>(Field->getInClassInitializer());
if (V.HasImmediateCalls || InLifetimeExtendingContext ||
ContainsAnyTemporaries) {
+ HasRewrittenInit = true;
ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
CurContext};
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
@@ -5905,7 +5907,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
Field, InitializationContext->Context,
- Init);
+ HasRewrittenInit ? Init : nullptr);
}
// DR1351:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 0b1edf3e5c96b..525239101471e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
case Stmt::CXXDefaultArgExprClass:
case Stmt::CXXDefaultInitExprClass: {
Bldr.takeNodes(Pred);
- ExplodedNodeSet PreVisit;
- getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+ ExplodedNodeSet CheckedSet;
+ getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
ExplodedNodeSet Tmp;
- StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+ StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
- const Expr *ArgE;
- if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
+ bool HasRewrittenInit = false;
+ const Expr *ArgE = nullptr;
+ if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
ArgE = DefE->getExpr();
- else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
+ HasRewrittenInit = DefE->hasRewrittenInit();
+ } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
ArgE = DefE->getExpr();
- else
+ HasRewrittenInit = DefE->hasRewrittenInit();
+ } else
llvm_unreachable("unknown constant wrapper kind");
- bool IsTemporary = false;
- if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
- ArgE = MTE->getSubExpr();
- IsTemporary = true;
- }
+ if (HasRewrittenInit) {
+ for (auto *I : CheckedSet) {
+ ProgramStateRef state = (*I).getState();
+ const LocationContext *LCtx = (*I).getLocationContext();
+ SVal Val = state->getSVal(ArgE, LCtx);
+ state = state->BindExpr(S, LCtx, Val);
+ Bldr2.generateNode(S, I, 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;
+ }
+
+ std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
+ if (!ConstantVal)
+ ConstantVal = UnknownVal();
- std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
- if (!ConstantVal)
- ConstantVal = UnknownVal();
-
- 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),
+ const LocationContext *LCtx = Pred->getLocationContext();
+ for (auto *I : CheckedSet) {
+ 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);
+
+ Bldr2.generateNode(S, I, State);
+ }
}
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
diff --git a/clang/test/Analysis/cxx-uninitialized-object.cpp b/clang/test/Analysis/cxx-uninitialized-object.cpp
index e3fa8ae8d7f29..c62fd1c312056 100644
--- a/clang/test/Analysis/cxx-uninitialized-object.cpp
+++ b/clang/test/Analysis/cxx-uninitialized-object.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++14 -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++14 -verify=expected,pedantic %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=optin.cplusplus.UninitializedObject \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
@@ -1114,17 +1114,16 @@ void fCXX11MemberInitTest1() {
CXX11MemberInitTest1();
}
+#ifdef PEDANTIC
struct CXX11MemberInitTest2 {
struct RecordType {
- // TODO: we'd expect the note: {{uninitialized field 'this->rec.a'}}
- int a; // no-note
- // TODO: we'd expect the note: {{uninitialized field 'this->rec.b'}}
- int b; // no-note
+ int a; // expected-note {{uninitialized field 'this->a'}}
+ int b; // expected-note {{uninitialized field 'this->b'}}
RecordType(int) {}
};
- RecordType rec = RecordType(int());
+ RecordType rec = RecordType(int()); // expected-warning {{2 uninitialized fields at the end of the constructor call}}
int dontGetFilteredByNonPedanticMode = 0;
CXX11MemberInitTest2() {}
@@ -1135,6 +1134,8 @@ void fCXX11MemberInitTest2() {
CXX11MemberInitTest2(); // no-warning
}
+#endif // PEDANTIC
+
//===----------------------------------------------------------------------===//
// "Esoteric" primitive type tests.
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4458ad294af7c..524f4e0c400d1 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -121,11 +121,10 @@ 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]+}}} }}
- // 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]+}}} }}
+ // The lifetime lifetime of object bound to reference members of aggregates,
+ // that are created from default member initializer was extended.
RefAggregate defaultInitExtended{i};
- clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
+ clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
}
void lambda() {
|
|
@llvm/pr-subscribers-clang Author: None (yronglin) ChangesDepends on #87933 Clang now support the following:
But CFG and ExprEngine need to be updated to address this change. This PR add Full diff: https://github.com/llvm/llvm-project/pull/91879.diff 6 Files Affected:
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index 3d6a1cc84c7b1..534793b837bbb 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -97,6 +97,22 @@ 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) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 64e6155de090c..02317257c2740 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -556,6 +556,10 @@ class CFGBuilder {
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);
@@ -2254,16 +2258,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
+ return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
case Stmt::CXXDefaultInitExprClass:
- // 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);
+ return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2433,6 +2431,40 @@ 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();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index bb4b116fd73ca..eed328c716382 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5813,6 +5813,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
InitializationContext.emplace(Loc, Field, CurContext);
Expr *Init = nullptr;
+ bool HasRewrittenInit = false;
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
@@ -5862,6 +5863,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
isa_and_present<ExprWithCleanups>(Field->getInClassInitializer());
if (V.HasImmediateCalls || InLifetimeExtendingContext ||
ContainsAnyTemporaries) {
+ HasRewrittenInit = true;
ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
CurContext};
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
@@ -5905,7 +5907,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
Field, InitializationContext->Context,
- Init);
+ HasRewrittenInit ? Init : nullptr);
}
// DR1351:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 0b1edf3e5c96b..525239101471e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
case Stmt::CXXDefaultArgExprClass:
case Stmt::CXXDefaultInitExprClass: {
Bldr.takeNodes(Pred);
- ExplodedNodeSet PreVisit;
- getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+ ExplodedNodeSet CheckedSet;
+ getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
ExplodedNodeSet Tmp;
- StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+ StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
- const Expr *ArgE;
- if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
+ bool HasRewrittenInit = false;
+ const Expr *ArgE = nullptr;
+ if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
ArgE = DefE->getExpr();
- else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
+ HasRewrittenInit = DefE->hasRewrittenInit();
+ } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
ArgE = DefE->getExpr();
- else
+ HasRewrittenInit = DefE->hasRewrittenInit();
+ } else
llvm_unreachable("unknown constant wrapper kind");
- bool IsTemporary = false;
- if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
- ArgE = MTE->getSubExpr();
- IsTemporary = true;
- }
+ if (HasRewrittenInit) {
+ for (auto *I : CheckedSet) {
+ ProgramStateRef state = (*I).getState();
+ const LocationContext *LCtx = (*I).getLocationContext();
+ SVal Val = state->getSVal(ArgE, LCtx);
+ state = state->BindExpr(S, LCtx, Val);
+ Bldr2.generateNode(S, I, 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;
+ }
+
+ std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
+ if (!ConstantVal)
+ ConstantVal = UnknownVal();
- std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
- if (!ConstantVal)
- ConstantVal = UnknownVal();
-
- 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),
+ const LocationContext *LCtx = Pred->getLocationContext();
+ for (auto *I : CheckedSet) {
+ 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);
+
+ Bldr2.generateNode(S, I, State);
+ }
}
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
diff --git a/clang/test/Analysis/cxx-uninitialized-object.cpp b/clang/test/Analysis/cxx-uninitialized-object.cpp
index e3fa8ae8d7f29..c62fd1c312056 100644
--- a/clang/test/Analysis/cxx-uninitialized-object.cpp
+++ b/clang/test/Analysis/cxx-uninitialized-object.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++14 -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++14 -verify=expected,pedantic %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=optin.cplusplus.UninitializedObject \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
@@ -1114,17 +1114,16 @@ void fCXX11MemberInitTest1() {
CXX11MemberInitTest1();
}
+#ifdef PEDANTIC
struct CXX11MemberInitTest2 {
struct RecordType {
- // TODO: we'd expect the note: {{uninitialized field 'this->rec.a'}}
- int a; // no-note
- // TODO: we'd expect the note: {{uninitialized field 'this->rec.b'}}
- int b; // no-note
+ int a; // expected-note {{uninitialized field 'this->a'}}
+ int b; // expected-note {{uninitialized field 'this->b'}}
RecordType(int) {}
};
- RecordType rec = RecordType(int());
+ RecordType rec = RecordType(int()); // expected-warning {{2 uninitialized fields at the end of the constructor call}}
int dontGetFilteredByNonPedanticMode = 0;
CXX11MemberInitTest2() {}
@@ -1135,6 +1134,8 @@ void fCXX11MemberInitTest2() {
CXX11MemberInitTest2(); // no-warning
}
+#endif // PEDANTIC
+
//===----------------------------------------------------------------------===//
// "Esoteric" primitive type tests.
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4458ad294af7c..524f4e0c400d1 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -121,11 +121,10 @@ 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]+}}} }}
- // 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]+}}} }}
+ // The lifetime lifetime of object bound to reference members of aggregates,
+ // that are created from default member initializer was extended.
RefAggregate defaultInitExtended{i};
- clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
+ clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
}
void lambda() {
|
steakhal
reviewed
May 14, 2024
yronglin
commented
May 17, 2024
| @@ -5905,7 +5907,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { | |||
|
|
|||
| return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc, | |||
| Field, InitializationContext->Context, | |||
| Init); | |||
| HasRewrittenInit ? Init : nullptr); | |||
There was a problem hiding this comment.
@cor3ntin Could you please help confirm this change is make sense?
…it expression Signed-off-by: yronglin <[email protected]>
Signed-off-by: yronglin <[email protected]>
Signed-off-by: yronglin <[email protected]>
Signed-off-by: yronglin <[email protected]>
yronglin
force-pushed
the
default_init_in_cfg
branch
from
0c88047 to
354ffb9
Compare
Signed-off-by: yronglin <[email protected]>
|
Thanks for your review! The CI did passed(https://buildkite.com/llvm-project/github-pull-requests/builds/66630) but not shown on github for whatever reason. |
|
FYI this patch messes up some diagnostics with If the unreachable code is a constructor with a default parameter that is a builtin function, clang incorrectly highlights the call to the builtin instead of the call to the constructor. There's a similar issue if instead of a builtin the default parameter is the return value of another function which itself has default parameter that is a builtin: https://godbolt.org/z/ErarnbGbY This was originally observed in Chrome - see https://crbug.com/343231820. |
bgra8
added a commit
that referenced
this pull request
Jun 6, 2024
…ult 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 <[email protected]>
copybara-service bot
pushed a commit
to google/crubit
that referenced
this pull request
Jul 19, 2024
…CXXDefaultInitExprs may be copied in the AST. This fixes a crash that I believe did not exist before llvm/llvm-project#91879. The CXXDefaultInitExpr that is the in-class member initializer is now not the same object that is referenced through the constructor's initializers, so we could no longer retrieve the correct environment values for the expressions involved. We need to use the object referenced by the constructor's initializers. The minimal crash repro I could find still needed `#include <assert.h>`, so isn't an option for unit tests in collect_evidence_test.cc: ```cpp #include <assert.h> #include <memory> int* g; class C { C() {} std::unique_ptr<int, void (*)(int*)> field_{ g, [](int*) { __assert_fail("", __builtin_FILE(), 0, ""); }}; }; ``` PiperOrigin-RevId: 654006679 Change-Id: If4b8455132ae1b637be3544e15e63bcdf962772b
yronglin
added a commit
that referenced
this pull request
Feb 1, 2025
… expression (#117437) Clang currently support extending lifetime of object bound to reference members of aggregates, that are created from default member initializer. This PR address this change and updaye CFG and ExprEngine. This PR reapply #91879. Fixes #93725. --------- Signed-off-by: yronglin <[email protected]>
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 1, 2025
…efault init expression (#117437) Clang currently support extending lifetime of object bound to reference members of aggregates, that are created from default member initializer. This PR address this change and updaye CFG and ExprEngine. This PR reapply llvm/llvm-project#91879. Fixes llvm/llvm-project#93725. --------- Signed-off-by: yronglin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #92527
Clang now support the following:
CXXDefaultArgExprandCXXDefaultInitExpras needed where called or constructed.But CFG and ExprEngine need to be updated to address this change.
This PR add
CXXDefaultArgExprandCXXDefaultInitExprinto CFG, and correct handle these expressions in ExprEngine