From 0d384fe978f9675be9fe960940daa2fd599104ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= Date: Mon, 30 Sep 2024 09:16:27 +0200 Subject: [PATCH] [clang][analyzer] Move 'alpha.core.PointerSub' checker into 'security.PointerSub' (#107596) --- clang/docs/analyzer/checkers.rst | 86 +++++++++---------- .../clang/StaticAnalyzer/Checkers/Checkers.td | 10 +-- clang/test/Analysis/casts.c | 6 +- clang/test/Analysis/pointer-sub-notes.c | 2 +- clang/test/Analysis/pointer-sub.c | 2 +- 5 files changed, 51 insertions(+), 55 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 9847d449d76d04..a22bda189dd295 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -1544,6 +1544,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) @@ -2761,49 +2804,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 (ObjC) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 747ebd8c2e4dee..6bc389f9da265f 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -290,11 +290,6 @@ def PointerArithChecker : Checker<"PointerArithm">, "elements">, Documentation; -def PointerSubChecker : Checker<"PointerSub">, - HelpText<"Check for pointer subtractions on two pointers pointing to " - "different memory chunks">, - Documentation; - 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.">, @@ -1003,6 +998,11 @@ def MmapWriteExecChecker : Checker<"MmapWriteExec">, HelpText<"Warn on mmap() calls with both writable and executable access">, Documentation; +def PointerSubChecker : Checker<"PointerSub">, + HelpText<"Check for pointer subtractions on two pointers pointing to " + "different memory chunks">, + Documentation; + 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/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 59681b4e7555af..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=alpha.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 cf9eac1abc2dcc..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,alpha.core.PointerSub -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];