[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 19 06:17:19 PDT 2024


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.keri at 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.keri at 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.keri at 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];



More information about the cfe-commits mailing list