li.zhe.hua updated this revision to Diff 426860.
li.zhe.hua edited the summary of this revision.
li.zhe.hua added a comment.
Expand `ignoreExprWithCleanups` comment with more context.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124807/new/
https://reviews.llvm.org/D124807
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1152,4 +1152,39 @@
});
}
+// FIXME: Remove this test once it provides no additional test coverage.
+TEST_F(FlowConditionTest, DoesNotAssertForImplicitCastToBool) {
+ std::string Code = R"(
+ void target(int *Ptr) {
+ bool Foo = false;
+ if (Ptr) {
+ Foo = true;
+ /*[[p1]]*/
+ }
+
+ (void)0;
+ /*[[p2]]*/
+ }
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ const Environment &Env1 = Results[1].second.Env;
+ auto &FooVal1 =
+ *cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::Reference));
+ EXPECT_TRUE(Env1.flowConditionImplies(FooVal1));
+
+ const Environment &Env2 = Results[0].second.Env;
+ auto &FooVal2 =
+ *cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::Reference));
+ EXPECT_FALSE(Env2.flowConditionImplies(FooVal2));
+ });
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -77,26 +77,26 @@
: StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
void VisitIfStmt(const IfStmt *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
void VisitWhileStmt(const WhileStmt *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
void VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
- auto *LHS = S->getLHS()->IgnoreParenImpCasts();
+ auto *LHS = ignoreExprWithCleanups(S->getLHS())->IgnoreParens();
assert(LHS != nullptr);
extendFlowCondition(*LHS);
}
void VisitConditionalOperator(const ConditionalOperator *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -33,7 +33,7 @@
namespace clang {
namespace dataflow {
-static const Expr *skipExprWithCleanups(const Expr *E) {
+const Expr *ignoreExprWithCleanups(const Expr *E) {
if (auto *C = dyn_cast_or_null<ExprWithCleanups>(E))
return C->getSubExpr();
return E;
@@ -157,7 +157,7 @@
// The CFG does not contain `ParenExpr` as top-level statements in basic
// blocks, however sub-expressions can still be of that type.
- InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+ InitExpr = ignoreExprWithCleanups(D.getInit()->IgnoreParens());
assert(InitExpr != nullptr);
if (D.getType()->isReferenceType()) {
@@ -605,6 +605,7 @@
void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env) {
assert(!isa<ParenExpr>(&S));
+ assert(!isa<ExprWithCleanups>(&S));
TransferVisitor(StmtToEnv, Env).Visit(&S);
}
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -220,8 +220,8 @@
void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
LatticeTransferState &State) {
- if (auto *OptionalVal = cast_or_null<StructValue>(
- State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer))) {
+ if (auto *OptionalVal = cast_or_null<StructValue>(State.Env.getValue(
+ *ObjectExpr->IgnoreParens(), SkipPast::ReferenceThenPointer))) {
auto *HasValueVal = getHasValue(OptionalVal);
assert(HasValueVal != nullptr);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -15,6 +15,7 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
@@ -425,6 +426,8 @@
}
Value *Environment::getValue(const Expr &E, SkipPast SP) const {
+ assert(!isa<ParenExpr>(&E));
+ assert(!isa<ExprWithCleanups>(&E));
auto *Loc = getStorageLocation(E, SP);
if (Loc == nullptr)
return nullptr;
Index: clang/include/clang/Analysis/FlowSensitive/Transfer.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -35,9 +35,19 @@
///
/// Requirements:
///
-/// The type of `S` must not be `ParenExpr`.
+/// The type of `S` must not be `ParenExpr` or `ExprWithCleanups`.
void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);
+/// Skip past any `ExprWithCleanups` which might surround `E`. Returns a null
+/// pointer if `E` is null.
+///
+/// The CFG omits `ExprWithCleanups` nodes (as it does with `ParenExpr`), and so
+/// the transfer function doesn't accept them as valid input. Manual traversal
+/// of the AST should skip and unwrap any `ExprWithCleanups` it might expect to
+/// see. They are safe to skip, as the CFG will emit calls to destructors as
+/// appropriate.
+const Expr *ignoreExprWithCleanups(const Expr *E);
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -226,6 +226,10 @@
/// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if `E`
/// is assigned a storage location in the environment, otherwise returns null.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ParenExpr` or `ExprWithCleanups`.
Value *getValue(const Expr &E, SkipPast SP) const;
/// Transfers ownership of `Loc` to the analysis context and returns a
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
