https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/93676
From a896030e71d09ebe7239d6fab343606918ee4c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Wed, 29 May 2024 14:28:43 +0200 Subject: [PATCH 1/5] [clang][analyzer] Improved PointerSubChecker The checker is made more exact (only pointer into array is allowed) and more tests are added. --- .../Checkers/PointerSubChecker.cpp | 34 +++++---- clang/test/Analysis/pointer-sub.c | 74 +++++++++++++++++++ clang/test/Analysis/ptr-arith.c | 14 +--- 3 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 clang/test/Analysis/pointer-sub.c diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index 2438cf30b39b5..4caa9ded93ed4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -35,7 +35,7 @@ class PointerSubChecker void PointerSubChecker::checkPreStmt(const BinaryOperator *B, CheckerContext &C) const { // When doing pointer subtraction, if the two pointers do not point to the - // same memory chunk, emit a warning. + // same array, emit a warning. if (B->getOpcode() != BO_Sub) return; @@ -44,24 +44,30 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, const MemRegion *LR = LV.getAsRegion(); const MemRegion *RR = RV.getAsRegion(); - - if (!(LR && RR)) - return; - - const MemRegion *BaseLR = LR->getBaseRegion(); - const MemRegion *BaseRR = RR->getBaseRegion(); - - if (BaseLR == BaseRR) + if (!LR || !RR) return; - // Allow arithmetic on different symbolic regions. - if (isa<SymbolicRegion>(BaseLR) || isa<SymbolicRegion>(BaseRR)) - return; + const auto *ElemLR = dyn_cast<ElementRegion>(LR); + const auto *ElemRR = dyn_cast<ElementRegion>(RR); + // FIXME: We want to verify that these are elements of an array. + // Because behavior of ElementRegion it may be confused with a cast. + // There is not a simple way to distinguish it from array element (check the + // types?). Because this missing check a warning is missing in the rare case + // when two casted pointers to the same region (variable) are subtracted. + if (ElemLR && ElemRR) { + const MemRegion *SuperLR = ElemLR->getSuperRegion(); + const MemRegion *SuperRR = ElemRR->getSuperRegion(); + if (SuperLR == SuperRR) + return; + // Allow arithmetic on different symbolic regions. + if (isa<SymbolicRegion>(SuperLR) || isa<SymbolicRegion>(SuperRR)) + return; + } if (ExplodedNode *N = C.generateNonFatalErrorNode()) { constexpr llvm::StringLiteral Msg = - "Subtraction of two pointers that do not point to the same memory " - "chunk may cause incorrect result."; + "Subtraction of two pointers that do not point into the same array " + "is undefined behavior."; auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); R->addRange(B->getSourceRange()); C.emitReport(std::move(R)); diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c new file mode 100644 index 0000000000000..010c739d6ad10 --- /dev/null +++ b/clang/test/Analysis/pointer-sub.c @@ -0,0 +1,74 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s + +void f1(void) { + int x, y, z[10]; + int d = &y - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} + d = z - &y; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} + d = &x - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} + d = (long*)&x - (long*)&x; +} + +void f2(void) { + int a[10], b[10], c; + int *p = &a[2]; + int *q = &a[8]; + int d = q - p; // no-warning + + 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) + d = &a[4] - a; // no warning + + q = a + 11; + d = q - a; // ? + + d = &c - p; // expected-warning{{Subtraction of two pointers that}} +} + +void f3(void) { + int a[3][4]; + int d; + + d = &(a[2]) - &(a[1]); + d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}} + d = a[1] - a[1]; + d = &(a[1][2]) - &(a[1][0]); + d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two pointers that}} +} + +void f4(void) { + int n = 4, m = 3; + int a[n][m]; + int (*p)[m] = a; // p == &a[0] + p += 1; // p == &a[1] + int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} + + d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} + d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}} +} + +typedef struct { + int a; + int b; + int c[10]; + int d[10]; +} S; + +void f5(void) { + S s; + int y; + int d; + + d = &s.b - &s.a; // expected-warning{{Subtraction of two pointers that}} + d = &s.c[0] - &s.a; // expected-warning{{Subtraction of two pointers that}} + d = &s.b - &y; // expected-warning{{Subtraction of two pointers that}} + d = &s.c[3] - &s.c[2]; + d = &s.d[3] - &s.c[2]; // expected-warning{{Subtraction of two pointers that}} + d = s.d - s.c; // expected-warning{{Subtraction of two pointers that}} + + S sa[10]; + d = &sa[2] - &sa[1]; + d = &sa[2].a - &sa[1].b; // expected-warning{{Subtraction of two pointers that}} +} diff --git a/clang/test/Analysis/ptr-arith.c b/clang/test/Analysis/ptr-arith.c index 40c8188704e81..f99dfabb07366 100644 --- a/clang/test/Analysis/ptr-arith.c +++ b/clang/test/Analysis/ptr-arith.c @@ -1,5 +1,5 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -Wno-pointer-to-int-cast -verify -triple x86_64-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,debug.ExprInspection -Wno-pointer-to-int-cast -verify -triple x86_64-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,debug.ExprInspection -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s void clang_analyzer_eval(int); void clang_analyzer_dump(int); @@ -35,16 +35,6 @@ domain_port (const char *domain_b, const char *domain_e, return port; } -void f3(void) { - int x, y; - int d = &y - &x; // expected-warning{{Subtraction of two pointers that do not point to the same memory chunk may cause incorrect result}} - - int a[10]; - int *p = &a[2]; - int *q = &a[8]; - d = q-p; // no-warning -} - void f4(void) { int *p; p = (int*) 0x10000; // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} From 7c2e5604f20c9fe1089008bd346577426bd92234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 30 May 2024 17:56:37 +0200 Subject: [PATCH 2/5] improved checker in some cases, added tests --- .../Checkers/PointerSubChecker.cpp | 13 ++-- clang/test/Analysis/casts.c | 4 ++ clang/test/Analysis/pointer-sub.c | 63 +++++++++++++++++-- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index 4caa9ded93ed4..87764aa369f83 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -47,13 +47,16 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, if (!LR || !RR) return; + // Allow subtraction of identical pointers. + if (LR == RR) + return; + + // No warning if one operand is unknown. + if (isa<SymbolicRegion>(LR) || isa<SymbolicRegion>(RR)) + return; + const auto *ElemLR = dyn_cast<ElementRegion>(LR); const auto *ElemRR = dyn_cast<ElementRegion>(RR); - // FIXME: We want to verify that these are elements of an array. - // Because behavior of ElementRegion it may be confused with a cast. - // There is not a simple way to distinguish it from array element (check the - // types?). Because this missing check a warning is missing in the rare case - // when two casted pointers to the same region (variable) are subtracted. if (ElemLR && ElemRR) { const MemRegion *SuperLR = ElemLR->getSuperRegion(); const MemRegion *SuperRR = ElemRR->getSuperRegion(); diff --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c index 30cd74be564fd..7dad4edfa89b9 100644 --- a/clang/test/Analysis/casts.c +++ b/clang/test/Analysis/casts.c @@ -138,7 +138,9 @@ void multiDimensionalArrayPointerCasts(void) { clang_analyzer_eval(y1 == y2); // expected-warning{{TRUE}} // FIXME: should be FALSE (i.e. equal pointers). + // FIXME: pointer subtraction warning might be incorrect clang_analyzer_eval(y1 - y2); // expected-warning{{UNKNOWN}} + // expected-warning@-1{{Subtraction of two pointers that do not point into the same array is undefined behavior}} // FIXME: should be TRUE (i.e. same symbol). clang_analyzer_eval(*y1 == *y2); // expected-warning{{UNKNOWN}} @@ -147,7 +149,9 @@ void multiDimensionalArrayPointerCasts(void) { clang_analyzer_eval(y1 == y3); // expected-warning{{TRUE}} // FIXME: should be FALSE (i.e. equal pointers). + // FIXME: pointer subtraction warning might be incorrect clang_analyzer_eval(y1 - y3); // expected-warning{{UNKNOWN}} + // expected-warning@-1{{Subtraction of two pointers that do not point into the same array is undefined behavior}} // FIXME: should be TRUE (i.e. same symbol). clang_analyzer_eval(*y1 == *y3); // expected-warning{{UNKNOWN}} diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index 010c739d6ad10..93033d23cd14a 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -1,30 +1,54 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s +#if 0 + +void f(void) { + int a[3][4]; + int d; + d = (int *)(((char *)(&a[2][2]) + 1) - 1) - &a[2][2]; + d = (int *)(((char *)(&a[2][2]) + 1) - 1) - (int *)(((char *)(&a[1][1]) + 1) - 1); + + long long l; + char *a1 = (char *)&l; + d = a1[3] - l; + + long long la1[3]; + long long la2[3]; + char *a2 = (char *)la1; + d = &a2[3] - (char *)&la2[2]; +} + +#else + void f1(void) { int x, y, z[10]; int d = &y - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} d = z - &y; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} - d = &x - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} - d = (long*)&x - (long*)&x; + d = &x - &x; // no-warning (subtraction of any two identical pointers is allowed) + d = (long *)&x - (long *)&x; + d = (&x + 1) - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} } void f2(void) { int a[10], b[10], c; int *p = &a[2]; int *q = &a[8]; - int d = q - p; // no-warning + int d = q - p; // no-warning (pointers into the same array) 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) - d = &a[4] - a; // no warning - q = a + 11; - d = q - a; // ? + d = q - a; // no-warning (no check for past-the-end array access in this checker) + d = &a[4] - a; // no-warning + d = &a[2] - p; // no-warning d = &c - p; // expected-warning{{Subtraction of two pointers that}} + + d = (int *)((char *)(&a[4]) + 4) - &a[4]; // no-warning (pointers into the same array data) + d = (int *)((char *)(&a[4]) + 3) - &a[4]; // expected-warning{{Subtraction of two pointers that}} } void f3(void) { @@ -36,6 +60,10 @@ void f3(void) { d = a[1] - a[1]; d = &(a[1][2]) - &(a[1][0]); d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two pointers that}} + + // FIXME: This warning is wrong: + // 2-dimensional array is internally converted into one-dimensional by the analyzer + d = (int *)(((char *)(&a[2][2]) + 2) - 1) - &a[2][2]; // expected-warning{{Subtraction of two pointers that}} } void f4(void) { @@ -43,9 +71,13 @@ void f4(void) { int a[n][m]; int (*p)[m] = a; // p == &a[0] p += 1; // p == &a[1] + + // FIXME: This warning is not needed int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} + // FIXME: This warning is not needed d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} + d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}} } @@ -72,3 +104,22 @@ void f5(void) { d = &sa[2] - &sa[1]; d = &sa[2].a - &sa[1].b; // expected-warning{{Subtraction of two pointers that}} } + +void f6(void) { + long long l; + char *a1 = (char *)&l; + int d = a1[3] - l; + + long long la1[3]; + long long la2[3]; + char *pla1 = (char *)la1; + char *pla2 = (char *)la2; + d = &pla2[3] - &pla1[3]; // expected-warning{{Subtraction of two pointers that}} +} + +void f7(int *p) { + int a[10]; + int d = &a[10] - p; // no-warning ('p' is unknown, even if it cannot point into 'a') +} + +#endif From d981a59cca2f4e3c55b3f80bd61969bf07f0b9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 31 May 2024 16:09:50 +0200 Subject: [PATCH 3/5] fixes and additions in tests --- clang/test/Analysis/pointer-sub.c | 37 ++++++++----------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index 93033d23cd14a..423292124a309 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -1,25 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s -#if 0 - -void f(void) { - int a[3][4]; - int d; - d = (int *)(((char *)(&a[2][2]) + 1) - 1) - &a[2][2]; - d = (int *)(((char *)(&a[2][2]) + 1) - 1) - (int *)(((char *)(&a[1][1]) + 1) - 1); - - long long l; - char *a1 = (char *)&l; - d = a1[3] - l; - - long long la1[3]; - long long la2[3]; - char *a2 = (char *)la1; - d = &a2[3] - (char *)&la2[2]; -} - -#else - void f1(void) { int x, y, z[10]; int d = &y - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} @@ -41,14 +21,14 @@ void f2(void) { q = a + 10; d = q - p; // no warning (use of pointer to one after the end is allowed) q = a + 11; - d = q - a; // no-warning (no check for past-the-end array access in this checker) + d = q - a; // no-warning (no check for past-the-end array pointers in this checker) d = &a[4] - a; // no-warning d = &a[2] - p; // no-warning d = &c - p; // expected-warning{{Subtraction of two pointers that}} - d = (int *)((char *)(&a[4]) + 4) - &a[4]; // no-warning (pointers into the same array data) - d = (int *)((char *)(&a[4]) + 3) - &a[4]; // expected-warning{{Subtraction of two pointers that}} + d = (int *)((char *)(&a[4]) + sizeof(int)) - &a[4]; // no-warning (pointers into the same array data) + d = (int *)((char *)(&a[4]) + 1) - &a[4]; // expected-warning{{Subtraction of two pointers that}} } void f3(void) { @@ -61,9 +41,10 @@ void f3(void) { d = &(a[1][2]) - &(a[1][0]); d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two pointers that}} - // FIXME: This warning is wrong: - // 2-dimensional array is internally converted into one-dimensional by the analyzer - d = (int *)(((char *)(&a[2][2]) + 2) - 1) - &a[2][2]; // expected-warning{{Subtraction of two pointers that}} + d = (int *)((char *)(&a[2][2]) + sizeof(int)) - &a[2][2]; // expected-warning{{Subtraction of two pointers that}} + d = (int *)((char *)(&a[2][2]) + 1) - &a[2][2]; // expected-warning{{Subtraction of two pointers that}} + d = (int (*)[4])((char *)&a[2] + sizeof(int (*)[4])) - &a[2]; // expected-warning{{Subtraction of two pointers that}} + d = (int (*)[4])((char *)&a[2] + 1) - &a[2]; // expected-warning{{Subtraction of two pointers that}} } void f4(void) { @@ -114,6 +95,8 @@ void f6(void) { long long la2[3]; char *pla1 = (char *)la1; char *pla2 = (char *)la2; + d = pla1[1] - pla1[0]; + d = (long long *)&pla1[1] - &l; // expected-warning{{Subtraction of two pointers that}} d = &pla2[3] - &pla1[3]; // expected-warning{{Subtraction of two pointers that}} } @@ -121,5 +104,3 @@ void f7(int *p) { int a[10]; int d = &a[10] - p; // no-warning ('p' is unknown, even if it cannot point into 'a') } - -#endif From 45c8c416260a11c29582e577dfbff9cfa39c01e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 6 Jun 2024 17:29:34 +0200 Subject: [PATCH 4/5] check for index out of array, more exact messages --- .../Checkers/PointerSubChecker.cpp | 88 +++++++++++++++++-- clang/test/Analysis/pointer-sub.c | 22 ++++- 2 files changed, 98 insertions(+), 12 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index 87764aa369f83..b73534136fdf0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -17,6 +17,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "llvm/ADT/StringRef.h" using namespace clang; @@ -26,12 +27,84 @@ namespace { class PointerSubChecker : public Checker< check::PreStmt<BinaryOperator> > { const BugType BT{this, "Pointer subtraction"}; + 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."; + + bool checkArrayBounds(CheckerContext &C, const Expr *E, + const ElementRegion *ElemReg, + const MemRegion *Reg) const; + void reportBug(CheckerContext &C, const Expr *E, + const llvm::StringLiteral &Msg) const; public: void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; }; } +bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, + const ElementRegion *ElemReg, + const MemRegion *Reg) const { + if (!ElemReg) + return true; + + ProgramStateRef State = C.getState(); + const MemRegion *SuperReg = ElemReg->getSuperRegion(); + SValBuilder &SVB = C.getSValBuilder(); + + if (SuperReg == Reg) { + if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex()); + I && (!I->isOne() && !I->isZero())) + reportBug(C, E, 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(C, E, 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(C, E, Msg_NegativeArrayIndex); + return false; + } + } + return true; +} + +void PointerSubChecker::reportBug(CheckerContext &C, const Expr *E, + const llvm::StringLiteral &Msg) const { + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); + R->addRange(E->getSourceRange()); + C.emitReport(std::move(R)); + } +} + void PointerSubChecker::checkPreStmt(const BinaryOperator *B, CheckerContext &C) const { // When doing pointer subtraction, if the two pointers do not point to the @@ -57,6 +130,12 @@ 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)) + return; + if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR)) + return; + if (ElemLR && ElemRR) { const MemRegion *SuperLR = ElemLR->getSuperRegion(); const MemRegion *SuperRR = ElemRR->getSuperRegion(); @@ -67,14 +146,7 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, return; } - if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - constexpr llvm::StringLiteral Msg = - "Subtraction of two pointers that do not point into the same array " - "is undefined behavior."; - auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); - R->addRange(B->getSourceRange()); - C.emitReport(std::move(R)); - } + reportBug(C, B, Msg_MemRegionDifferent); } void ento::registerPointerSubChecker(CheckerManager &mgr) { diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index 423292124a309..be4e5de99faae 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -6,7 +6,16 @@ void f1(void) { d = z - &y; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} d = &x - &x; // no-warning (subtraction of any two identical pointers is allowed) d = (long *)&x - (long *)&x; - d = (&x + 1) - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} + 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}} } void f2(void) { @@ -21,7 +30,7 @@ void f2(void) { q = a + 10; d = q - p; // no warning (use of pointer to one after the end is allowed) q = a + 11; - d = q - a; // no-warning (no check for past-the-end array pointers in this checker) + 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 @@ -53,10 +62,10 @@ void f4(void) { int (*p)[m] = a; // p == &a[0] p += 1; // p == &a[1] - // FIXME: This warning is not needed + // FIXME: This is a known problem with -Wpointer-arith int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} - // FIXME: This warning is not needed + // FIXME: This is a known problem with -Wpointer-arith d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}} @@ -104,3 +113,8 @@ void f7(int *p) { int a[10]; int d = &a[10] - p; // no-warning ('p' is unknown, even if it cannot point into 'a') } + +void f8(int n) { + int a[10]; + int d = a[n] - a[0]; +} From 987d2b07bf1b4f22b986112a9dc117f4b0a0573d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Mon, 10 Jun 2024 09:56:34 +0200 Subject: [PATCH 5/5] added issue link in test --- clang/test/Analysis/pointer-sub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index be4e5de99faae..9a446547e2868 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -62,10 +62,10 @@ void f4(void) { int (*p)[m] = a; // p == &a[0] p += 1; // p == &a[1] - // FIXME: This is a known problem with -Wpointer-arith + // FIXME: This is a known problem with -Wpointer-arith (https://github.com/llvm/llvm-project/issues/28328) int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} - // FIXME: This is a known problem with -Wpointer-arith + // FIXME: This is a known problem with -Wpointer-arith (https://github.com/llvm/llvm-project/issues/28328) d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}} d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits