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.k...@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.k...@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@-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@-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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits