llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Balázs Kéri (balazske) <details> <summary>Changes</summary> The checker is made more exact (only pointer into array is allowed) and more tests are added. --- Full diff: https://github.com/llvm/llvm-project/pull/93676.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp (+20-14) - (added) clang/test/Analysis/pointer-sub.c (+74) - (modified) clang/test/Analysis/ptr-arith.c (+2-12) ``````````diff 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}} `````````` </details> https://github.com/llvm/llvm-project/pull/93676 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
