samestep updated this revision to Diff 446914.
samestep added a comment.
Don't allow globals
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130306/new/
https://reviews.llvm.org/D130306
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -39,14 +39,14 @@
template <typename Matcher>
void runDataflow(llvm::StringRef Code, Matcher Match,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- bool ApplyBuiltinTransfer = true,
- llvm::StringRef TargetFun = "target") {
+ DataflowAnalysisOptions Options,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ llvm::StringRef TargetFun = "target") {
ASSERT_THAT_ERROR(
test::checkDataflow<NoopAnalysis>(
Code, TargetFun,
- [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
- return NoopAnalysis(C, ApplyBuiltinTransfer);
+ [Options](ASTContext &C, Environment &) {
+ return NoopAnalysis(C, Options);
},
[&Match](
llvm::ArrayRef<
@@ -54,12 +54,19 @@
Results,
ASTContext &ASTCtx) { Match(Results, ASTCtx); },
{"-fsyntax-only", "-fno-delayed-template-parsing",
- "-std=" +
- std::string(
- LangStandard::getLangStandardForKind(Std).getName())}),
+ "-std=" + std::string(
+ LangStandard::getLangStandardForKind(Std).getName())}),
llvm::Succeeded());
}
+template <typename Matcher>
+void runDataflow(llvm::StringRef Code, Matcher Match,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ bool ApplyBuiltinTransfer = true,
+ llvm::StringRef TargetFun = "target") {
+ runDataflow(Code, Match, {ApplyBuiltinTransfer}, Std, TargetFun);
+}
+
TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
std::string Code = R"(
void target() {
@@ -3862,4 +3869,95 @@
});
}
+TEST(TransferTest, ContextSensitiveOptionDisabled) {
+ std::string Code = R"(
+ bool GiveBool();
+ void SetBool(bool &Var) { Var = true; }
+
+ void target() {
+ bool Foo = GiveBool();
+ SetBool(Foo);
+ // [[p]]
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ auto &FooVal =
+ *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+ EXPECT_FALSE(Env.flowConditionImplies(FooVal));
+ EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+}
+
+TEST(TransferTest, ContextSensitiveSetTrue) {
+ std::string Code = R"(
+ bool GiveBool();
+ void SetBool(bool &Var) { Var = true; }
+
+ void target() {
+ bool Foo = GiveBool();
+ SetBool(Foo);
+ // [[p]]
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ auto &FooVal =
+ *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveSetFalse) {
+ std::string Code = R"(
+ bool GiveBool();
+ void SetBool(bool &Var) { Var = false; }
+
+ void target() {
+ bool Foo = GiveBool();
+ SetBool(Foo);
+ // [[p]]
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ auto &FooVal =
+ *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -74,8 +74,9 @@
class TerminatorVisitor : public ConstStmtVisitor<TerminatorVisitor> {
public:
TerminatorVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env,
- int BlockSuccIdx)
- : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
+ int BlockSuccIdx, TransferOptions TransferOptions)
+ : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx),
+ TransferOptions(TransferOptions) {}
void VisitIfStmt(const IfStmt *S) {
auto *Cond = S->getCond();
@@ -118,7 +119,7 @@
void extendFlowCondition(const Expr &Cond) {
// The terminator sub-expression might not be evaluated.
if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
- transfer(StmtToEnv, Cond, Env);
+ transfer(StmtToEnv, Cond, Env, TransferOptions);
// FIXME: The flow condition must be an r-value, so `SkipPast::None` should
// suffice.
@@ -150,6 +151,7 @@
const StmtToEnvMap &StmtToEnv;
Environment &Env;
int BlockSuccIdx;
+ TransferOptions TransferOptions;
};
/// Computes the input state for a given basic block by joining the output
@@ -217,7 +219,8 @@
if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
TerminatorVisitor(StmtToEnv, PredState.Env,
- blockIndexInPredecessor(*Pred, Block))
+ blockIndexInPredecessor(*Pred, Block),
+ Analysis.builtinTransferOptions())
.Visit(PredTerminatorStmt);
}
}
@@ -253,7 +256,8 @@
assert(S != nullptr);
if (Analysis.applyBuiltinTransfer())
- transfer(StmtToEnvMapImpl(CFCtx, BlockStates), *S, State.Env);
+ transfer(StmtToEnvMapImpl(CFCtx, BlockStates), *S, State.Env,
+ Analysis.builtinTransferOptions());
Analysis.transferTypeErased(S, State.Lattice, State.Env);
if (HandleTransferredStmt != nullptr)
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -20,7 +20,9 @@
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/OperatorKinds.h"
@@ -46,8 +48,9 @@
class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
public:
- TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env)
- : StmtToEnv(StmtToEnv), Env(Env) {}
+ TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env,
+ TransferOptions Options)
+ : StmtToEnv(StmtToEnv), Env(Env), Options(Options) {}
void VisitBinaryOperator(const BinaryOperator *S) {
const Expr *LHS = S->getLHS();
@@ -503,6 +506,35 @@
if (ArgLoc == nullptr)
return;
Env.setStorageLocation(*S, *ArgLoc);
+ } else if (const FunctionDecl *F = S->getDirectCallee()) {
+ // This case is for context-sensitive analysis, which we only do if we
+ // have the callee body available in the translation unit.
+ if (!Options.ContextSensitive || F->getBody() == nullptr)
+ return;
+
+ auto &ASTCtx = F->getASTContext();
+
+ // FIXME: Cache these CFGs.
+ auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
+ // FIXME: Handle errors here and below.
+ assert(CFCtx);
+ auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
+
+ auto CalleeEnv = Env.pushCall(S);
+
+ // FIXME: Use the same analysis as the caller for the callee.
+ DataflowAnalysisOptions Options;
+ auto Analysis = NoopAnalysis(ASTCtx, Options);
+
+ auto BlockToOutputState =
+ dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
+ assert(BlockToOutputState);
+ assert(ExitBlock < BlockToOutputState->size());
+
+ auto ExitState = (*BlockToOutputState)[ExitBlock];
+ assert(ExitState);
+
+ Env = ExitState->Env;
}
}
@@ -633,10 +665,12 @@
const StmtToEnvMap &StmtToEnv;
Environment &Env;
+ TransferOptions Options;
};
-void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env) {
- TransferVisitor(StmtToEnv, Env).Visit(&S);
+void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env,
+ TransferOptions Options) {
+ TransferVisitor(StmtToEnv, Env, Options).Visit(&S);
}
} // namespace dataflow
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -202,6 +202,47 @@
}
}
+Environment Environment::pushCall(const CallExpr *Call) const {
+ Environment Env(*this);
+
+ // FIXME: Currently this only works if the callee is never a method and the
+ // same callee is never analyzed from multiple separate callsites. To
+ // generalize this, we'll need to store a "context" field (probably a stack of
+ // `const CallExpr *`s) in the `Environment`, and then change the
+ // `DataflowAnalysisContext` class to hold a map from contexts to "frames",
+ // where each frame stores its own version of what are currently the
+ // `DeclToLoc`, `ExprToLoc`, and `ThisPointeeLoc` fields.
+
+ const auto *FuncDecl = Call->getDirectCallee();
+ assert(FuncDecl != nullptr);
+ const auto *Body = FuncDecl->getBody();
+ assert(Body != nullptr);
+ // FIXME: In order to allow the callee to reference globals, we probably need
+ // to call `initGlobalVars` here in some way.
+
+ auto ParamIt = FuncDecl->param_begin();
+ auto ParamEnd = FuncDecl->param_end();
+ auto ArgIt = Call->arg_begin();
+ auto ArgEnd = Call->arg_end();
+
+ // FIXME: We iterate through the arguments instead of the parameters with the
+ // intention that this will still work in the presence of default parameters,
+ // but in general that is probably insufficient because those default
+ // parameters still need to be given `StorageLocation`s anyway, so this code
+ // will need to be generalized later.
+ for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
+ assert(ParamIt != ParamEnd);
+
+ const VarDecl *Param = *ParamIt;
+ const Expr *Arg = *ArgIt;
+ auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
+ assert(ArgLoc != nullptr);
+ Env.setStorageLocation(*Param, *ArgLoc);
+ }
+
+ return Env;
+}
+
bool Environment::equivalentTo(const Environment &Other,
Environment::ValueModel &Model) const {
assert(DACtx == Other.DACtx);
Index: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -23,6 +23,7 @@
#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Transfer.h"
#include "llvm/ADT/Any.h"
#include "llvm/ADT/Optional.h"
#include "llvm/Support/Error.h"
@@ -36,6 +37,9 @@
// (at which point the built-in transfer functions can be simply a standalone
// analysis).
bool ApplyBuiltinTransfer = true;
+
+ /// Only has an effect if `ApplyBuiltinTransfer` is true.
+ TransferOptions BuiltinTransferOptions;
};
/// Type-erased lattice element container.
@@ -90,6 +94,11 @@
/// Determines whether to apply the built-in transfer functions, which model
/// the heap and stack in the `Environment`.
bool applyBuiltinTransfer() const { return Options.ApplyBuiltinTransfer; }
+
+ /// Returns the options to be passed to the built-in transfer functions.
+ TransferOptions builtinTransferOptions() const {
+ return Options.BuiltinTransferOptions;
+ }
};
/// Type-erased model of the program at a given program point.
Index: clang/include/clang/Analysis/FlowSensitive/Transfer.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -20,6 +20,12 @@
namespace clang {
namespace dataflow {
+struct TransferOptions {
+ /// Determines whether to analyze function bodies when present in the
+ /// translation unit.
+ bool ContextSensitive = false;
+};
+
/// Maps statements to the environments of basic blocks that contain them.
class StmtToEnvMap {
public:
@@ -36,7 +42,8 @@
/// Requirements:
///
/// `S` must not be `ParenExpr` or `ExprWithCleanups`.
-void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);
+void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env,
+ TransferOptions Options);
} // 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
@@ -129,6 +129,21 @@
/// with a symbolic representation of the `this` pointee.
Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx);
+ /// Creates and returns an environment to use for an inline analysis of the
+ /// callee. Uses the storage location from each argument in the `Call` as the
+ /// storage location for the corresponding parameter in the callee.
+ ///
+ /// Requirements:
+ ///
+ /// The callee of `Call` must be a `FunctionDecl` with a body.
+ ///
+ /// The body of the callee must not reference globals.
+ ///
+ /// The arguments of `Call` must map 1:1 to the callee's parameters.
+ ///
+ /// Each argument of `Call` must already have a `StorageLocation`.
+ Environment pushCall(const CallExpr *Call) const;
+
/// Returns true if and only if the environment is equivalent to `Other`, i.e
/// the two environments:
/// - have the same mappings from declarations to storage locations,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
