mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
To model the fact that values get deleted by a delete expression, this also
necessitates adding `DataflowEnvironment::unsetValue(StorageLocation &)` and
`StructValue::unsetValue(ValueDecl &)`.
Modeling object deletion in this way isn't perfect. In the framework, a value
that isn't present typically simply means "we didn't compute a value for this
storage location or struct field". Still, removing the values still seems better
than leaving them in place.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147698
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Value.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
@@ -5170,4 +5170,94 @@
});
}
+TEST(TransferTest, NewAndDeleteExpressions) {
+ std::string Code = R"(
+ void target() {
+ int *p = new int(42);
+ // [[after_new]]
+ delete p;
+ // [[after_delete]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &EnvAfterNew =
+ getEnvironmentAtAnnotation(Results, "after_new");
+ const Environment &EnvAfterDelete =
+ getEnvironmentAtAnnotation(Results, "after_delete");
+
+ auto &PAfterNew =
+ getValueForDecl<PointerValue>(ASTCtx, EnvAfterNew, "p");
+ auto &PAfterDelete =
+ getValueForDecl<PointerValue>(ASTCtx, EnvAfterNew, "p");
+
+ EXPECT_EQ(&PAfterNew, &PAfterDelete);
+
+ EXPECT_THAT(EnvAfterNew.getValue(PAfterNew.getPointeeLoc()), NotNull());
+ EXPECT_THAT(EnvAfterDelete.getValue(PAfterDelete.getPointeeLoc()),
+ IsNull());
+ });
+}
+
+TEST(TransferTest, NewAndDeleteExpressions_Structs) {
+ std::string Code = R"(
+ struct Inner {
+ int InnerField;
+ };
+
+ struct Outer {
+ Inner OuterField;
+ };
+
+ void target() {
+ Outer *p = new Outer;
+ // Access the fields to make sure the analysis actually generates children
+ // for them in the `AggregateStorageLoc` and `StructValue`.
+ p->OuterField.InnerField;
+ // [[after_new]]
+ delete p;
+ // [[after_delete]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &EnvAfterNew =
+ getEnvironmentAtAnnotation(Results, "after_new");
+ const Environment &EnvAfterDelete =
+ getEnvironmentAtAnnotation(Results, "after_delete");
+
+ const ValueDecl *OuterField = findValueDecl(ASTCtx, "OuterField");
+ const ValueDecl *InnerField = findValueDecl(ASTCtx, "InnerField");
+
+ auto &PAfterNew =
+ getValueForDecl<PointerValue>(ASTCtx, EnvAfterNew, "p");
+ auto &PAfterDelete =
+ getValueForDecl<PointerValue>(ASTCtx, EnvAfterNew, "p");
+
+ EXPECT_EQ(&PAfterNew, &PAfterDelete);
+
+ // Storage locations always exist. (They _can't_ be different because
+ // PAfterNew is identical to PAfterDelete.)
+ auto &OuterLoc =
+ cast<AggregateStorageLocation>(PAfterNew.getPointeeLoc());
+ auto &OuterFieldLoc =
+ cast<AggregateStorageLocation>(OuterLoc.getChild(*OuterField));
+ auto &InnerFieldLoc = OuterFieldLoc.getChild(*InnerField);
+
+ // Values for the struct and all fields exist after the new.
+ EXPECT_THAT(EnvAfterNew.getValue(OuterLoc), NotNull());
+ EXPECT_THAT(EnvAfterNew.getValue(OuterFieldLoc), NotNull());
+ EXPECT_THAT(EnvAfterNew.getValue(InnerFieldLoc), NotNull());
+
+ // But the values no longer exist after the delete.
+ EXPECT_THAT(EnvAfterDelete.getValue(OuterLoc), IsNull());
+ EXPECT_THAT(EnvAfterDelete.getValue(OuterFieldLoc), IsNull());
+ EXPECT_THAT(EnvAfterDelete.getValue(InnerFieldLoc), IsNull());
+ });
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -469,6 +469,27 @@
Env.setValue(Loc, Env.create<PointerValue>(*ThisPointeeLoc));
}
+ void VisitCXXNewExpr(const CXXNewExpr *S) {
+ auto &Loc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, Loc);
+ if (Value *Val = Env.createValue(S->getType()))
+ Env.setValue(Loc, *Val);
+ }
+
+ void VisitCXXDeleteExpr(const CXXDeleteExpr *S) {
+ const auto *PointerVal = cast_or_null<PointerValue>(
+ Env.getValue(*S->getArgument(), SkipPast::Reference));
+ if (PointerVal == nullptr)
+ return;
+
+ Env.unsetValue(PointerVal->getPointeeLoc());
+
+ // We consciously don't destroy the `PointerValue` itself because some
+ // analyses may want to be able to continue to use it after a `delete`.
+ // For example, an analysis that diagnoses double deletes may want to attach
+ // properties to the `PointerValue` and access those after the first delete.
+ }
+
void VisitReturnStmt(const ReturnStmt *S) {
if (!Env.getAnalysisOptions().ContextSensitiveOpts)
return;
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -634,6 +634,46 @@
}
}
+void Environment::unsetValue(const StorageLocation &Loc) {
+ auto It = LocToVal.find(&Loc);
+
+ if (It == LocToVal.end()) {
+ assert(false);
+ return;
+ }
+
+ Value &Val = *It->second;
+
+ LocToVal.erase(It);
+
+ if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
+ auto &AggregateLoc = *cast<AggregateStorageLocation>(&Loc);
+
+ const QualType Type = AggregateLoc.getType();
+ assert(Type->isRecordType());
+
+ for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
+ assert(Field != nullptr);
+ StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
+ MemberLocToStruct.erase(&FieldLoc);
+ if (auto *FieldVal = StructVal->getChild(*Field))
+ unsetValue(FieldLoc);
+ }
+ }
+
+ auto MemberLocToStructIt = MemberLocToStruct.find(&Loc);
+ if (MemberLocToStructIt != MemberLocToStruct.end()) {
+ // `Loc` is the location of a struct member so we need to also unset the
+ // value of the member in the corresponding `StructValue`.
+
+ auto [StructVal, Member] = MemberLocToStructIt->second;
+ assert(StructVal != nullptr);
+ assert(Member != nullptr);
+
+ StructVal->unsetChild(*Member);
+ }
+}
+
Value *Environment::getValue(const StorageLocation &Loc) const {
auto It = LocToVal.find(&Loc);
return It == LocToVal.end() ? nullptr : It->second;
Index: clang/include/clang/Analysis/FlowSensitive/Value.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Value.h
+++ clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -307,6 +307,20 @@
/// Assigns `Val` as the child value for `D`.
void setChild(const ValueDecl &D, Value &Val) { Children[&D] = &Val; }
+ /// Removes the association between `D` and its current child value.
+ ///
+ /// Requirements:
+ ///
+ /// `D` must currently be associated with a value.
+ void unsetChild(const ValueDecl &D) {
+ auto It = Children.find(&D);
+ if (It == Children.end()) {
+ assert(false);
+ return;
+ }
+ Children.erase(It);
+ }
+
private:
llvm::DenseMap<const ValueDecl *, Value *> Children;
};
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -307,6 +307,13 @@
/// Assigns `Val` as the value of `Loc` in the environment.
void setValue(const StorageLocation &Loc, Value &Val);
+ /// Removes the association between `Loc` and its current value.
+ ///
+ /// Requirements:
+ ///
+ /// There must be a value currently associated with `Loc`.
+ void unsetValue(const StorageLocation &Loc);
+
/// Returns the value assigned to `Loc` in the environment or null if `Loc`
/// isn't assigned a value in the environment.
Value *getValue(const StorageLocation &Loc) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
