[clang] e607360 - [clang][analyzer] Remove array bounds check from PointerSubChecker (#102580)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 12 02:22:33 PDT 2024
Author: Balázs Kéri
Date: 2024-08-12T11:22:30+02:00
New Revision: e607360fcde2994080bb8cec9d4be3a4091fe9a9
URL: https://github.com/llvm/llvm-project/commit/e607360fcde2994080bb8cec9d4be3a4091fe9a9
DIFF: https://github.com/llvm/llvm-project/commit/e607360fcde2994080bb8cec9d4be3a4091fe9a9.diff
LOG: [clang][analyzer] Remove array bounds check from PointerSubChecker (#102580)
At pointer subtraction only pointers are allowed that point into an
array (or one after the end), this fact was checker by the checker. This
check is now removed because it is a special case of array indexing
error that is handled by different checkers (like ArrayBoundsV2).
Added:
Modified:
clang/docs/analyzer/checkers.rst
clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
clang/test/Analysis/pointer-sub-notes.c
clang/test/Analysis/pointer-sub.c
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 55832d20bd27a1..46b0b7b9c82376 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2501,7 +2501,11 @@ alpha.core.PointerSub (C)
Check for pointer subtractions on two pointers pointing to
diff erent memory
chunks. According to the C standard §6.5.6 only subtraction of pointers that
point into (or one past the end) the same array object is valid (for this
-purpose non-array variables are like arrays of size 1).
+purpose non-array variables are like arrays of size 1). This checker only
+searches for
diff erent memory objects at subtraction, but does not check if the
+array index is correct. Furthermore, only cases are reported where
+stack-allocated objects are involved (no warnings on pointers to memory
+allocated by `malloc`).
.. code-block:: c
@@ -2511,11 +2515,6 @@ purpose non-array variables are like arrays of size 1).
x = &d[4] - &c[1]; // warn: 'c' and 'd' are
diff erent arrays
x = (&a + 1) - &a;
x = &b - &a; // warn: 'a' and 'b' are
diff erent variables
- x = (&a + 2) - &a; // warn: for a variable it is only valid to have a pointer
- // to one past the address of it
- x = &c[10] - &c[0];
- x = &c[11] - &c[0]; // warn: index larger than one past the end
- x = &c[-1] - &c[0]; // warn: negative index
}
struct S {
@@ -2538,9 +2537,6 @@ offsets of members in a structure, using pointer subtractions. This is still
undefined behavior according to the standard and code like this can be replaced
with the `offsetof` macro.
-The checker only reports cases where stack-allocated objects are involved (no
-warnings on pointers to memory allocated by `malloc`).
-
.. _alpha-core-StackAddressAsyncEscape:
alpha.core.StackAddressAsyncEscape (C)
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b856b0edc61514..f0dc5efd75f7d6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -31,99 +31,12 @@ class PointerSubChecker
const llvm::StringLiteral Msg_MemRegionDifferent =
"Subtraction of two pointers that do not point into the same array "
"is undefined behavior.";
- const llvm::StringLiteral Msg_LargeArrayIndex =
- "Using an array index greater than the array size at pointer subtraction "
- "is undefined behavior.";
- const llvm::StringLiteral Msg_NegativeArrayIndex =
- "Using a negative array index at pointer subtraction "
- "is undefined behavior.";
- const llvm::StringLiteral Msg_BadVarIndex =
- "Indexing the address of a variable with other than 1 at this place "
- "is undefined behavior.";
-
- /// Check that an array is indexed in the allowed range that is 0 to "one
- /// after the end". The "array" can be address of a non-array variable.
- /// @param E Expression of the pointer subtraction.
- /// @param ElemReg An indexed region in the subtraction expression.
- /// @param Reg Region of the other side of the expression.
- bool checkArrayBounds(CheckerContext &C, const Expr *E,
- const ElementRegion *ElemReg,
- const MemRegion *Reg) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
};
}
-static bool isArrayVar(const MemRegion *R) {
- while (R) {
- if (isa<VarRegion>(R))
- return true;
- if (const auto *ER = dyn_cast<ElementRegion>(R))
- R = ER->getSuperRegion();
- else
- return false;
- }
- return false;
-}
-
-bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E,
- const ElementRegion *ElemReg,
- const MemRegion *Reg) const {
- if (!ElemReg)
- return true;
-
- const MemRegion *SuperReg = ElemReg->getSuperRegion();
- if (!isArrayVar(SuperReg))
- return true;
-
- auto ReportBug = [&](const llvm::StringLiteral &Msg) {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
- auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
- R->addRange(E->getSourceRange());
- C.emitReport(std::move(R));
- }
- };
-
- ProgramStateRef State = C.getState();
- SValBuilder &SVB = C.getSValBuilder();
-
- if (SuperReg == Reg) {
- // Case like `(&x + 1) - &x`. Only 1 or 0 is allowed as index.
- if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
- I && (!I->isOne() && !I->isZero()))
- ReportBug(Msg_BadVarIndex);
- return false;
- }
-
- DefinedOrUnknownSVal ElemCount =
- getDynamicElementCount(State, SuperReg, SVB, ElemReg->getElementType());
- auto IndexTooLarge = SVB.evalBinOp(C.getState(), BO_GT, ElemReg->getIndex(),
- ElemCount, SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (IndexTooLarge) {
- ProgramStateRef S1, S2;
- std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
- if (S1 && !S2) {
- ReportBug(Msg_LargeArrayIndex);
- return false;
- }
- }
- auto IndexTooSmall = SVB.evalBinOp(State, BO_LT, ElemReg->getIndex(),
- SVB.makeZeroVal(SVB.getArrayIndexType()),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (IndexTooSmall) {
- ProgramStateRef S1, S2;
- std::tie(S1, S2) = State->assume(*IndexTooSmall);
- if (S1 && !S2) {
- ReportBug(Msg_NegativeArrayIndex);
- return false;
- }
- }
- return true;
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext &C) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -151,9 +64,11 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
const auto *ElemLR = dyn_cast<ElementRegion>(LR);
const auto *ElemRR = dyn_cast<ElementRegion>(RR);
- if (!checkArrayBounds(C, B->getLHS(), ElemLR, RR))
+ // Allow cases like "(&x + 1) - &x".
+ if (ElemLR && ElemLR->getSuperRegion() == RR)
return;
- if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
+ // Allow cases like "&x - (&x + 1)".
+ if (ElemRR && ElemRR->getSuperRegion() == LR)
return;
const ValueDecl *DiffDeclL = nullptr;
diff --git a/clang/test/Analysis/pointer-sub-notes.c b/clang/test/Analysis/pointer-sub-notes.c
index 6e347557354226..59681b4e7555af 100644
--- a/clang/test/Analysis/pointer-sub-notes.c
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -1,22 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -analyzer-output=text -verify %s
-void negative_1() {
- int a[3];
- int x = -1;
- // FIXME: should indicate that 'x' is -1
- int d = &a[x] - &a[0]; // expected-warning{{Using a negative array index at pointer subtraction is undefined behavior}} \
- // expected-note{{Using a negative array index at pointer subtraction is undefined behavior}}
-}
-
-void negative_2() {
- int a[3];
- int *p1 = a, *p2 = a;
- --p2;
- // FIXME: should indicate that 'p2' is negative
- int d = p1 - p2; // expected-warning{{Using a negative array index at pointer subtraction is undefined behavior}} \
- // expected-note{{Using a negative array index at pointer subtraction is undefined behavior}}
-}
-
void
diff erent_1() {
int a[3]; // expected-note{{Array at the left-hand side of subtraction}}
int b[3]; // expected-note{{Array at the right-hand side of subtraction}}
diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c
index 194c8918899525..cf9eac1abc2dcc 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -9,13 +9,7 @@ void f1(void) {
d = (&x + 1) - &x; // no-warning ('&x' is like a single-element array)
d = &x - (&x + 1); // no-warning
d = (&x + 0) - &x; // no-warning
- d = (&x - 1) - &x; // expected-warning{{Indexing the address of a variable with other than 1 at this place is undefined behavior}}
- d = (&x + 2) - &x; // expected-warning{{Indexing the address of a variable with other than 1 at this place is undefined behavior}}
-
- d = (z + 9) - z; // no-warning (pointers to same array)
- d = (z + 10) - z; // no-warning (pointer to "one after the end")
- d = (z + 11) - z; // expected-warning{{Using an array index greater than the array size at pointer subtraction is undefined behavior}}
- d = (z - 1) - z; // expected-warning{{Using a negative array index at pointer subtraction is undefined behavior}}
+ d = (z + 10) - z; // no-warning
}
void f2(void) {
@@ -28,11 +22,6 @@ void f2(void) {
q = &b[3];
d = q - p; // expected-warning{{Subtraction of two pointers that}}
- q = a + 10;
- d = q - p; // no warning (use of pointer to one after the end is allowed)
- q = a + 11;
- d = q - a; // expected-warning{{Using an array index greater than the array size at pointer subtraction is undefined behavior}}
-
d = &a[4] - a; // no-warning
d = &a[2] - p; // no-warning
d = &c - p; // expected-warning{{Subtraction of two pointers that}}
More information about the cfe-commits
mailing list