Skip to content

[analyzer] Fix a crash from element region construction during ArrayInitLoopExpr analysis #113570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 26, 2024
Merged

[analyzer] Fix a crash from element region construction during ArrayInitLoopExpr analysis #113570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 12 additions & 57 deletions clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,70 +513,25 @@ ProgramStateRef ExprEngine::updateObjectsUnderConstruction(
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
//
// The resulting expression for a multidimensional array.
// ArrayInitLoopExpr <-- we're here
// |-OpaqueValueExpr
// | `-DeclRefExpr <-- match this
// `-ArrayInitLoopExpr
// |-OpaqueValueExpr
// | `-ArraySubscriptExpr
// | |-ImplicitCastExpr
// | | `-OpaqueValueExpr
// | | `-DeclRefExpr
// | `-ArrayInitIndexExpr
// `-CXXConstructExpr <-- extract this
// ` ...

const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
const LocationContext *LCtx, NonLoc Idx) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
MemRegionManager &MRMgr = SVB.getRegionManager();
ASTContext &Ctx = SVB.getContext();

// 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 =
const Expr *SourceArray = AILE->getCommonExpr()->getSourceExpr();
const auto *Ctor =
cast<CXXConstructExpr>(extractElementInitializerFromNestedAILE(AILE));

SVal Base = UnknownVal();
if (const auto *ME = dyn_cast<MemberExpr>(OVESrc))
Base = State->getSVal(ME, LCtx);
else if (const auto *DRE = dyn_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);
const auto *SourceArrayRegion =
cast<SubRegion>(State->getSVal(SourceArray, LCtx).getAsRegion());
const ElementRegion *ElementRegion =
MRMgr.getElementRegion(Ctor->getType(), Idx, SourceArrayRegion, Ctx);

return State->BindExpr(CE->getArg(0), LCtx, NthElem);
return State->BindExpr(Ctor->getArg(0), LCtx,
loc::MemRegionVal(ElementRegion));
}

void ExprEngine::handleConstructor(const Expr *E,
Expand Down
38 changes: 38 additions & 0 deletions clang/test/Analysis/array-init-loop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,41 @@ void no_crash() {
}

} // namespace crash

namespace array_subscript_initializer {
struct S {
int x;
};

void no_crash() {
S arr[][2] = {{1, 2}};

const auto [a, b] = arr[0]; // no-crash
Copy link
Member Author

@isuckatcs isuckatcs Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steakhal // no-crash at the end of the line. Similarly in the other test case.

Copy link
Contributor

@steakhal steakhal Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. Im too tired now.


clang_analyzer_eval(a.x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(b.x == 2); // expected-warning{{TRUE}}
}
} // namespace array_subscript_initializer

namespace iterator_initializer {
struct S {
int x;
};

void no_crash() {
S arr[][2] = {{1, 2}, {3, 4}};

int i = 0;
for (const auto [a, b] : arr) { // no-crash
if (i == 0) {
clang_analyzer_eval(a.x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(b.x == 2); // expected-warning{{TRUE}}
} else {
clang_analyzer_eval(a.x == 3); // expected-warning{{TRUE}}
clang_analyzer_eval(b.x == 4); // expected-warning{{TRUE}}
}

++i;
}
}
} // namespace iterator_initializer
Loading