li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
`IgnoreParenImpCasts` will remove implicit casts to bool
(e.g. `PointerToBoolean`), such that the resulting expression may not
be of the `bool` type. The `cast_or_null<BoolValue>` in
`extendFlowCondition` will then trigger an assert.
We work around this by unwrapping implicit casts unless we encounter
one that changes the type to `bool`. It's hard to tell if this is a
principled approach or a workaround. We should re-evaluate the need
for this once we have better support for pointers and/or integers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124807
Files:
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,26 @@
});
}
+// FIXME: Remove this test once it provides no additional test coverage.
+TEST_F(FlowConditionTest, DoesNotAssertForImplicitCastToBool) {
+ std::string Code = R"(
+ void target(int *Ptr) {
+ if (Ptr) {
+ (void)0;
+ /*[[p1]]*/
+ } else {
+ (void)1;
+ /*[[p2]]*/
+ }
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ });
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -17,6 +17,7 @@
#include <vector>
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/IgnoreExpr.h"
#include "clang/AST/OperationKinds.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
@@ -68,6 +69,28 @@
return BlockPos - Pred.succ_begin();
}
+/// Skips past any parentheses and implicit casts for `E`, except if they would
+/// perform a type conversion to bool.
+///
+/// FIXME: Its hard to tell if this is a principled solution or workaround to an
+/// issue. Re-evaluate the necessity for this function after we model pointers
+/// and integers in the framework.
+static const Expr *ignoreParenImpCastsExceptToBool(const Expr *E) {
+ const Expr *LastE = nullptr;
+ while (E != LastE) {
+ LastE = E;
+ E = IgnoreParensSingleStep(const_cast<Expr *>(E));
+
+ bool WasBool = E->getType()->isBooleanType();
+ auto *Unwrapped = IgnoreImplicitCastsExtraSingleStep(const_cast<Expr *>(E));
+ if (WasBool && !Unwrapped->getType()->isBooleanType())
+ return E;
+ else
+ E = Unwrapped;
+ }
+ return E;
+}
+
/// Extends the flow condition of an environment based on a terminator
/// statement.
class TerminatorVisitor : public ConstStmtVisitor<TerminatorVisitor> {
@@ -77,26 +100,26 @@
: StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
void VisitIfStmt(const IfStmt *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreParenImpCastsExceptToBool(S->getCond());
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
void VisitWhileStmt(const WhileStmt *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreParenImpCastsExceptToBool(S->getCond());
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 = ignoreParenImpCastsExceptToBool(S->getLHS());
assert(LHS != nullptr);
extendFlowCondition(*LHS);
}
void VisitConditionalOperator(const ConditionalOperator *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreParenImpCastsExceptToBool(S->getCond());
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
