[clang] [clang][analyzer] Move 'alpha.core.PointerSub' checker into 'core.PointerSub' (PR #107596)

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 27 02:11:11 PDT 2024


https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/107596

>From be6fbcad245bd16b013e9337270e0ade23a5b9c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Fri, 6 Sep 2024 16:30:59 +0200
Subject: [PATCH 1/2] [clang][analyzer] Move 'alpha.core.PointerSub' checker
 into 'core.PointerSub'

---
 clang/docs/analyzer/checkers.rst              | 86 +++++++++----------
 .../clang/StaticAnalyzer/Checkers/Checkers.td | 10 +--
 .../test/Analysis/analyzer-enabled-checkers.c |  1 +
 clang/test/Analysis/pointer-sub-notes.c       |  2 +-
 clang/test/Analysis/pointer-sub.c             |  2 +-
 ...c-library-functions-arg-enabled-checkers.c |  1 +
 6 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 847bf4baf74887..05e7b985d52cec 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -170,6 +170,49 @@ disable this behavior with the option
    obj->x = 1; // warn
  }
 
+.. _core-PointerSub:
+
+core.PointerSub (C)
+"""""""""""""""""""
+Check for pointer subtractions on two pointers pointing to different 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). This checker only
+searches for different 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
+
+ void test() {
+   int a, b, c[10], d[10];
+   int x = &c[3] - &c[1];
+   x = &d[4] - &c[1]; // warn: 'c' and 'd' are different arrays
+   x = (&a + 1) - &a;
+   x = &b - &a; // warn: 'a' and 'b' are different variables
+ }
+
+ struct S {
+   int x[10];
+   int y[10];
+ };
+
+ void test1() {
+   struct S a[10];
+   struct S b;
+   int d = &a[4] - &a[6];
+   d = &a[0].x[3] - &a[0].x[1];
+   d = a[0].y - a[0].x; // warn: 'S.b' and 'S.a' are different objects
+   d = (char *)&b.y - (char *)&b.x; // warn: different members of the same object
+   d = (char *)&b.y - (char *)&b; // warn: object of type S is not the same array as a member of it
+ }
+
+There may be existing applications that use code like above for calculating
+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.
+
 .. _core-StackAddressEscape:
 
 core.StackAddressEscape (C)
@@ -2526,49 +2569,6 @@ Check for pointer arithmetic on locations other than array elements.
    p = &x + 1; // warn
  }
 
-.. _alpha-core-PointerSub:
-
-alpha.core.PointerSub (C)
-"""""""""""""""""""""""""
-Check for pointer subtractions on two pointers pointing to different 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). This checker only
-searches for different 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
-
- void test() {
-   int a, b, c[10], d[10];
-   int x = &c[3] - &c[1];
-   x = &d[4] - &c[1]; // warn: 'c' and 'd' are different arrays
-   x = (&a + 1) - &a;
-   x = &b - &a; // warn: 'a' and 'b' are different variables
- }
-
- struct S {
-   int x[10];
-   int y[10];
- };
-
- void test1() {
-   struct S a[10];
-   struct S b;
-   int d = &a[4] - &a[6];
-   d = &a[0].x[3] - &a[0].x[1];
-   d = a[0].y - a[0].x; // warn: 'S.b' and 'S.a' are different objects
-   d = (char *)&b.y - (char *)&b.x; // warn: different members of the same object
-   d = (char *)&b.y - (char *)&b; // warn: object of type S is not the same array as a member of it
- }
-
-There may be existing applications that use code like above for calculating
-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.
-
 .. _alpha-core-StackAddressAsyncEscape:
 
 alpha.core.StackAddressAsyncEscape (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 585246547b3dce..f4167b18b4879a 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -257,6 +257,11 @@ def NonnullGlobalConstantsChecker: Checker<"NonnilStringConstants">,
   Documentation<NotDocumented>,
   Hidden;
 
+def PointerSubChecker : Checker<"PointerSub">,
+  HelpText<"Check for pointer subtractions on two pointers pointing to "
+           "different memory chunks">,
+  Documentation<HasDocumentation>;
+
 } // end "core"
 
 let ParentPackage = CoreAlpha in {
@@ -291,11 +296,6 @@ def PointerArithChecker : Checker<"PointerArithm">,
            "elements">,
   Documentation<HasDocumentation>;
 
-def PointerSubChecker : Checker<"PointerSub">,
-  HelpText<"Check for pointer subtractions on two pointers pointing to "
-           "different memory chunks">,
-  Documentation<HasDocumentation>;
-
 def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
   HelpText<"Check for division by variable that is later compared against 0. "
            "Either the comparison is useless or there is division by zero.">,
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index e605c62a66ad0e..db0c2f8366e38c 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -19,6 +19,7 @@
 // CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
+// CHECK-NEXT: core.PointerSub
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult
diff --git a/clang/test/Analysis/pointer-sub-notes.c b/clang/test/Analysis/pointer-sub-notes.c
index 59681b4e7555af..95653fb352dbd9 100644
--- a/clang/test/Analysis/pointer-sub-notes.c
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.PointerSub -analyzer-output=text -verify %s
 
 void different_1() {
   int a[3]; // expected-note{{Array at the left-hand side of subtraction}}
diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c
index cf9eac1abc2dcc..649a60f545e810 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core.PointerSub -analyzer-output=text-minimal -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text-minimal -verify %s
 
 void f1(void) {
   int x, y, z[10];
diff --git a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
index 345a4e8f44efd1..7cf20011ba81ca 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -27,6 +27,7 @@
 // CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
+// CHECK-NEXT: core.PointerSub
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult

>From 5729e1618c7f648dd2de5efd169a9f89d635cca3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Fri, 27 Sep 2024 11:09:16 +0200
Subject: [PATCH 2/2] move checker to 'security' package

---
 clang/docs/analyzer/checkers.rst              | 86 +++++++++----------
 .../clang/StaticAnalyzer/Checkers/Checkers.td | 10 +--
 .../test/Analysis/analyzer-enabled-checkers.c |  1 -
 clang/test/Analysis/casts.c                   |  6 +-
 clang/test/Analysis/pointer-sub-notes.c       |  2 +-
 clang/test/Analysis/pointer-sub.c             |  2 +-
 ...c-library-functions-arg-enabled-checkers.c |  1 -
 7 files changed, 51 insertions(+), 57 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 05e7b985d52cec..bcf637665f676b 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -170,49 +170,6 @@ disable this behavior with the option
    obj->x = 1; // warn
  }
 
-.. _core-PointerSub:
-
-core.PointerSub (C)
-"""""""""""""""""""
-Check for pointer subtractions on two pointers pointing to different 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). This checker only
-searches for different 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
-
- void test() {
-   int a, b, c[10], d[10];
-   int x = &c[3] - &c[1];
-   x = &d[4] - &c[1]; // warn: 'c' and 'd' are different arrays
-   x = (&a + 1) - &a;
-   x = &b - &a; // warn: 'a' and 'b' are different variables
- }
-
- struct S {
-   int x[10];
-   int y[10];
- };
-
- void test1() {
-   struct S a[10];
-   struct S b;
-   int d = &a[4] - &a[6];
-   d = &a[0].x[3] - &a[0].x[1];
-   d = a[0].y - a[0].x; // warn: 'S.b' and 'S.a' are different objects
-   d = (char *)&b.y - (char *)&b.x; // warn: different members of the same object
-   d = (char *)&b.y - (char *)&b; // warn: object of type S is not the same array as a member of it
- }
-
-There may be existing applications that use code like above for calculating
-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.
-
 .. _core-StackAddressEscape:
 
 core.StackAddressEscape (C)
@@ -1352,6 +1309,49 @@ Warn on ``mmap()`` calls with both writable and executable access.
    //       code
  }
 
+.. _security-PointerSub:
+
+security.PointerSub (C)
+"""""""""""""""""""""""
+Check for pointer subtractions on two pointers pointing to different 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). This checker only
+searches for different 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
+
+ void test() {
+   int a, b, c[10], d[10];
+   int x = &c[3] - &c[1];
+   x = &d[4] - &c[1]; // warn: 'c' and 'd' are different arrays
+   x = (&a + 1) - &a;
+   x = &b - &a; // warn: 'a' and 'b' are different variables
+ }
+
+ struct S {
+   int x[10];
+   int y[10];
+ };
+
+ void test1() {
+   struct S a[10];
+   struct S b;
+   int d = &a[4] - &a[6];
+   d = &a[0].x[3] - &a[0].x[1];
+   d = a[0].y - a[0].x; // warn: 'S.b' and 'S.a' are different objects
+   d = (char *)&b.y - (char *)&b.x; // warn: different members of the same object
+   d = (char *)&b.y - (char *)&b; // warn: object of type S is not the same array as a member of it
+ }
+
+There may be existing applications that use code like above for calculating
+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.
+
 .. _security-putenv-stack-array:
 
 security.PutenvStackArray (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index f4167b18b4879a..757eda90446390 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -257,11 +257,6 @@ def NonnullGlobalConstantsChecker: Checker<"NonnilStringConstants">,
   Documentation<NotDocumented>,
   Hidden;
 
-def PointerSubChecker : Checker<"PointerSub">,
-  HelpText<"Check for pointer subtractions on two pointers pointing to "
-           "different memory chunks">,
-  Documentation<HasDocumentation>;
-
 } // end "core"
 
 let ParentPackage = CoreAlpha in {
@@ -1004,6 +999,11 @@ def MmapWriteExecChecker : Checker<"MmapWriteExec">,
   HelpText<"Warn on mmap() calls with both writable and executable access">,
   Documentation<HasDocumentation>;
 
+def PointerSubChecker : Checker<"PointerSub">,
+  HelpText<"Check for pointer subtractions on two pointers pointing to "
+           "different memory chunks">,
+  Documentation<HasDocumentation>;
+
 def PutenvStackArray : Checker<"PutenvStackArray">,
   HelpText<"Finds calls to the function 'putenv' which pass a pointer to "
            "an automatic (stack-allocated) array as the argument.">,
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index db0c2f8366e38c..e605c62a66ad0e 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -19,7 +19,6 @@
 // CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
-// CHECK-NEXT: core.PointerSub
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult
diff --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c
index 462a9865f15640..30cd74be564fda 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]; // expected-note2{{Array at the right-hand side of subtraction}}
+  static int x[10][10];
   int *y1 = &(x[3][5]);
   char *z = ((char *) y1) + 2;
   int *y2 = (int *)(z - 2);
@@ -138,9 +138,7 @@ 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 at -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}}
 
@@ -149,9 +147,7 @@ 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 at -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-notes.c b/clang/test/Analysis/pointer-sub-notes.c
index 95653fb352dbd9..7f94d6544d0f84 100644
--- a/clang/test/Analysis/pointer-sub-notes.c
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core.PointerSub -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=security.PointerSub -analyzer-output=text -verify %s
 
 void different_1() {
   int a[3]; // expected-note{{Array at the left-hand side of subtraction}}
diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c
index 649a60f545e810..1c9d676ebb8f24 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text-minimal -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=security.PointerSub -analyzer-output=text-minimal -verify %s
 
 void f1(void) {
   int x, y, z[10];
diff --git a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
index 7cf20011ba81ca..345a4e8f44efd1 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -27,7 +27,6 @@
 // CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
-// CHECK-NEXT: core.PointerSub
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult



More information about the cfe-commits mailing list