https://github.com/bazuzi updated
https://github.com/llvm/llvm-project/pull/91616
>From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Thu, 9 May 2024 12:00:06 -0400
Subject: [PATCH 1/7] [clang][dataflow] Fully support Environment construction
for Stmt analysis.
Assume in fewer places that the analysis is of a FunctionDecl, and initialize
the Environment properly for Stmts.
Moves constructors for Environment to header to make it more obvious
that there are only minor differences between them and very little
initialization in the constructors.
Tested check-clang-tooling.
---
.../FlowSensitive/DataflowEnvironment.h | 133 ++++++++++++------
.../FlowSensitive/DataflowEnvironment.cpp | 113 +++++++--------
.../TypeErasedDataflowAnalysis.cpp | 2 +-
.../FlowSensitive/DataflowEnvironmentTest.cpp | 33 +++++
.../Analysis/FlowSensitive/TestingSupport.h | 4 +-
5 files changed, 176 insertions(+), 109 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index cdf89c7def2c9..dfc3f6a35e4e0 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -19,6 +19,7 @@
#include "clang/AST/DeclBase.h"
#include "clang/AST/Expr.h"
#include "clang/AST/Type.h"
+#include "clang/Analysis/FlowSensitive/ASTOps.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/Formula.h"
@@ -28,8 +29,10 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/PointerUnion.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorHandling.h"
+#include <cassert>
#include <memory>
#include <type_traits>
#include <utility>
@@ -155,7 +158,9 @@ class Environment {
/// Creates an environment that uses `DACtx` to store objects that encompass
/// the state of a program.
- explicit Environment(DataflowAnalysisContext &DACtx);
+ explicit Environment(DataflowAnalysisContext &DACtx)
+ : DACtx(&DACtx),
+ FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {}
// Copy-constructor is private, Environments should not be copied. See
fork().
Environment &operator=(const Environment &Other) = delete;
@@ -164,23 +169,25 @@ class Environment {
Environment &operator=(Environment &&Other) = default;
/// Creates an environment that uses `DACtx` to store objects that encompass
- /// the state of a program.
- ///
- /// If `DeclCtx` is a function, initializes the environment with symbolic
- /// representations of the function parameters.
- ///
- /// If `DeclCtx` is a non-static member function, initializes the environment
- /// with a symbolic representation of the `this` pointee.
- Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx);
+ /// the state of a program, with `FD` as the current analysis target.
+ Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD)
+ : Environment(DACtx) {
+ TargetStack.push_back(&FD);
+ }
+
+ /// Creates an environment that uses `DACtx` to store objects that encompass
+ /// the state of a program, with `S` as the current analysis target.
+ Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) {
+ TargetStack.push_back(&S);
+ }
/// Assigns storage locations and values to all parameters, captures, global
- /// variables, fields and functions referenced in the function currently
being
- /// analyzed.
+ /// variables, fields and functions referenced in the target currently
+ /// being analyzed.
///
- /// Requirements:
- ///
- /// The function must have a body, i.e.
- /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true.
+ /// If the current analysis target is a non-static member function,
+ /// initializes the environment with a symbolic representation of the `this`
+ /// pointee.
void initialize();
/// Returns a new environment that is a copy of this one.
@@ -365,46 +372,51 @@ class Environment {
RecordStorageLocation &
getResultObjectLocation(const Expr &RecordPRValue) const;
- /// Returns the return value of the current function. This can be null if:
+ /// Returns the return value of the function currently being analyzed.
+ /// This can be null if:
/// - The function has a void return type
/// - No return value could be determined for the function, for example
/// because it calls a function without a body.
///
/// Requirements:
- /// The current function must have a non-reference return type.
+ /// The current analysis target must be a function and must have a
+ /// non-reference return type.
Value *getReturnValue() const {
assert(getCurrentFunc() != nullptr &&
!getCurrentFunc()->getReturnType()->isReferenceType());
return ReturnVal;
}
- /// Returns the storage location for the reference returned by the current
- /// function. This can be null if function doesn't return a single consistent
- /// reference.
+ /// Returns the storage location for the reference returned by the function
+ /// currently being analyzed. This can be null if the function doesn't return
+ /// a single consistent reference.
///
/// Requirements:
- /// The current function must have a reference return type.
+ /// The current analysis target must be a function and must have a reference
+ /// return type.
StorageLocation *getReturnStorageLocation() const {
assert(getCurrentFunc() != nullptr &&
getCurrentFunc()->getReturnType()->isReferenceType());
return ReturnLoc;
}
- /// Sets the return value of the current function.
+ /// Sets the return value of the function currently being analyzed.
///
/// Requirements:
- /// The current function must have a non-reference return type.
+ /// The current analysis target must be a function and must have a
+ /// non-reference return type.
void setReturnValue(Value *Val) {
assert(getCurrentFunc() != nullptr &&
!getCurrentFunc()->getReturnType()->isReferenceType());
ReturnVal = Val;
}
- /// Sets the storage location for the reference returned by the current
- /// function.
+ /// Sets the storage location for the reference returned by the function
+ /// currently being analyzed.
///
/// Requirements:
- /// The current function must have a reference return type.
+ /// The current analysis target must be a function and must have a reference
+ /// return type.
void setReturnStorageLocation(StorageLocation *Loc) {
assert(getCurrentFunc() != nullptr &&
getCurrentFunc()->getReturnType()->isReferenceType());
@@ -641,23 +653,25 @@ class Environment {
/// (or the flow condition is overly constraining) or if the solver times
out.
bool allows(const Formula &) const;
- /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
- /// returns null.
- const DeclContext *getDeclCtx() const { return CallStack.back(); }
+ /// Returns the current analysis target.
+ llvm::PointerUnion<const FunctionDecl *, Stmt *> getCurrentTarget() const {
+ return TargetStack.back();
+ }
/// Returns the function currently being analyzed, or null if the code being
/// analyzed isn't part of a function.
const FunctionDecl *getCurrentFunc() const {
- return dyn_cast<FunctionDecl>(getDeclCtx());
+ return getCurrentTarget().dyn_cast<const FunctionDecl *>();
}
- /// Returns the size of the call stack.
- size_t callStackSize() const { return CallStack.size(); }
+ /// Returns the size of the target stack. All but the first item in the stack
+ /// are expected to be function calls.
+ size_t targetStackSize() const { return TargetStack.size(); }
/// Returns whether this `Environment` can be extended to analyze the given
/// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and
a
/// given `MaxDepth`.
- bool canDescend(unsigned MaxDepth, const DeclContext *Callee) const;
+ bool canDescend(unsigned MaxDepth, const FunctionDecl *Callee) const;
/// Returns the `DataflowAnalysisContext` used by the environment.
DataflowAnalysisContext &getDataflowAnalysisContext() const { return *DACtx;
}
@@ -718,9 +732,39 @@ class Environment {
void pushCallInternal(const FunctionDecl *FuncDecl,
ArrayRef<const Expr *> Args);
+ // FIXME: Add support for resetting globals after function calls to enable
+ // the implementation of sound analyses.
+
/// Assigns storage locations and values to all global variables, fields
- /// and functions referenced in `FuncDecl`. `FuncDecl` must have a body.
- void initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl);
+ /// and functions referenced in `Source`.
+ template <typename FuncOrStmt>
+ void initFieldsGlobalsAndFuncs(const FuncOrStmt *Source) {
+
+ ReferencedDecls Referenced = getReferencedDecls(*Source);
+
+ // These have to be added before the lines that follow to ensure that
+ // `create*` work correctly for structs.
+ DACtx->addModeledFields(Referenced.Fields);
+
+ for (const VarDecl *D : Referenced.Globals) {
+ if (getStorageLocation(*D) != nullptr)
+ continue;
+
+ // We don't run transfer functions on the initializers of global
+ // variables, so they won't be associated with a value or storage
+ // location. We therefore intentionally don't pass an initializer to
+ // `createObject()`; in particular, this ensures that `createObject()`
+ // will initialize the fields of record-type variables with values.
+ setStorageLocation(*D, createObject(*D, nullptr));
+ }
+
+ for (const FunctionDecl *FD : Referenced.Functions) {
+ if (getStorageLocation(*FD) != nullptr)
+ continue;
+ auto &Loc = createStorageLocation(*FD);
+ setStorageLocation(*FD, Loc);
+ }
+ }
static PrValueToResultObject
buildResultObjectMap(DataflowAnalysisContext *DACtx,
@@ -728,19 +772,25 @@ class Environment {
RecordStorageLocation *ThisPointeeLoc,
RecordStorageLocation *LocForRecordReturnVal);
+ static PrValueToResultObject
+ buildResultObjectMap(DataflowAnalysisContext *DACtx, Stmt *S,
+ RecordStorageLocation *ThisPointeeLoc,
+ RecordStorageLocation *LocForRecordReturnVal);
+
// `DACtx` is not null and not owned by this object.
DataflowAnalysisContext *DACtx;
- // FIXME: move the fields `CallStack`, `ResultObjectMap`, `ReturnVal`,
+ // FIXME: move the fields `TargetStack`, `ResultObjectMap`, `ReturnVal`,
// `ReturnLoc` and `ThisPointeeLoc` into a separate call-context object,
// shared between environments in the same call.
// https://github.com/llvm/llvm-project/issues/59005
- // `DeclContext` of the block being analysed if provided.
- std::vector<const DeclContext *> CallStack;
+ // The stack of statements or functions being analyzed. Only the first item
in
+ // the stack is expected to ever be a `Stmt *`.
+ std::vector<llvm::PointerUnion<const FunctionDecl *, Stmt *>> TargetStack;
// Maps from prvalues of record type to their result objects. Shared between
- // all environments for the same function.
+ // all environments for the same analysis target.
// FIXME: It's somewhat unsatisfactory that we have to use a `shared_ptr`
// here, though the cost is acceptable: The overhead of a `shared_ptr` is
// incurred when it is copied, and this happens only relatively rarely (when
@@ -749,7 +799,7 @@ class Environment {
std::shared_ptr<PrValueToResultObject> ResultObjectMap;
// The following three member variables handle various different types of
- // return values.
+ // return values when the current analysis target is a function.
// - If the return type is not a reference and not a record: Value returned
// by the function.
Value *ReturnVal = nullptr;
@@ -762,7 +812,8 @@ class Environment {
RecordStorageLocation *LocForRecordReturnVal = nullptr;
// The storage location of the `this` pointee. Should only be null if the
- // function being analyzed is only a function and not a method.
+ // analysis target is not a function or is a function but not a
+ // method.
RecordStorageLocation *ThisPointeeLoc = nullptr;
// Maps from declarations and glvalue expression to storage locations that
are
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index cb6c8b2ef1072..41bdeaf0375ef 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -16,17 +16,22 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/FlowSensitive/ASTOps.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/ErrorHandling.h"
+#include <algorithm>
#include <cassert>
+#include <memory>
#include <utility>
#define DEBUG_TYPE "dataflow"
@@ -290,15 +295,15 @@ widenKeyToValueMap(const llvm::MapVector<Key, Value *>
&CurMap,
namespace {
// Visitor that builds a map from record prvalues to result objects.
-// This traverses the body of the function to be analyzed; for each result
-// object that it encounters, it propagates the storage location of the result
-// object to all record prvalues that can initialize it.
+// This traverses the statement to be analyzed; for each result object that it
+// encounters, it propagates the storage location of the result object to all
+// record prvalues that can initialize it.
class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
public:
// `ResultObjectMap` will be filled with a map from record prvalues to result
- // object. If the function being analyzed returns a record by value,
- // `LocForRecordReturnVal` is the location to which this record should be
- // written; otherwise, it is null.
+ // object. If the statement being analyzed is a function body and returns a
+ // record by value, `LocForRecordReturnVal` is the location to which this
+ // record should be written; otherwise, it is null.
explicit ResultObjectVisitor(
llvm::DenseMap<const Expr *, RecordStorageLocation *> &ResultObjectMap,
RecordStorageLocation *LocForRecordReturnVal,
@@ -514,25 +519,20 @@ class ResultObjectVisitor : public
RecursiveASTVisitor<ResultObjectVisitor> {
} // namespace
-Environment::Environment(DataflowAnalysisContext &DACtx)
- : DACtx(&DACtx),
- FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {}
-
-Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
- : Environment(DACtx) {
- CallStack.push_back(&DeclCtx);
-}
-
void Environment::initialize() {
- const DeclContext *DeclCtx = getDeclCtx();
- if (DeclCtx == nullptr)
+ llvm::PointerUnion<const FunctionDecl *, Stmt *> Target = getCurrentTarget();
+ if (Target.isNull())
return;
- const auto *FuncDecl = dyn_cast<FunctionDecl>(DeclCtx);
- if (FuncDecl == nullptr)
+ if (Stmt *S = Target.dyn_cast<Stmt *>()) {
+ initFieldsGlobalsAndFuncs(S);
+ ResultObjectMap =
+ std::make_shared<PrValueToResultObject>(buildResultObjectMap(
+ DACtx, S, getThisPointeeStorageLocation(), LocForRecordReturnVal));
return;
+ }
+ const auto *FuncDecl = Target.get<const FunctionDecl *>();
assert(FuncDecl->doesThisDeclarationHaveABody());
initFieldsGlobalsAndFuncs(FuncDecl);
@@ -546,7 +546,7 @@ void Environment::initialize() {
LocForRecordReturnVal = &cast<RecordStorageLocation>(
createStorageLocation(FuncDecl->getReturnType()));
- if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(DeclCtx)) {
+ if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl)) {
auto *Parent = MethodDecl->getParent();
assert(Parent != nullptr);
@@ -558,7 +558,7 @@ void Environment::initialize() {
setStorageLocation(*VarDecl, createObject(*VarDecl, nullptr));
} else if (Capture.capturesThis()) {
const auto *SurroundingMethodDecl =
- cast<CXXMethodDecl>(DeclCtx->getNonClosureAncestor());
+ cast<CXXMethodDecl>(FuncDecl->getNonClosureAncestor());
QualType ThisPointeeType =
SurroundingMethodDecl->getFunctionObjectParameterType();
setThisPointeeStorageLocation(
@@ -585,37 +585,6 @@ void Environment::initialize() {
LocForRecordReturnVal));
}
-// FIXME: Add support for resetting globals after function calls to enable
-// the implementation of sound analyses.
-void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
- assert(FuncDecl->doesThisDeclarationHaveABody());
-
- ReferencedDecls Referenced = getReferencedDecls(*FuncDecl);
-
- // These have to be added before the lines that follow to ensure that
- // `create*` work correctly for structs.
- DACtx->addModeledFields(Referenced.Fields);
-
- for (const VarDecl *D : Referenced.Globals) {
- if (getStorageLocation(*D) != nullptr)
- continue;
-
- // We don't run transfer functions on the initializers of global variables,
- // so they won't be associated with a value or storage location. We
- // therefore intentionally don't pass an initializer to `createObject()`;
- // in particular, this ensures that `createObject()` will initialize the
- // fields of record-type variables with values.
- setStorageLocation(*D, createObject(*D, nullptr));
- }
-
- for (const FunctionDecl *FD : Referenced.Functions) {
- if (getStorageLocation(*FD) != nullptr)
- continue;
- auto &Loc = createStorageLocation(*FD);
- setStorageLocation(*FD, Loc);
- }
-}
-
Environment Environment::fork() const {
Environment Copy(*this);
Copy.FlowConditionToken = DACtx->forkFlowCondition(FlowConditionToken);
@@ -623,8 +592,14 @@ Environment Environment::fork() const {
}
bool Environment::canDescend(unsigned MaxDepth,
- const DeclContext *Callee) const {
- return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack,
Callee);
+ const FunctionDecl *Callee) const {
+ return TargetStack.size() <= MaxDepth &&
+ !std::any_of(
+ TargetStack.begin(), TargetStack.end(),
+ [Callee](const llvm::PointerUnion<const FunctionDecl *, Stmt *>
+ &Target) {
+ return Callee == Target.dyn_cast<const FunctionDecl *>();
+ });
}
Environment Environment::pushCall(const CallExpr *Call) const {
@@ -669,7 +644,7 @@ void Environment::pushCallInternal(const FunctionDecl
*FuncDecl,
assert(FuncDecl->getDefinition() != nullptr);
FuncDecl = FuncDecl->getDefinition();
- CallStack.push_back(FuncDecl);
+ TargetStack.push_back(FuncDecl);
initFieldsGlobalsAndFuncs(FuncDecl);
@@ -753,7 +728,7 @@ LatticeEffect Environment::widen(const Environment &PrevEnv,
assert(ReturnLoc == PrevEnv.ReturnLoc);
assert(LocForRecordReturnVal == PrevEnv.LocForRecordReturnVal);
assert(ThisPointeeLoc == PrevEnv.ThisPointeeLoc);
- assert(CallStack == PrevEnv.CallStack);
+ assert(TargetStack == PrevEnv.TargetStack);
assert(ResultObjectMap == PrevEnv.ResultObjectMap);
auto Effect = LatticeEffect::Unchanged;
@@ -788,22 +763,20 @@ Environment Environment::join(const Environment &EnvA,
const Environment &EnvB,
assert(EnvA.DACtx == EnvB.DACtx);
assert(EnvA.LocForRecordReturnVal == EnvB.LocForRecordReturnVal);
assert(EnvA.ThisPointeeLoc == EnvB.ThisPointeeLoc);
- assert(EnvA.CallStack == EnvB.CallStack);
+ assert(EnvA.TargetStack == EnvB.TargetStack);
assert(EnvA.ResultObjectMap == EnvB.ResultObjectMap);
Environment JoinedEnv(*EnvA.DACtx);
- JoinedEnv.CallStack = EnvA.CallStack;
+ JoinedEnv.TargetStack = EnvA.TargetStack;
JoinedEnv.ResultObjectMap = EnvA.ResultObjectMap;
JoinedEnv.LocForRecordReturnVal = EnvA.LocForRecordReturnVal;
JoinedEnv.ThisPointeeLoc = EnvA.ThisPointeeLoc;
- if (EnvA.CallStack.empty()) {
+ if (EnvA.TargetStack.empty()) {
JoinedEnv.ReturnVal = nullptr;
} else {
- // FIXME: Make `CallStack` a vector of `FunctionDecl` so we don't need this
- // cast.
- auto *Func = dyn_cast<FunctionDecl>(EnvA.CallStack.back());
+ auto *Func = EnvA.getCurrentFunc();
assert(Func != nullptr);
JoinedEnv.ReturnVal =
joinValues(Func->getReturnType(), EnvA.ReturnVal, EnvA, EnvB.ReturnVal,
@@ -1229,16 +1202,26 @@ Environment::PrValueToResultObject
Environment::buildResultObjectMap(
RecordStorageLocation *LocForRecordReturnVal) {
assert(FuncDecl->doesThisDeclarationHaveABody());
- PrValueToResultObject Map;
+ PrValueToResultObject Map = buildResultObjectMap(
+ DACtx, FuncDecl->getBody(), ThisPointeeLoc, LocForRecordReturnVal);
ResultObjectVisitor Visitor(Map, LocForRecordReturnVal, *DACtx);
if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FuncDecl))
Visitor.TraverseConstructorInits(Ctor, ThisPointeeLoc);
- Visitor.TraverseStmt(FuncDecl->getBody());
return Map;
}
+Environment::PrValueToResultObject Environment::buildResultObjectMap(
+ DataflowAnalysisContext *DACtx, Stmt *S,
+ RecordStorageLocation *ThisPointeeLoc,
+ RecordStorageLocation *LocForRecordReturnVal) {
+ PrValueToResultObject Map;
+ ResultObjectVisitor Visitor(Map, LocForRecordReturnVal, *DACtx);
+ Visitor.TraverseStmt(S);
+ return Map;
+}
+
RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
const Environment &Env) {
Expr *ImplicitObject = MCE.getImplicitObjectArgument();
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 12eff4dd4b781..254007eb80725 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -476,7 +476,7 @@ runTypeErasedDataflowAnalysis(
PrettyStackTraceAnalysis CrashInfo(ACFG, "runTypeErasedDataflowAnalysis");
std::optional<Environment> MaybeStartingEnv;
- if (InitEnv.callStackSize() == 1) {
+ if (InitEnv.targetStackSize() == 1) {
MaybeStartingEnv = InitEnv.fork();
MaybeStartingEnv->initialize();
}
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index 4195648161246..ef0280e74072c 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -9,6 +9,8 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "TestingSupport.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/Stmt.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
@@ -403,4 +405,35 @@ TEST_F(EnvironmentTest,
Contains(Member));
}
+TEST_F(EnvironmentTest, Stmt) {
+ using namespace ast_matchers;
+
+ std::string Code = R"cc(
+ struct S {int i;};
+ void foo() {
+ S AnS = S{1};
+ }
+ )cc";
+ auto Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+ auto &Context = Unit->getASTContext();
+
+ ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+ auto *DeclStatement = const_cast<DeclStmt *>(
+ selectFirst<DeclStmt>("d", match(declStmt().bind("d"), Context)));
+ ASSERT_THAT(DeclStatement, NotNull());
+ auto *ConstructExpr = selectFirst<CXXConstructExpr>(
+ "c", match(cxxConstructExpr().bind("c"), Context));
+ ASSERT_THAT(ConstructExpr, NotNull());
+
+ // Verify that we can retrieve the result object location for the
construction
+ // expression when we analyze the DeclStmt for `AnS`.
+ Environment Env(DAContext, *DeclStatement);
+ // Don't crash when initializing.
+ Env.initialize();
+ // And don't crash when retrieving the result object location.
+ Env.getResultObjectLocation(*ConstructExpr);
+}
+
} // namespace
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 3b0e05ed72220..7348f8b1740d0 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -355,8 +355,8 @@ checkDataflow(AnalysisInputs<AnalysisT> AI,
auto SetupTest = [&StmtToAnnotations,
PrevSetupTest = std::move(AI.SetupTest)](
AnalysisOutputs &AO) -> llvm::Error {
- auto MaybeStmtToAnnotations = buildStatementToAnnotationMapping(
- cast<FunctionDecl>(AO.InitEnv.getDeclCtx()), AO.Code);
+ auto MaybeStmtToAnnotations =
+ buildStatementToAnnotationMapping(AO.InitEnv.getCurrentFunc(),
AO.Code);
if (!MaybeStmtToAnnotations) {
return MaybeStmtToAnnotations.takeError();
}
>From 6982564302fb3c9f524ae68c47fa5603d1216e09 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Thu, 9 May 2024 18:01:07 -0400
Subject: [PATCH 2/7] Add test and crash fix for analysis of Stmt.
---
.../FlowSensitive/DataflowEnvironment.cpp | 8 +++----
.../TypeErasedDataflowAnalysisTest.cpp | 24 +++++++++++++++++++
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 41bdeaf0375ef..32547db4fd119 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -773,14 +773,12 @@ Environment Environment::join(const Environment &EnvA,
const Environment &EnvB,
JoinedEnv.LocForRecordReturnVal = EnvA.LocForRecordReturnVal;
JoinedEnv.ThisPointeeLoc = EnvA.ThisPointeeLoc;
- if (EnvA.TargetStack.empty()) {
+ if (EnvA.TargetStack.empty() || !EnvA.getCurrentFunc()) {
JoinedEnv.ReturnVal = nullptr;
} else {
- auto *Func = EnvA.getCurrentFunc();
- assert(Func != nullptr);
JoinedEnv.ReturnVal =
- joinValues(Func->getReturnType(), EnvA.ReturnVal, EnvA, EnvB.ReturnVal,
- EnvB, JoinedEnv, Model);
+ joinValues(EnvA.getCurrentFunc()->getReturnType(), EnvA.ReturnVal,
EnvA,
+ EnvB.ReturnVal, EnvB, JoinedEnv, Model);
}
if (EnvA.ReturnLoc == EnvB.ReturnLoc)
diff --git
a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index b0b579d2bc19e..2ddf0a6f66fd4 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -146,6 +146,30 @@ TEST_F(DataflowAnalysisTest,
DiagnoseFunctionDiagnoserCalledOnEachElement) {
" (Lifetime ends)\n")));
}
+TEST_F(DataflowAnalysisTest, CanAnalyzeStmt) {
+ std::string Code = R"cc(
+ struct S {int i;};
+ S getAnS() {return S{1};};
+ void foo() {
+ S AnS = getAnS();
+ }
+ )cc";
+ AST = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"});
+ const auto &DeclStatement = matchNode<DeclStmt>(declStmt());
+ const auto &Func = matchNode<FunctionDecl>(functionDecl(hasName("foo")));
+
+ ACFG = std::make_unique<AdornedCFG>(llvm::cantFail(AdornedCFG::build(
+ Func, const_cast<DeclStmt &>(DeclStatement), AST->getASTContext())));
+
+ NoopAnalysis Analysis = NoopAnalysis(AST->getASTContext());
+ DACtx = std::make_unique<DataflowAnalysisContext>(
+ std::make_unique<WatchedLiteralsSolver>());
+ Environment Env(*DACtx, const_cast<DeclStmt &>(DeclStatement));
+
+ ASSERT_THAT_ERROR(runDataflowAnalysis(*ACFG, Analysis, Env).takeError(),
+ llvm::Succeeded());
+}
+
// Tests for the statement-to-block map.
using StmtToBlockTest = DataflowAnalysisTest;
>From 112e23c593b184f24f0a4c34755f4c183a1a1f08 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Fri, 10 May 2024 11:00:17 -0400
Subject: [PATCH 3/7] Store the initial analysis target outside of the
CallStack.
Reverts TargetStack to CallStack and stores the initial target statement
and, when present, its containing FunctionDecl separately from
CallStack.
---
.../FlowSensitive/DataflowEnvironment.h | 57 +++++++++--------
.../FlowSensitive/DataflowEnvironment.cpp | 61 +++++++++----------
.../TypeErasedDataflowAnalysis.cpp | 2 +-
3 files changed, 62 insertions(+), 58 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index dfc3f6a35e4e0..83ce1fa47b586 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -29,13 +29,13 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/MapVector.h"
-#include "llvm/ADT/PointerUnion.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorHandling.h"
#include <cassert>
#include <memory>
#include <type_traits>
#include <utility>
+#include <vector>
namespace clang {
namespace dataflow {
@@ -169,25 +169,28 @@ class Environment {
Environment &operator=(Environment &&Other) = default;
/// Creates an environment that uses `DACtx` to store objects that encompass
- /// the state of a program, with `FD` as the current analysis target.
- Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD)
- : Environment(DACtx) {
- TargetStack.push_back(&FD);
+ /// the state of a program, with `S` as the initial analysis target.
+ Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) {
+ InitialTargetStmt = &S;
}
/// Creates an environment that uses `DACtx` to store objects that encompass
- /// the state of a program, with `S` as the current analysis target.
- Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) {
- TargetStack.push_back(&S);
+ /// the state of a program, with `FD` as the initial analysis target.
+ ///
+ /// Requirements:
+ ///
+ /// The function must have a body, i.e.
+ /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true.
+ Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD)
+ : Environment(DACtx, *FD.getBody()) {
+ InitialTargetFunc = &FD;
}
/// Assigns storage locations and values to all parameters, captures, global
- /// variables, fields and functions referenced in the target currently
- /// being analyzed.
+ /// variables, fields and functions referenced in the initial analysis
target.
///
- /// If the current analysis target is a non-static member function,
- /// initializes the environment with a symbolic representation of the `this`
- /// pointee.
+ /// If the target is a non-static member function, initializes the
environment
+ /// with a symbolic representation of the `this` pointee.
void initialize();
/// Returns a new environment that is a copy of this one.
@@ -200,7 +203,7 @@ class Environment {
/// forked flow condition references the original).
Environment fork() const;
- /// Creates and returns an environment to use for an inline analysis of the
+ /// 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.
///
@@ -653,20 +656,14 @@ class Environment {
/// (or the flow condition is overly constraining) or if the solver times
out.
bool allows(const Formula &) const;
- /// Returns the current analysis target.
- llvm::PointerUnion<const FunctionDecl *, Stmt *> getCurrentTarget() const {
- return TargetStack.back();
- }
-
/// Returns the function currently being analyzed, or null if the code being
/// analyzed isn't part of a function.
const FunctionDecl *getCurrentFunc() const {
- return getCurrentTarget().dyn_cast<const FunctionDecl *>();
+ return CallStack.empty() ? InitialTargetFunc : CallStack.back();
}
- /// Returns the size of the target stack. All but the first item in the stack
- /// are expected to be function calls.
- size_t targetStackSize() const { return TargetStack.size(); }
+ /// Returns the size of the call stack.
+ size_t callStackSize() const { return CallStack.size(); }
/// Returns whether this `Environment` can be extended to analyze the given
/// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and
a
@@ -785,9 +782,17 @@ class Environment {
// shared between environments in the same call.
// https://github.com/llvm/llvm-project/issues/59005
- // The stack of statements or functions being analyzed. Only the first item
in
- // the stack is expected to ever be a `Stmt *`.
- std::vector<llvm::PointerUnion<const FunctionDecl *, Stmt *>> TargetStack;
+ // The stack of functions called from the initial analysis target and
+ // recursively being analyzed.
+ std::vector<const FunctionDecl *> CallStack;
+
+ // If the initial analysis target is a function, this is the function
+ // declaration. Otherwise, this is null.
+ const FunctionDecl *InitialTargetFunc = nullptr;
+ // If the initial analysis target is a function, this is its body. If the
+ // initial analysis target was not provided, this is null. Otherwise, this is
+ // the initial analysis target.
+ Stmt *InitialTargetStmt = nullptr;
// Maps from prvalues of record type to their result objects. Shared between
// all environments for the same analysis target.
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 32547db4fd119..8c2d5dc2f2f01 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -520,33 +520,30 @@ class ResultObjectVisitor : public
RecursiveASTVisitor<ResultObjectVisitor> {
} // namespace
void Environment::initialize() {
- llvm::PointerUnion<const FunctionDecl *, Stmt *> Target = getCurrentTarget();
- if (Target.isNull())
+ if (InitialTargetStmt == nullptr)
return;
- if (Stmt *S = Target.dyn_cast<Stmt *>()) {
- initFieldsGlobalsAndFuncs(S);
+ if (InitialTargetFunc == nullptr) {
+ initFieldsGlobalsAndFuncs(InitialTargetStmt);
ResultObjectMap =
std::make_shared<PrValueToResultObject>(buildResultObjectMap(
- DACtx, S, getThisPointeeStorageLocation(), LocForRecordReturnVal));
+ DACtx, InitialTargetStmt, getThisPointeeStorageLocation(),
+ LocForRecordReturnVal));
return;
}
- const auto *FuncDecl = Target.get<const FunctionDecl *>();
- assert(FuncDecl->doesThisDeclarationHaveABody());
-
- initFieldsGlobalsAndFuncs(FuncDecl);
+ initFieldsGlobalsAndFuncs(InitialTargetFunc);
- for (const auto *ParamDecl : FuncDecl->parameters()) {
+ for (const auto *ParamDecl : InitialTargetFunc->parameters()) {
assert(ParamDecl != nullptr);
setStorageLocation(*ParamDecl, createObject(*ParamDecl, nullptr));
}
- if (FuncDecl->getReturnType()->isRecordType())
+ if (InitialTargetFunc->getReturnType()->isRecordType())
LocForRecordReturnVal = &cast<RecordStorageLocation>(
- createStorageLocation(FuncDecl->getReturnType()));
+ createStorageLocation(InitialTargetFunc->getReturnType()));
- if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl)) {
+ if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(InitialTargetFunc)) {
auto *Parent = MethodDecl->getParent();
assert(Parent != nullptr);
@@ -558,7 +555,7 @@ void Environment::initialize() {
setStorageLocation(*VarDecl, createObject(*VarDecl, nullptr));
} else if (Capture.capturesThis()) {
const auto *SurroundingMethodDecl =
- cast<CXXMethodDecl>(FuncDecl->getNonClosureAncestor());
+ cast<CXXMethodDecl>(InitialTargetFunc->getNonClosureAncestor());
QualType ThisPointeeType =
SurroundingMethodDecl->getFunctionObjectParameterType();
setThisPointeeStorageLocation(
@@ -580,9 +577,10 @@ void Environment::initialize() {
// We do this below the handling of `CXXMethodDecl` above so that we can
// be sure that the storage location for `this` has been set.
- ResultObjectMap = std::make_shared<PrValueToResultObject>(
- buildResultObjectMap(DACtx, FuncDecl, getThisPointeeStorageLocation(),
- LocForRecordReturnVal));
+ ResultObjectMap =
+ std::make_shared<PrValueToResultObject>(buildResultObjectMap(
+ DACtx, InitialTargetFunc, getThisPointeeStorageLocation(),
+ LocForRecordReturnVal));
}
Environment Environment::fork() const {
@@ -593,13 +591,7 @@ Environment Environment::fork() const {
bool Environment::canDescend(unsigned MaxDepth,
const FunctionDecl *Callee) const {
- return TargetStack.size() <= MaxDepth &&
- !std::any_of(
- TargetStack.begin(), TargetStack.end(),
- [Callee](const llvm::PointerUnion<const FunctionDecl *, Stmt *>
- &Target) {
- return Callee == Target.dyn_cast<const FunctionDecl *>();
- });
+ return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack,
Callee);
}
Environment Environment::pushCall(const CallExpr *Call) const {
@@ -644,7 +636,7 @@ void Environment::pushCallInternal(const FunctionDecl
*FuncDecl,
assert(FuncDecl->getDefinition() != nullptr);
FuncDecl = FuncDecl->getDefinition();
- TargetStack.push_back(FuncDecl);
+ CallStack.push_back(FuncDecl);
initFieldsGlobalsAndFuncs(FuncDecl);
@@ -728,8 +720,10 @@ LatticeEffect Environment::widen(const Environment
&PrevEnv,
assert(ReturnLoc == PrevEnv.ReturnLoc);
assert(LocForRecordReturnVal == PrevEnv.LocForRecordReturnVal);
assert(ThisPointeeLoc == PrevEnv.ThisPointeeLoc);
- assert(TargetStack == PrevEnv.TargetStack);
+ assert(CallStack == PrevEnv.CallStack);
assert(ResultObjectMap == PrevEnv.ResultObjectMap);
+ assert(InitialTargetFunc == PrevEnv.InitialTargetFunc);
+ assert(InitialTargetStmt == PrevEnv.InitialTargetStmt);
auto Effect = LatticeEffect::Unchanged;
@@ -763,22 +757,27 @@ Environment Environment::join(const Environment &EnvA,
const Environment &EnvB,
assert(EnvA.DACtx == EnvB.DACtx);
assert(EnvA.LocForRecordReturnVal == EnvB.LocForRecordReturnVal);
assert(EnvA.ThisPointeeLoc == EnvB.ThisPointeeLoc);
- assert(EnvA.TargetStack == EnvB.TargetStack);
+ assert(EnvA.CallStack == EnvB.CallStack);
assert(EnvA.ResultObjectMap == EnvB.ResultObjectMap);
+ assert(EnvA.InitialTargetFunc == EnvB.InitialTargetFunc);
+ assert(EnvA.InitialTargetStmt == EnvB.InitialTargetStmt);
Environment JoinedEnv(*EnvA.DACtx);
- JoinedEnv.TargetStack = EnvA.TargetStack;
+ JoinedEnv.CallStack = EnvA.CallStack;
JoinedEnv.ResultObjectMap = EnvA.ResultObjectMap;
JoinedEnv.LocForRecordReturnVal = EnvA.LocForRecordReturnVal;
JoinedEnv.ThisPointeeLoc = EnvA.ThisPointeeLoc;
+ JoinedEnv.InitialTargetFunc = EnvA.InitialTargetFunc;
+ JoinedEnv.InitialTargetStmt = EnvA.InitialTargetStmt;
- if (EnvA.TargetStack.empty() || !EnvA.getCurrentFunc()) {
+ auto *Func = EnvA.getCurrentFunc();
+ if (!Func) {
JoinedEnv.ReturnVal = nullptr;
} else {
JoinedEnv.ReturnVal =
- joinValues(EnvA.getCurrentFunc()->getReturnType(), EnvA.ReturnVal,
EnvA,
- EnvB.ReturnVal, EnvB, JoinedEnv, Model);
+ joinValues(Func->getReturnType(), EnvA.ReturnVal, EnvA, EnvB.ReturnVal,
+ EnvB, JoinedEnv, Model);
}
if (EnvA.ReturnLoc == EnvB.ReturnLoc)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 254007eb80725..b923bfbf78176 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -476,7 +476,7 @@ runTypeErasedDataflowAnalysis(
PrettyStackTraceAnalysis CrashInfo(ACFG, "runTypeErasedDataflowAnalysis");
std::optional<Environment> MaybeStartingEnv;
- if (InitEnv.targetStackSize() == 1) {
+ if (InitEnv.callStackSize() == 0 && InitEnv.getCurrentFunc()) {
MaybeStartingEnv = InitEnv.fork();
MaybeStartingEnv->initialize();
}
>From b526d217118ffdbaf98019070eaaa57e113e84aa Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Fri, 10 May 2024 12:01:05 -0400
Subject: [PATCH 4/7] Account for changes to stack size and initialize when
analyzing Stmts.
---
.../include/clang/Analysis/FlowSensitive/DataflowEnvironment.h | 3 ++-
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 2 +-
.../lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 83ce1fa47b586..b1a267125152e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -667,7 +667,8 @@ class Environment {
/// Returns whether this `Environment` can be extended to analyze the given
/// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and
a
- /// given `MaxDepth`.
+ /// given `MaxDepth`. Note that the `MaxDepth` does not count any initial
+ /// target function, which was not called.
bool canDescend(unsigned MaxDepth, const FunctionDecl *Callee) const;
/// Returns the `DataflowAnalysisContext` used by the environment.
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 8c2d5dc2f2f01..59eaf6db2f134 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -591,7 +591,7 @@ Environment Environment::fork() const {
bool Environment::canDescend(unsigned MaxDepth,
const FunctionDecl *Callee) const {
- return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack,
Callee);
+ return CallStack.size() < MaxDepth && !llvm::is_contained(CallStack, Callee);
}
Environment Environment::pushCall(const CallExpr *Call) const {
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index b923bfbf78176..675b42550f177 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -476,7 +476,7 @@ runTypeErasedDataflowAnalysis(
PrettyStackTraceAnalysis CrashInfo(ACFG, "runTypeErasedDataflowAnalysis");
std::optional<Environment> MaybeStartingEnv;
- if (InitEnv.callStackSize() == 0 && InitEnv.getCurrentFunc()) {
+ if (InitEnv.callStackSize() == 0) {
MaybeStartingEnv = InitEnv.fork();
MaybeStartingEnv->initialize();
}
>From e151dd46c2dfca627d21a3e0eb8c6a30ab257843 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Fri, 10 May 2024 12:45:10 -0400
Subject: [PATCH 5/7] Un-template initFieldsGlobalsAndFuncs and update
ResultObjectVisitor arg and comments.
---
.../FlowSensitive/DataflowEnvironment.h | 31 +------------
.../FlowSensitive/DataflowEnvironment.cpp | 43 +++++++++++++++----
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index b1a267125152e..5f46c2e11c620 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -734,35 +734,8 @@ class Environment {
// the implementation of sound analyses.
/// Assigns storage locations and values to all global variables, fields
- /// and functions referenced in `Source`.
- template <typename FuncOrStmt>
- void initFieldsGlobalsAndFuncs(const FuncOrStmt *Source) {
-
- ReferencedDecls Referenced = getReferencedDecls(*Source);
-
- // These have to be added before the lines that follow to ensure that
- // `create*` work correctly for structs.
- DACtx->addModeledFields(Referenced.Fields);
-
- for (const VarDecl *D : Referenced.Globals) {
- if (getStorageLocation(*D) != nullptr)
- continue;
-
- // We don't run transfer functions on the initializers of global
- // variables, so they won't be associated with a value or storage
- // location. We therefore intentionally don't pass an initializer to
- // `createObject()`; in particular, this ensures that `createObject()`
- // will initialize the fields of record-type variables with values.
- setStorageLocation(*D, createObject(*D, nullptr));
- }
-
- for (const FunctionDecl *FD : Referenced.Functions) {
- if (getStorageLocation(*FD) != nullptr)
- continue;
- auto &Loc = createStorageLocation(*FD);
- setStorageLocation(*FD, Loc);
- }
- }
+ /// and functions in `Referenced`.
+ void initFieldsGlobalsAndFuncs(const ReferencedDecls &Referenced);
static PrValueToResultObject
buildResultObjectMap(DataflowAnalysisContext *DACtx,
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 59eaf6db2f134..d1aec9eb3b147 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -295,15 +295,15 @@ widenKeyToValueMap(const llvm::MapVector<Key, Value *>
&CurMap,
namespace {
// Visitor that builds a map from record prvalues to result objects.
-// This traverses the statement to be analyzed; for each result object that it
-// encounters, it propagates the storage location of the result object to all
+// This traverses the AST sub-tree to be visited; for each result object that
+// it encounters, it propagates the storage location of the result object to
all
// record prvalues that can initialize it.
class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
public:
// `ResultObjectMap` will be filled with a map from record prvalues to result
- // object. If the statement being analyzed is a function body and returns a
- // record by value, `LocForRecordReturnVal` is the location to which this
- // record should be written; otherwise, it is null.
+ // object. If the sub-tree to be visited returns a record by value,
+ // `LocForRecordReturnVal` is the location to which this record should be
+ // written; otherwise, it is null.
explicit ResultObjectVisitor(
llvm::DenseMap<const Expr *, RecordStorageLocation *> &ResultObjectMap,
RecordStorageLocation *LocForRecordReturnVal,
@@ -524,15 +524,15 @@ void Environment::initialize() {
return;
if (InitialTargetFunc == nullptr) {
- initFieldsGlobalsAndFuncs(InitialTargetStmt);
+ initFieldsGlobalsAndFuncs(getReferencedDecls(*InitialTargetStmt));
ResultObjectMap =
std::make_shared<PrValueToResultObject>(buildResultObjectMap(
DACtx, InitialTargetStmt, getThisPointeeStorageLocation(),
- LocForRecordReturnVal));
+ /*LocForRecordReturnValue=*/nullptr));
return;
}
- initFieldsGlobalsAndFuncs(InitialTargetFunc);
+ initFieldsGlobalsAndFuncs(getReferencedDecls(*InitialTargetFunc));
for (const auto *ParamDecl : InitialTargetFunc->parameters()) {
assert(ParamDecl != nullptr);
@@ -583,6 +583,31 @@ void Environment::initialize() {
LocForRecordReturnVal));
}
+void Environment::initFieldsGlobalsAndFuncs(const ReferencedDecls &Referenced)
{
+ // These have to be added before the lines that follow to ensure that
+ // `create*` work correctly for structs.
+ DACtx->addModeledFields(Referenced.Fields);
+
+ for (const VarDecl *D : Referenced.Globals) {
+ if (getStorageLocation(*D) != nullptr)
+ continue;
+
+ // We don't run transfer functions on the initializers of global variables,
+ // so they won't be associated with a value or storage location. We
+ // therefore intentionally don't pass an initializer to `createObject()`;
in
+ // particular, this ensures that `createObject()` will initialize the
fields
+ // of record-type variables with values.
+ setStorageLocation(*D, createObject(*D, nullptr));
+ }
+
+ for (const FunctionDecl *FD : Referenced.Functions) {
+ if (getStorageLocation(*FD) != nullptr)
+ continue;
+ auto &Loc = createStorageLocation(*FD);
+ setStorageLocation(*FD, Loc);
+ }
+}
+
Environment Environment::fork() const {
Environment Copy(*this);
Copy.FlowConditionToken = DACtx->forkFlowCondition(FlowConditionToken);
@@ -638,7 +663,7 @@ void Environment::pushCallInternal(const FunctionDecl
*FuncDecl,
CallStack.push_back(FuncDecl);
- initFieldsGlobalsAndFuncs(FuncDecl);
+ initFieldsGlobalsAndFuncs(getReferencedDecls(*FuncDecl));
const auto *ParamIt = FuncDecl->param_begin();
>From a5880e606458afdca3f3ab634dcc829a54a96b46 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Fri, 10 May 2024 13:09:42 -0400
Subject: [PATCH 6/7] Put back the correct name of CallStack in comment.
---
.../include/clang/Analysis/FlowSensitive/DataflowEnvironment.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 5f46c2e11c620..c32d0b091e31c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -751,7 +751,7 @@ class Environment {
// `DACtx` is not null and not owned by this object.
DataflowAnalysisContext *DACtx;
- // FIXME: move the fields `TargetStack`, `ResultObjectMap`, `ReturnVal`,
+ // FIXME: move the fields `CallStack`, `ResultObjectMap`, `ReturnVal`,
// `ReturnLoc` and `ThisPointeeLoc` into a separate call-context object,
// shared between environments in the same call.
// https://github.com/llvm/llvm-project/issues/59005
>From 6ddaef73afc13f844653dca73067ba959a367fed Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <baz...@google.com>
Date: Wed, 15 May 2024 11:28:16 -0400
Subject: [PATCH 7/7] Respond to review comments.
Mostly improvements to tests and comments, plus an assertion and
explicit type.
---
.../FlowSensitive/DataflowEnvironment.h | 52 +++++++++----------
.../FlowSensitive/DataflowEnvironment.cpp | 16 +++---
.../FlowSensitive/DataflowEnvironmentTest.cpp | 16 +++---
.../TypeErasedDataflowAnalysisTest.cpp | 20 ++++---
4 files changed, 56 insertions(+), 48 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index c32d0b091e31c..097ff2bdfe7ad 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -162,20 +162,14 @@ class Environment {
: DACtx(&DACtx),
FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {}
- // Copy-constructor is private, Environments should not be copied. See
fork().
- Environment &operator=(const Environment &Other) = delete;
-
- Environment(Environment &&Other) = default;
- Environment &operator=(Environment &&Other) = default;
-
/// Creates an environment that uses `DACtx` to store objects that encompass
- /// the state of a program, with `S` as the initial analysis target.
+ /// the state of a program, with `S` as the statement to analyze.
Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) {
InitialTargetStmt = &S;
}
/// Creates an environment that uses `DACtx` to store objects that encompass
- /// the state of a program, with `FD` as the initial analysis target.
+ /// the state of a program, with `FD` as the function to analyze.
///
/// Requirements:
///
@@ -183,14 +177,21 @@ class Environment {
/// `FunctionDecl::doesThisDecalarationHaveABody()` must be true.
Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD)
: Environment(DACtx, *FD.getBody()) {
+ assert(FD.doesThisDeclarationHaveABody());
InitialTargetFunc = &FD;
}
+ // Copy-constructor is private, Environments should not be copied. See
fork().
+ Environment &operator=(const Environment &Other) = delete;
+
+ Environment(Environment &&Other) = default;
+ Environment &operator=(Environment &&Other) = default;
+
/// Assigns storage locations and values to all parameters, captures, global
- /// variables, fields and functions referenced in the initial analysis
target.
+ /// variables, fields and functions referenced in the `Stmt` or
`FunctionDecl`
+ /// passed to the constructor.
///
- /// If the target is a non-static member function, initializes the
environment
- /// with a symbolic representation of the `this` pointee.
+ /// If no `Stmt` or `FunctionDecl` was supplied, this function does nothing.
void initialize();
/// Returns a new environment that is a copy of this one.
@@ -662,13 +663,14 @@ class Environment {
return CallStack.empty() ? InitialTargetFunc : CallStack.back();
}
- /// Returns the size of the call stack.
+ /// Returns the size of the call stack, not counting the initial analysis
+ /// target.
size_t callStackSize() const { return CallStack.size(); }
/// Returns whether this `Environment` can be extended to analyze the given
- /// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and
a
- /// given `MaxDepth`. Note that the `MaxDepth` does not count any initial
- /// target function, which was not called.
+ /// `Callee` (i.e. if `pushCall` can be used).
+ /// Recursion is not allowed. `MaxDepth` is the maximum size of the call
stack
+ /// (i.e. the maximum value that `callStackSize()` may assume after the
call).
bool canDescend(unsigned MaxDepth, const FunctionDecl *Callee) const;
/// Returns the `DataflowAnalysisContext` used by the environment.
@@ -730,9 +732,6 @@ class Environment {
void pushCallInternal(const FunctionDecl *FuncDecl,
ArrayRef<const Expr *> Args);
- // FIXME: Add support for resetting globals after function calls to enable
- // the implementation of sound analyses.
-
/// Assigns storage locations and values to all global variables, fields
/// and functions in `Referenced`.
void initFieldsGlobalsAndFuncs(const ReferencedDecls &Referenced);
@@ -756,16 +755,16 @@ class Environment {
// shared between environments in the same call.
// https://github.com/llvm/llvm-project/issues/59005
- // The stack of functions called from the initial analysis target and
- // recursively being analyzed.
+ // The stack of functions called from the initial analysis target.
std::vector<const FunctionDecl *> CallStack;
- // If the initial analysis target is a function, this is the function
- // declaration. Otherwise, this is null.
+ // Initial function to analyze, if a function was passed to the constructor.
+ // Null otherwise.
const FunctionDecl *InitialTargetFunc = nullptr;
- // If the initial analysis target is a function, this is its body. If the
- // initial analysis target was not provided, this is null. Otherwise, this is
- // the initial analysis target.
+ // Top-level statement of the initial analysis target.
+ // If a function was passed to the constructor, this is its body.
+ // If a statement was passed to the constructor, this is that statement.
+ // Null if no analysis target was passed to the constructor.
Stmt *InitialTargetStmt = nullptr;
// Maps from prvalues of record type to their result objects. Shared between
@@ -791,8 +790,7 @@ class Environment {
RecordStorageLocation *LocForRecordReturnVal = nullptr;
// The storage location of the `this` pointee. Should only be null if the
- // analysis target is not a function or is a function but not a
- // method.
+ // analysis target is not a method.
RecordStorageLocation *ThisPointeeLoc = nullptr;
// Maps from declarations and glvalue expression to storage locations that
are
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index d1aec9eb3b147..338a85525b384 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -295,15 +295,14 @@ widenKeyToValueMap(const llvm::MapVector<Key, Value *>
&CurMap,
namespace {
// Visitor that builds a map from record prvalues to result objects.
-// This traverses the AST sub-tree to be visited; for each result object that
-// it encounters, it propagates the storage location of the result object to
all
-// record prvalues that can initialize it.
+// For each result object that it encounters, it propagates the storage
location
+// of the result object to all record prvalues that can initialize it.
class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
public:
// `ResultObjectMap` will be filled with a map from record prvalues to result
- // object. If the sub-tree to be visited returns a record by value,
- // `LocForRecordReturnVal` is the location to which this record should be
- // written; otherwise, it is null.
+ // object. If this visitor will traverse a function that returns a record by
+ // value, `LocForRecordReturnVal` is the location to which this record should
+ // be written; otherwise, it is null.
explicit ResultObjectVisitor(
llvm::DenseMap<const Expr *, RecordStorageLocation *> &ResultObjectMap,
RecordStorageLocation *LocForRecordReturnVal,
@@ -583,6 +582,9 @@ void Environment::initialize() {
LocForRecordReturnVal));
}
+// FIXME: Add support for resetting globals after function calls to enable the
+// implementation of sound analyses.
+
void Environment::initFieldsGlobalsAndFuncs(const ReferencedDecls &Referenced)
{
// These have to be added before the lines that follow to ensure that
// `create*` work correctly for structs.
@@ -796,7 +798,7 @@ Environment Environment::join(const Environment &EnvA,
const Environment &EnvB,
JoinedEnv.InitialTargetFunc = EnvA.InitialTargetFunc;
JoinedEnv.InitialTargetStmt = EnvA.InitialTargetStmt;
- auto *Func = EnvA.getCurrentFunc();
+ const FunctionDecl *Func = EnvA.getCurrentFunc();
if (!Func) {
JoinedEnv.ReturnVal = nullptr;
} else {
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index ef0280e74072c..bd710a00c47ce 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -409,7 +409,7 @@ TEST_F(EnvironmentTest, Stmt) {
using namespace ast_matchers;
std::string Code = R"cc(
- struct S {int i;};
+ struct S { int i; };
void foo() {
S AnS = S{1};
}
@@ -420,20 +420,20 @@ TEST_F(EnvironmentTest, Stmt) {
ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
- auto *DeclStatement = const_cast<DeclStmt *>(
- selectFirst<DeclStmt>("d", match(declStmt().bind("d"), Context)));
+ auto *DeclStatement = const_cast<DeclStmt *>(selectFirst<DeclStmt>(
+ "d", match(declStmt(hasSingleDecl(varDecl(hasName("AnS")))).bind("d"),
+ Context)));
ASSERT_THAT(DeclStatement, NotNull());
- auto *ConstructExpr = selectFirst<CXXConstructExpr>(
- "c", match(cxxConstructExpr().bind("c"), Context));
- ASSERT_THAT(ConstructExpr, NotNull());
+ auto *Init = (cast<VarDecl>(*DeclStatement->decl_begin()))->getInit();
+ ASSERT_THAT(Init, NotNull());
- // Verify that we can retrieve the result object location for the
construction
+ // Verify that we can retrieve the result object location for the initializer
// expression when we analyze the DeclStmt for `AnS`.
Environment Env(DAContext, *DeclStatement);
// Don't crash when initializing.
Env.initialize();
// And don't crash when retrieving the result object location.
- Env.getResultObjectLocation(*ConstructExpr);
+ Env.getResultObjectLocation(*Init);
}
} // namespace
diff --git
a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 2ddf0a6f66fd4..1a52b82d65665 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -148,14 +148,14 @@ TEST_F(DataflowAnalysisTest,
DiagnoseFunctionDiagnoserCalledOnEachElement) {
TEST_F(DataflowAnalysisTest, CanAnalyzeStmt) {
std::string Code = R"cc(
- struct S {int i;};
- S getAnS() {return S{1};};
+ struct S { bool b; };
void foo() {
- S AnS = getAnS();
+ S AnS = S{true};
}
)cc";
AST = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"});
- const auto &DeclStatement = matchNode<DeclStmt>(declStmt());
+ const auto &DeclStatement =
+ matchNode<DeclStmt>(declStmt(hasSingleDecl(varDecl(hasName("AnS")))));
const auto &Func = matchNode<FunctionDecl>(functionDecl(hasName("foo")));
ACFG = std::make_unique<AdornedCFG>(llvm::cantFail(AdornedCFG::build(
@@ -166,8 +166,16 @@ TEST_F(DataflowAnalysisTest, CanAnalyzeStmt) {
std::make_unique<WatchedLiteralsSolver>());
Environment Env(*DACtx, const_cast<DeclStmt &>(DeclStatement));
- ASSERT_THAT_ERROR(runDataflowAnalysis(*ACFG, Analysis, Env).takeError(),
- llvm::Succeeded());
+
llvm::Expected<std::vector<std::optional<DataflowAnalysisState<NoopLattice>>>>
+ Results = runDataflowAnalysis(*ACFG, Analysis, Env);
+
+ ASSERT_THAT_ERROR(Results.takeError(), llvm::Succeeded());
+ const Environment &ExitBlockEnv = Results->front()->Env;
+ BoolValue *BoolFieldValue = cast<BoolValue>(
+ getFieldValue(ExitBlockEnv.get<RecordStorageLocation>(
+ *cast<VarDecl>((*DeclStatement.decl_begin()))),
+ "b", AST->getASTContext(), ExitBlockEnv));
+ EXPECT_TRUE(Env.proves(BoolFieldValue->formula()));
}
// Tests for the statement-to-block map.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
