Skip to content

Commit e952f05

Browse files
committed
Delete special case supporting weird casts
Simplify the code by removing a special case that tried to ensure identical behavior between analysis with and without eagerly-assume in a weird theoretical situation.
1 parent cbb46e5 commit e952f05

File tree

2 files changed

+23
-18
lines changed

2 files changed

+23
-18
lines changed

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,14 +2839,7 @@ void ExprEngine::processBranch(
28392839

28402840
const Expr *EagerlyAssumeExpr =
28412841
PrevState->get<LastEagerlyAssumeAssumptionAt>();
2842-
const Expr *ConditionExpr = dyn_cast<Expr>(Condition);
2843-
if (ConditionExpr) {
2844-
// Ignore casts to ensure equivalent behavior with and without
2845-
// eagerly-assume. This is a mostly theoretical question an I don't see a
2846-
// good reason for putting casts around a conditional expression.
2847-
ConditionExpr = ConditionExpr->IgnoreParenCasts();
2848-
}
2849-
bool DidEagerlyAssume = EagerlyAssumeExpr == ConditionExpr;
2842+
bool DidEagerlyAssume = EagerlyAssumeExpr == dyn_cast<Expr>(Condition);
28502843

28512844
// Process the true branch.
28522845
if (builder.isFeasible(true)) {

clang/test/Analysis/out-of-bounds.c

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -verify %s
1+
// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -verify=expected,eagerlyassume %s
22
// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
33

44
// Note that eagerly-assume=false is tested separately because the
@@ -268,15 +268,6 @@ void no_suppression_when_no_assumption_third_iteration(int len) {
268268
}
269269
}
270270

271-
void loop_suppress_in_third_iteration_cast(int len) {
272-
int buf[2] = {0};
273-
for (int i = 0; (unsigned)(i < len); i++) {
274-
// Check that a (somewhat arbitrary) cast does not hinder the recognition
275-
// of the condition expression.
276-
buf[i] = 1; // no-warning
277-
}
278-
}
279-
280271
void loop_suppress_in_third_iteration_logical_and(int len, int flag) {
281272
int buf[2] = {0};
282273
for (int i = 0; i < len && flag; i++) {
@@ -303,6 +294,27 @@ void loop_suppress_in_third_iteration_logical_and_2(int len, int flag) {
303294
}
304295
}
305296

297+
void loop_suppress_in_third_iteration_cast(int len) {
298+
int buf[2] = {0};
299+
for (int i = 0; (unsigned)(i < len); i++) {
300+
// The behavior of this suppression is slightly different under
301+
// eagerly-assume=true (the default) and eagerly-assume=false:
302+
// * When eager assumptions are disabled, it's enough to look for cases
303+
// where we get two non-null states from splitting the state over the
304+
// 'SVal' that represents the full loop condition.
305+
// * When eager assumptions are enabled, we also accept situations when the
306+
// loop condition expression triggers an eager state split and therefore
307+
// we won't see a state split at the "normal" point because it's executed
308+
// on two already separated execution paths.
309+
// However, for the sake of simplicity we don't activate the suppression in
310+
// cases when _a subexpression_ of the loop condition triggers an eager
311+
// assumption. There are already many differences between analysis with and
312+
// without eager assumptions, so it would be pointless to write more
313+
// complicated code to eliminate these rare differences.
314+
buf[i] = 1; // eagerlyassume-warning{{Out of bound access to memory}}
315+
}
316+
}
317+
306318
int coinflip(void);
307319
int do_while_report_after_one_iteration(void) {
308320
int i = 0;

0 commit comments

Comments
 (0)