samestep updated this revision to Diff 449958.
samestep added a comment.
Don't assert returns to not be references
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130600/new/
https://reviews.llvm.org/D130600
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.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
@@ -4121,4 +4121,112 @@
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
+TEST(TransferTest, ContextSensitiveReturnVoid) {
+ std::string Code = R"(
+ void Noop() { return; }
+
+ void target() {
+ Noop();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ // This just tests that the analysis doesn't crash.
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnTrue) {
+ std::string Code = R"(
+ bool GiveBool() { return true; }
+
+ void target() {
+ bool Foo = GiveBool();
+ // [[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, ContextSensitiveReturnFalse) {
+ std::string Code = R"(
+ bool GiveBool() { return false; }
+
+ void target() {
+ bool Foo = GiveBool();
+ // [[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}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnArg) {
+ std::string Code = R"(
+ bool GiveBool();
+ bool GiveBack(bool Arg) { return Arg; }
+
+ void target() {
+ bool Foo = GiveBool();
+ bool Bar = GiveBack(Foo);
+ bool Baz = Foo == Bar;
+ // [[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 *BazDecl = findValueDecl(ASTCtx, "Baz");
+ ASSERT_THAT(BazDecl, NotNull());
+
+ auto &BazVal =
+ *cast<BoolValue>(Env.getValue(*BazDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(BazVal));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -332,6 +332,25 @@
std::make_unique<PointerValue>(*ThisPointeeLoc)));
}
+ void VisitReturnStmt(const ReturnStmt *S) {
+ auto *Ret = S->getRetValue();
+ if (Ret == nullptr)
+ return;
+
+ auto *Val = Env.getValue(*Ret, SkipPast::None);
+ if (Val == nullptr)
+ return;
+
+ // FIXME: Support reference-type returns.
+ if (Val->getKind() == Value::Kind::Reference)
+ return;
+
+ auto *Loc = Env.getReturnStorageLocation();
+ assert(Loc != nullptr);
+ // FIXME: Model NRVO.
+ Env.setValue(*Loc, *Val);
+ }
+
void VisitMemberExpr(const MemberExpr *S) {
ValueDecl *Member = S->getMemberDecl();
assert(Member != nullptr);
@@ -540,7 +559,11 @@
auto ExitState = (*BlockToOutputState)[ExitBlock];
assert(ExitState);
- Env.popCall(ExitState->Env);
+ auto &ExitEnv = ExitState->Env;
+ auto *ReturnLoc = ExitEnv.getReturnStorageLocation();
+ assert(ReturnLoc != nullptr);
+ Env.setStorageLocation(*S, *ReturnLoc);
+ Env.popCall(ExitEnv);
}
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,9 +154,9 @@
: DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
Environment::Environment(const Environment &Other)
- : DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
- ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
- MemberLocToStruct(Other.MemberLocToStruct),
+ : DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+ DeclToLoc(Other.DeclToLoc), ExprToLoc(Other.ExprToLoc),
+ LocToVal(Other.LocToVal), MemberLocToStruct(Other.MemberLocToStruct),
FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
}
@@ -179,6 +179,9 @@
if (Value *ParamVal = createValue(ParamDecl->getType()))
setValue(ParamLoc, *ParamVal);
}
+
+ QualType ReturnType = FuncDecl->getReturnType();
+ ReturnLoc = &createStorageLocation(ReturnType);
}
if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(&DeclCtx)) {
@@ -209,6 +212,9 @@
// FIXME: In order to allow the callee to reference globals, we probably need
// to call `initGlobalVars` here in some way.
+ QualType ReturnType = FuncDecl->getReturnType();
+ Env.ReturnLoc = &Env.createStorageLocation(ReturnType);
+
auto ParamIt = FuncDecl->param_begin();
auto ArgIt = Call->arg_begin();
auto ArgEnd = Call->arg_end();
@@ -241,12 +247,13 @@
}
void Environment::popCall(const Environment &CalleeEnv) {
- // We ignore `DACtx` because it's already the same in both. We don't bring
- // back `DeclToLoc` and `ExprToLoc` because we want to be able to later
- // analyze the same callee in a different context, and `setStorageLocation`
- // requires there to not already be a storage location assigned. Conceptually,
- // these maps capture information from the local scope, so when popping that
- // scope, we do not propagate the maps.
+ // We ignore `DACtx` because it's already the same in both. We don't want the
+ // callee's `ReturnLoc`. We don't bring back `DeclToLoc` and `ExprToLoc`
+ // because we want to be able to later analyze the same callee in a different
+ // context, and `setStorageLocation` requires there to not already be a
+ // storage location assigned. Conceptually, these maps capture information
+ // from the local scope, so when popping that scope, we do not propagate the
+ // maps.
this->LocToVal = std::move(CalleeEnv.LocToVal);
this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
@@ -256,6 +263,9 @@
Environment::ValueModel &Model) const {
assert(DACtx == Other.DACtx);
+ if (ReturnLoc != Other.ReturnLoc)
+ return false;
+
if (DeclToLoc != Other.DeclToLoc)
return false;
@@ -285,11 +295,14 @@
LatticeJoinEffect Environment::join(const Environment &Other,
Environment::ValueModel &Model) {
assert(DACtx == Other.DACtx);
+ assert(ReturnLoc == Other.ReturnLoc);
auto Effect = LatticeJoinEffect::Unchanged;
Environment JoinedEnv(*DACtx);
+ JoinedEnv.ReturnLoc = ReturnLoc;
+
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
Effect = LatticeJoinEffect::Changed;
@@ -382,6 +395,10 @@
return DACtx->getThisPointeeStorageLocation();
}
+StorageLocation *Environment::getReturnStorageLocation() const {
+ return ReturnLoc;
+}
+
PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
return DACtx->getOrCreateNullPointerValue(PointeeType);
}
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -222,6 +222,9 @@
/// in the environment.
StorageLocation *getThisPointeeStorageLocation() const;
+ /// Returns the storage location of the return value or null, if unset.
+ StorageLocation *getReturnStorageLocation() const;
+
/// Returns a pointer value that represents a null pointer. Calls with
/// `PointeeType` that are canonically equivalent will return the same result.
PointerValue &getOrCreateNullPointerValue(QualType PointeeType);
@@ -374,6 +377,11 @@
// `DACtx` is not null and not owned by this object.
DataflowAnalysisContext *DACtx;
+ // In a properly initialized `Environment`, `ReturnLoc` should only be null if
+ // its `DeclContext` could not be cast to a `FunctionDecl`.
+ StorageLocation *ReturnLoc = nullptr;
+ // FIXME: Move `ThisPointeeLoc` here from `DataflowAnalysisContext`.
+
// Maps from program declarations and statements to storage locations that are
// assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
// include only storage locations that are in scope for a particular basic
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -324,6 +324,7 @@
llvm::DenseMap<const ValueDecl *, StorageLocation *> DeclToLoc;
llvm::DenseMap<const Expr *, StorageLocation *> ExprToLoc;
+ // FIXME: Move this into `Environment`.
StorageLocation *ThisPointeeLoc = nullptr;
// Null pointer values, keyed by the canonical pointee type.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
