https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/95899
From 1eb6e7ebde0e97e1cd077dc27ffd3ebd6ed0e93d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Tue, 18 Jun 2024 10:09:24 +0200 Subject: [PATCH 1/3] [clang][analyzer] Add notes to PointerSubChecker Notes are added to indicate the array declarations of the arrays in a found invalid pointer subtraction. --- .../Checkers/PointerSubChecker.cpp | 45 ++++++++++++------- clang/test/Analysis/pointer-sub-notes.c | 34 ++++++++++++++ 2 files changed, 64 insertions(+), 15 deletions(-) create mode 100644 clang/test/Analysis/pointer-sub-notes.c diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index b73534136fdf0..a983e66df0818 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -43,8 +43,6 @@ class PointerSubChecker 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; @@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, if (!ElemReg) 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(); const MemRegion *SuperReg = ElemReg->getSuperRegion(); SValBuilder &SVB = C.getSValBuilder(); @@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, if (SuperReg == Reg) { if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex()); I && (!I->isOne() && !I->isZero())) - reportBug(C, E, Msg_BadVarIndex); + ReportBug(Msg_BadVarIndex); return false; } @@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, ProgramStateRef S1, S2; std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge); if (S1 && !S2) { - reportBug(C, E, Msg_LargeArrayIndex); + ReportBug(Msg_LargeArrayIndex); return false; } } @@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, ProgramStateRef S1, S2; std::tie(S1, S2) = State->assume(*IndexTooSmall); if (S1 && !S2) { - reportBug(C, E, Msg_NegativeArrayIndex); + ReportBug(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 @@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR)) return; + const ValueDecl *DiffDeclL = nullptr; + const ValueDecl *DiffDeclR = nullptr; + if (ElemLR && ElemRR) { const MemRegion *SuperLR = ElemLR->getSuperRegion(); const MemRegion *SuperRR = ElemRR->getSuperRegion(); @@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, // Allow arithmetic on different symbolic regions. if (isa<SymbolicRegion>(SuperLR) || isa<SymbolicRegion>(SuperRR)) return; + if (const auto *SuperDLR = dyn_cast<DeclRegion>(SuperLR)) + DiffDeclL = SuperDLR->getDecl(); + if (const auto *SuperDRR = dyn_cast<DeclRegion>(SuperRR)) + DiffDeclR = SuperDRR->getDecl(); } - reportBug(C, B, Msg_MemRegionDifferent); + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { + auto R = + std::make_unique<PathSensitiveBugReport>(BT, Msg_MemRegionDifferent, N); + R->addRange(B->getSourceRange()); + if (DiffDeclL) + R->addNote("Array at the left-hand side of subtraction", + {DiffDeclL, C.getSourceManager()}); + if (DiffDeclR) + R->addNote("Array at the right-hand side of subtraction", + {DiffDeclR, C.getSourceManager()}); + C.emitReport(std::move(R)); + } } void ento::registerPointerSubChecker(CheckerManager &mgr) { diff --git a/clang/test/Analysis/pointer-sub-notes.c b/clang/test/Analysis/pointer-sub-notes.c new file mode 100644 index 0000000000000..dfdace3a58deb --- /dev/null +++ b/clang/test/Analysis/pointer-sub-notes.c @@ -0,0 +1,34 @@ +// 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 different_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}} + int d = &a[2] - &b[0]; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ + // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} +} + +void different_2() { + int a[3]; // expected-note{{Array at the right-hand side of subtraction}} + int b[3]; // expected-note{{Array at the left-hand side of subtraction}} + int *p1 = a + 1; + int *p2 = b; + int d = p2 - p1; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ + // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} +} From de36da1d01a3dbdfb122f4599d69d482151c246a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Tue, 18 Jun 2024 15:43:47 +0200 Subject: [PATCH 2/3] check if the notes would appear at same location --- .../Checkers/PointerSubChecker.cpp | 14 ++++++++------ clang/test/Analysis/pointer-sub-notes.c | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index a983e66df0818..a29169dd1c277 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -154,12 +154,14 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg_MemRegionDifferent, N); R->addRange(B->getSourceRange()); - if (DiffDeclL) - R->addNote("Array at the left-hand side of subtraction", - {DiffDeclL, C.getSourceManager()}); - if (DiffDeclR) - R->addNote("Array at the right-hand side of subtraction", - {DiffDeclR, C.getSourceManager()}); + if (DiffDeclL != DiffDeclR) { + if (DiffDeclL) + R->addNote("Array at the left-hand side of subtraction", + {DiffDeclL, C.getSourceManager()}); + if (DiffDeclR) + R->addNote("Array at the right-hand side of subtraction", + {DiffDeclR, C.getSourceManager()}); + } C.emitReport(std::move(R)); } } diff --git a/clang/test/Analysis/pointer-sub-notes.c b/clang/test/Analysis/pointer-sub-notes.c index dfdace3a58deb..33fc2388df9c6 100644 --- a/clang/test/Analysis/pointer-sub-notes.c +++ b/clang/test/Analysis/pointer-sub-notes.c @@ -32,3 +32,20 @@ void different_2() { int d = p2 - p1; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} } + +int different_3() { + struct { + int array[5]; + } a, b; + return &a.array[3] - &b.array[2]; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ + // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} +} + +int different_5() { + struct { + int array1[5]; // expected-note{{Array at the left-hand side of subtraction}} + int array2[5]; // expected-note{{Array at the right-hand side of subtraction}} + } a; + return &a.array1[3] - &a.array2[4]; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ + // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} +} From 5f667f40d8533fc555025e9ae8f8158b3586b685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Wed, 19 Jun 2024 15:16:49 +0200 Subject: [PATCH 3/3] fixed failing tests --- clang/test/Analysis/casts.c | 2 +- clang/test/Analysis/pointer-sub-notes.c | 15 +++++++++++++++ clang/test/Analysis/pointer-sub.c | 14 ++++++++------ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c index 7dad4edfa89b9..462a9865f1564 100644 --- a/clang/test/Analysis/casts.c +++ b/clang/test/Analysis/casts.c @@ -129,7 +129,7 @@ void locAsIntegerCasts(void *p) { } void multiDimensionalArrayPointerCasts(void) { - static int x[10][10]; + static int x[10][10]; // expected-note2{{Array at the right-hand side of subtraction}} int *y1 = &(x[3][5]); char *z = ((char *) y1) + 2; int *y2 = (int *)(z - 2); diff --git a/clang/test/Analysis/pointer-sub-notes.c b/clang/test/Analysis/pointer-sub-notes.c index 33fc2388df9c6..6a74918d1e58a 100644 --- a/clang/test/Analysis/pointer-sub-notes.c +++ b/clang/test/Analysis/pointer-sub-notes.c @@ -49,3 +49,18 @@ int different_5() { return &a.array1[3] - &a.array2[4]; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} } + +void different_6() { + int d; + static int x[10][10]; // expected-note2{{Array at the left-hand side of subtraction}} + int *y1 = &(x[3][5]); + char *z = ((char *) y1) + 2; + int *y2 = (int *)(z - 2); + int *y3 = ((int *)x) + 35; // This is offset for [3][5]. + + d = y2 - y1; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ + // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} + d = y3 - y1; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}} \ + // expected-note{{Subtraction of two pointers that do not point into the same array is undefined behavior}} + d = y3 - y2; +} diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index 9a446547e2868..88e6dec2d172f 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -19,7 +19,8 @@ void f1(void) { } void f2(void) { - int a[10], b[10], c; + int a[10], b[10], c; // expected-note{{Array at the left-hand side of subtraction}} \ + // expected-note2{{Array at the right-hand side of subtraction}} int *p = &a[2]; int *q = &a[8]; int d = q - p; // no-warning (pointers into the same array) @@ -41,7 +42,8 @@ void f2(void) { } void f3(void) { - int a[3][4]; + int a[3][4]; // expected-note{{Array at the left-hand side of subtraction}} \ + // expected-note2{{Array at the right-hand side of subtraction}} int d; d = &(a[2]) - &(a[1]); @@ -74,8 +76,8 @@ void f4(void) { typedef struct { int a; int b; - int c[10]; - int d[10]; + int c[10]; // expected-note2{{Array at the right-hand side of subtraction}} + int d[10]; // expected-note2{{Array at the left-hand side of subtraction}} } S; void f5(void) { @@ -100,8 +102,8 @@ void f6(void) { char *a1 = (char *)&l; int d = a1[3] - l; - long long la1[3]; - long long la2[3]; + long long la1[3]; // expected-note{{Array at the right-hand side of subtraction}} + long long la2[3]; // expected-note{{Array at the left-hand side of subtraction}} char *pla1 = (char *)la1; char *pla2 = (char *)la2; d = pla1[1] - pla1[0]; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits