[analyzer] ArrayInitLoopExpr with array of non-POD type

This patch introduces the evaluation of ArrayInitLoopExpr
in case of structured bindings and implicit copy/move
constructor. The idea is to call the copy constructor for
every element in the array. The parameter of the copy
constructor is also manually selected, as it is not a part
of the CFG.

Differential Revision: https://reviews.llvm.org/D129496
This commit is contained in:
isuckatcs 2022-07-17 18:25:16 +02:00
parent 904a87ace3
commit 8a13326d18
9 changed files with 385 additions and 30 deletions

View File

@ -298,6 +298,11 @@ public:
const ConstructionContextLayer *TopLayer);
Kind getKind() const { return K; }
virtual const ArrayInitLoopExpr *getArrayInitLoop() const { return nullptr; }
// Only declared to silence -Wnon-virtual-dtor warnings.
virtual ~ConstructionContext() = default;
};
/// An abstract base class for local variable constructors.
@ -314,6 +319,12 @@ protected:
public:
const DeclStmt *getDeclStmt() const { return DS; }
const ArrayInitLoopExpr *getArrayInitLoop() const override {
const auto *Var = cast<VarDecl>(DS->getSingleDecl());
return dyn_cast<ArrayInitLoopExpr>(Var->getInit());
}
static bool classof(const ConstructionContext *CC) {
return CC->getKind() >= VARIABLE_BEGIN &&
CC->getKind() <= VARIABLE_END;
@ -381,6 +392,10 @@ protected:
public:
const CXXCtorInitializer *getCXXCtorInitializer() const { return I; }
const ArrayInitLoopExpr *getArrayInitLoop() const override {
return dyn_cast<ArrayInitLoopExpr>(I->getInit());
}
static bool classof(const ConstructionContext *CC) {
return CC->getKind() >= INITIALIZER_BEGIN &&
CC->getKind() <= INITIALIZER_END;

View File

@ -622,6 +622,11 @@ public:
getIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr *E,
const LocationContext *LCtx);
/// Retreives the size of the array in the pending ArrayInitLoopExpr.
static Optional<unsigned> getPendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx);
/// By looking at a certain item that may be potentially part of an object's
/// ConstructionContext, retrieve such object's location. A particular
/// statement can be transparently passed as \p Item in most cases.
@ -816,7 +821,9 @@ private:
/// Checks whether our policies allow us to inline a non-POD type array
/// construction.
bool shouldInlineArrayConstruction(const ArrayType *Type);
bool shouldInlineArrayConstruction(const ProgramStateRef State,
const CXXConstructExpr *CE,
const LocationContext *LCtx);
/// Checks whether we construct an array of non-POD type, and decides if the
/// constructor should be inkoved once again.
@ -916,6 +923,16 @@ private:
const CXXConstructExpr *E,
const LocationContext *LCtx);
/// Sets the size of the array in a pending ArrayInitLoopExpr.
static ProgramStateRef setPendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx,
unsigned Idx);
static ProgramStateRef removePendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx);
/// Store the location of a C++ object corresponding to a statement
/// until the statement is actually encountered. For example, if a DeclStmt
/// has CXXConstructExpr as its initializer, the object would be considered

View File

@ -1659,9 +1659,13 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) {
appendInitializer(Block, I);
if (Init) {
// If the initializer is an ArrayInitLoopExpr, we want to extract the
// initializer, that's used for each element.
const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init);
findConstructionContexts(
ConstructionContextLayer::create(cfg->getBumpVectorContext(), I),
Init);
AILE ? AILE->getSubExpr() : Init);
if (HasTemporaries) {
// For expression with temporaries go directly to subexpression to omit
@ -2931,9 +2935,13 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
autoCreateBlock();
appendStmt(Block, DS);
// If the initializer is an ArrayInitLoopExpr, we want to extract the
// initializer, that's used for each element.
const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init);
findConstructionContexts(
ConstructionContextLayer::create(cfg->getBumpVectorContext(), DS),
Init);
AILE ? AILE->getSubExpr() : Init);
// Keep track of the last non-null block, as 'Block' can be nulled out
// if the initializer expression is something like a 'while' in a

View File

@ -196,6 +196,14 @@ typedef llvm::ImmutableMap<
IndexOfElementToConstructMap;
REGISTER_TRAIT_WITH_PROGRAMSTATE(IndexOfElementToConstruct,
IndexOfElementToConstructMap)
// This trait is responsible for holding our pending ArrayInitLoopExprs.
// It pairs the LocationContext and the initializer CXXConstructExpr with
// the size of the array that's being copy initialized.
typedef llvm::ImmutableMap<
std::pair<const CXXConstructExpr *, const LocationContext *>, unsigned>
PendingInitLoopMap;
REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingInitLoop, PendingInitLoopMap)
//===----------------------------------------------------------------------===//
// Engine construction and deletion.
//===----------------------------------------------------------------------===//
@ -462,6 +470,34 @@ ProgramStateRef ExprEngine::setIndexOfElementToConstruct(
return State->set<IndexOfElementToConstruct>(Key, Idx);
}
Optional<unsigned> ExprEngine::getPendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx) {
return Optional<unsigned>::create(
State->get<PendingInitLoop>({E, LCtx->getStackFrame()}));
}
ProgramStateRef ExprEngine::removePendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx) {
auto Key = std::make_pair(E, LCtx->getStackFrame());
assert(E && State->contains<PendingInitLoop>(Key));
return State->remove<PendingInitLoop>(Key);
}
ProgramStateRef ExprEngine::setPendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx,
unsigned Size) {
auto Key = std::make_pair(E, LCtx->getStackFrame());
assert(!State->contains<PendingInitLoop>(Key) && Size > 0);
return State->set<PendingInitLoop>(Key, Size);
}
Optional<unsigned>
ExprEngine::getIndexOfElementToConstruct(ProgramStateRef State,
const CXXConstructExpr *E,
@ -487,17 +523,22 @@ ExprEngine::addObjectUnderConstruction(ProgramStateRef State,
const LocationContext *LC, SVal V) {
ConstructedObjectKey Key(Item, LC->getStackFrame());
const CXXConstructExpr *E = nullptr;
const Expr *Init = nullptr;
if (auto DS = dyn_cast_or_null<DeclStmt>(Item.getStmtOrNull())) {
if (auto VD = dyn_cast_or_null<VarDecl>(DS->getSingleDecl()))
E = dyn_cast<CXXConstructExpr>(VD->getInit());
Init = VD->getInit();
}
if (!E && !Item.getStmtOrNull()) {
auto CtorInit = Item.getCXXCtorInitializer();
E = dyn_cast<CXXConstructExpr>(CtorInit->getInit());
}
if (!Init && !Item.getStmtOrNull())
Init = Item.getCXXCtorInitializer()->getInit();
// In an ArrayInitLoopExpr the real initializer is returned by
// getSubExpr().
if (const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init))
Init = AILE->getSubExpr();
const auto *E = dyn_cast_or_null<CXXConstructExpr>(Init);
// FIXME: Currently the state might already contain the marker due to
// incorrect handling of temporaries bound to default parameters.
@ -2797,6 +2838,11 @@ void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex,
const Expr *Arr = Ex->getCommonExpr()->getSourceExpr();
for (auto *Node : CheckerPreStmt) {
// The constructor visitior has already taken care of everything.
if (auto *CE = dyn_cast<CXXConstructExpr>(Ex->getSubExpr()))
break;
const LocationContext *LCtx = Node->getLocationContext();
ProgramStateRef state = Node->getState();

View File

@ -462,6 +462,59 @@ ProgramStateRef ExprEngine::updateObjectsUnderConstruction(
llvm_unreachable("Unhandled construction context!");
}
static ProgramStateRef
bindRequiredArrayElementToEnvironment(ProgramStateRef State,
const ArrayInitLoopExpr *AILE,
const LocationContext *LCtx, SVal Idx) {
// The ctor in this case is guaranteed to be a copy ctor, otherwise we hit a
// compile time error.
//
// -ArrayInitLoopExpr <-- we're here
// |-OpaqueValueExpr
// | `-DeclRefExpr <-- match this
// `-CXXConstructExpr
// `-ImplicitCastExpr
// `-ArraySubscriptExpr
// |-ImplicitCastExpr
// | `-OpaqueValueExpr
// | `-DeclRefExpr
// `-ArrayInitIndexExpr
//
// The resulting expression might look like the one below in an implicit
// copy/move ctor.
//
// ArrayInitLoopExpr <-- we're here
// |-OpaqueValueExpr
// | `-MemberExpr <-- match this
// | (`-CXXStaticCastExpr) <-- move ctor only
// | `-DeclRefExpr
// `-CXXConstructExpr
// `-ArraySubscriptExpr
// |-ImplicitCastExpr
// | `-OpaqueValueExpr
// | `-MemberExpr
// | `-DeclRefExpr
// `-ArrayInitIndexExpr
//
// HACK: There is no way we can put the index of the array element into the
// CFG unless we unroll the loop, so we manually select and bind the required
// parameter to the environment.
const auto *CE = cast<CXXConstructExpr>(AILE->getSubExpr());
const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
SVal Base = UnknownVal();
if (const auto *ME = dyn_cast<MemberExpr>(OVESrc))
Base = State->getSVal(ME, LCtx);
else if (const auto *DRE = cast<DeclRefExpr>(OVESrc))
Base = State->getLValue(cast<VarDecl>(DRE->getDecl()), LCtx);
else
llvm_unreachable("ArrayInitLoopExpr contains unexpected source expression");
SVal NthElem = State->getLValue(CE->getType(), Idx, Base);
return State->BindExpr(CE->getArg(0), LCtx, NthElem);
}
void ExprEngine::handleConstructor(const Expr *E,
ExplodedNode *Pred,
ExplodedNodeSet &destNodes) {
@ -502,12 +555,26 @@ void ExprEngine::handleConstructor(const Expr *E,
// Inherited constructors are always base class constructors.
assert(CE && !CIE && "A complete constructor is inherited?!");
// If the ctor is part of an ArrayInitLoopExpr, we want to handle it
// differently.
auto *AILE = CC ? CC->getArrayInitLoop() : nullptr;
unsigned Idx = 0;
if (CE->getType()->isArrayType()) {
Idx = getIndexOfElementToConstruct(State, CE, LCtx).value_or(0u);
if (CE->getType()->isArrayType() || AILE) {
Idx = getIndexOfElementToConstruct(State, CE, LCtx).getValueOr(0u);
State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1);
}
if (AILE) {
// Only set this once even though we loop through it multiple times.
if (!getPendingInitLoop(State, CE, LCtx))
State = setPendingInitLoop(State, CE, LCtx,
AILE->getArraySize().getLimitedValue());
State = bindRequiredArrayElementToEnvironment(
State, AILE, LCtx, svalBuilder.makeArrayIndex(Idx));
}
// The target region is found from construction context.
std::tie(State, Target) =
handleConstructionContext(CE, State, LCtx, CC, CallOpts, Idx);

View File

@ -265,9 +265,13 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
ShouldRepeatCall = shouldRepeatCtorCall(state, CCE, callerCtx);
if (!ShouldRepeatCall &&
getIndexOfElementToConstruct(state, CCE, callerCtx))
state = removeIndexOfElementToConstruct(state, CCE, callerCtx);
if (!ShouldRepeatCall) {
if (getIndexOfElementToConstruct(state, CCE, callerCtx))
state = removeIndexOfElementToConstruct(state, CCE, callerCtx);
if (getPendingInitLoop(state, CCE, callerCtx))
state = removePendingInitLoop(state, CCE, callerCtx);
}
}
if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
@ -815,8 +819,7 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
// We still allow construction into ElementRegion targets when they don't
// represent array elements.
if (CallOpts.IsArrayCtorOrDtor) {
if (!shouldInlineArrayConstruction(
dyn_cast<ArrayType>(CtorExpr->getType())))
if (!shouldInlineArrayConstruction(Pred->getState(), CtorExpr, CurLC))
return CIP_DisallowedOnce;
}
@ -1082,10 +1085,14 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
return true;
}
bool ExprEngine::shouldInlineArrayConstruction(const ArrayType *Type) {
if (!Type)
bool ExprEngine::shouldInlineArrayConstruction(const ProgramStateRef State,
const CXXConstructExpr *CE,
const LocationContext *LCtx) {
if (!CE)
return false;
auto Type = CE->getType();
// FIXME: Handle other arrays types.
if (const auto *CAT = dyn_cast<ConstantArrayType>(Type)) {
unsigned Size = getContext().getConstantArrayElementCount(CAT);
@ -1093,6 +1100,10 @@ bool ExprEngine::shouldInlineArrayConstruction(const ArrayType *Type) {
return Size <= AMgr.options.maxBlockVisitOnPath;
}
// Check if we're inside an ArrayInitLoopExpr, and it's sufficiently small.
if (auto Size = getPendingInitLoop(State, CE, LCtx))
return *Size <= AMgr.options.maxBlockVisitOnPath;
return false;
}
@ -1111,6 +1122,9 @@ bool ExprEngine::shouldRepeatCtorCall(ProgramStateRef State,
return Size > getIndexOfElementToConstruct(State, E, LCtx);
}
if (auto Size = getPendingInitLoop(State, E, LCtx))
return Size > getIndexOfElementToConstruct(State, E, LCtx);
return false;
}

View File

@ -125,3 +125,97 @@ void move_ctor_uninit() {
clang_analyzer_eval(moved.arr[3]); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(moved.arr[4]); // expected-warning{{UNKNOWN}}
}
// The struct has a user defined copy and move ctor, which allow us to
// track the values more precisely when an array of this struct is being
// copy/move initialized by ArrayInitLoopExpr.
struct S2 {
inline static int c = 0;
int i;
S2() : i(++c) {}
S2(const S2 &copy) {
i = copy.i + 1;
}
S2(S2 &&move) {
i = move.i + 2;
}
};
void array_init_non_pod() {
S2::c = 0;
S2 arr[4];
auto [a, b, c, d] = arr;
clang_analyzer_eval(a.i == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(b.i == 3); // expected-warning{{TRUE}}
clang_analyzer_eval(c.i == 4); // expected-warning{{TRUE}}
clang_analyzer_eval(d.i == 5); // expected-warning{{TRUE}}
}
struct S3 {
int i;
};
void array_uninit_non_pod() {
S3 arr[1];
auto [a] = arr; // expected-warning@159{{ in implicit constructor is garbage or undefined }}
}
void lambda_init_non_pod() {
S2::c = 0;
S2 arr[4];
// FIXME: These should be TRUE, but we fail to capture the array properly.
auto l = [arr] { return arr[0].i; }();
clang_analyzer_eval(l == 2); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
l = [arr] { return arr[1].i; }();
clang_analyzer_eval(l == 3); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
l = [arr] { return arr[2].i; }();
clang_analyzer_eval(l == 4); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
l = [arr] { return arr[3].i; }();
clang_analyzer_eval(l == 5); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
}
void lambda_uninit_non_pod() {
S3 arr[4];
int l = [arr] { return arr[3].i; }();
}
// If this struct is being copy/move constructed by the implicit ctors, ArrayInitLoopExpr
// is responsible for the initialization of 'arr' by copy/move constructing each of the
// elements.
struct S5 {
S2 arr[4];
};
void copy_ctor_init_non_pod() {
S2::c = 0;
S5 orig;
S5 copy = orig;
clang_analyzer_eval(copy.arr[0].i == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(copy.arr[1].i == 3); // expected-warning{{TRUE}}
clang_analyzer_eval(copy.arr[2].i == 4); // expected-warning{{TRUE}}
clang_analyzer_eval(copy.arr[3].i == 5); // expected-warning{{TRUE}}
}
void move_ctor_init_non_pod() {
S2::c = 0;
S5 orig;
S5 moved = (S5 &&) orig;
clang_analyzer_eval(moved.arr[0].i == 3); // expected-warning{{TRUE}}
clang_analyzer_eval(moved.arr[1].i == 4); // expected-warning{{TRUE}}
clang_analyzer_eval(moved.arr[2].i == 5); // expected-warning{{TRUE}}
clang_analyzer_eval(moved.arr[3].i == 6); // expected-warning{{TRUE}}
}

View File

@ -439,16 +439,16 @@ namespace ArrayMembers {
NonPOD b = a;
clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{TRUE}}
NonPOD c;
c = b;
clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{TRUE}}
}
struct NestedNonPOD {
@ -502,16 +502,16 @@ namespace ArrayMembers {
NonPODDefaulted b = a;
clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{TRUE}}
NonPODDefaulted c;
c = b;
clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{TRUE}}
}
};

View File

@ -292,3 +292,97 @@ void array_big_a(void) {
clang_analyzer_eval(e == 5); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(f == 6); // expected-warning{{UNKNOWN}}
}
struct S {
int a = 1;
int b = 2;
};
void non_pod_val(void) {
S arr[2];
auto [x, y] = arr;
clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
}
void non_pod_val_syntax_2(void) {
S arr[2];
auto [x, y](arr);
clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
}
void non_pod_lref(void) {
S arr[2];
auto &[x, y] = arr;
clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
}
void non_pod_rref(void) {
S arr[2];
auto &&[x, y] = arr;
clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
}
struct SUD {
inline static int c = 0;
int a = 1;
int b = 2;
SUD() { ++c; };
SUD(const SUD &copy) {
a = copy.a + 1;
b = copy.b + 1;
}
};
void non_pod_user_defined_val(void) {
SUD arr[2];
auto [x, y] = arr;
clang_analyzer_eval(x.a == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(x.b == 3); // expected-warning{{TRUE}}
clang_analyzer_eval(y.a == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(y.b == 3); // expected-warning{{TRUE}}
}
void non_pod_user_defined_val_syntax_2(void) {
SUD::c = 0;
SUD arr[2];
auto [x, y](arr);
clang_analyzer_eval(SUD::c == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(x.a == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(x.b == 3); // expected-warning{{TRUE}}
clang_analyzer_eval(y.a == 2); // expected-warning{{TRUE}}
clang_analyzer_eval(y.b == 3); // expected-warning{{TRUE}}
}