[clang] [clang][analyzer] Bring checker 'alpha.unix.cstring.NotNullTerminated' out of alpha (PR #113899)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 25 05:58:45 PST 2024
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/113899
>From 9cf4203652f06a140288a5c1ab6d14bcc3612380 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 28 Oct 2024 11:23:55 +0100
Subject: [PATCH 1/3] [clang][analyzer] Bring checker
'alpha.unix.cstring.NotNullTerminated' out of alpha
---
clang/docs/analyzer/checkers.rst | 46 +++++++++----------
.../clang/StaticAnalyzer/Checkers/Checkers.td | 11 +++--
.../test/Analysis/analyzer-enabled-checkers.c | 1 +
clang/test/Analysis/bstring.cpp | 2 +-
...c-library-functions-arg-enabled-checkers.c | 1 +
clang/test/Analysis/string.c | 2 +-
clang/test/Analysis/string.cpp | 4 ++
7 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 58dbd686a6dc9f..15c08081e174d4 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1901,6 +1901,29 @@ Check the size argument passed into C string functions for common erroneous patt
.. _unix-cstring-NullArg:
+.. _alpha-unix-cstring-NotNullTerminated:
+
+unix.cstring.NotNullTerminated (C)
+""""""""""""""""""""""""""""""""""
+Check for arguments which are not null-terminated strings;
+applies to the ``strlen``, ``strcpy``, ``strcat``, ``strcmp`` family of functions.
+
+Only very fundamental cases are detected where the passed memory block is
+absolutely different from a null-terminated string. This checker does not
+find if a memory buffer is passed where the terminating zero character
+is missing.
+
+.. code-block:: c
+
+ void test1() {
+ int l = strlen((char *)&test); // warn
+ }
+
+ void test2() {
+ label:
+ int l = strlen((char *)&&label); // warn
+ }
+
unix.cstring.NullArg (C)
""""""""""""""""""""""""
Check for null pointers being passed as arguments to C string functions:
@@ -3367,29 +3390,6 @@ Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy, wmem
memcpy(a + 2, a + 1, 8); // warn
}
-.. _alpha-unix-cstring-NotNullTerminated:
-
-alpha.unix.cstring.NotNullTerminated (C)
-""""""""""""""""""""""""""""""""""""""""
-Check for arguments which are not null-terminated strings;
-applies to the ``strlen``, ``strcpy``, ``strcat``, ``strcmp`` family of functions.
-
-Only very fundamental cases are detected where the passed memory block is
-absolutely different from a null-terminated string. This checker does not
-find if a memory buffer is passed where the terminating zero character
-is missing.
-
-.. code-block:: c
-
- void test1() {
- int l = strlen((char *)&test); // warn
- }
-
- void test2() {
- label:
- int l = strlen((char *)&&label); // warn
- }
-
.. _alpha-unix-cstring-OutOfBounds:
alpha.unix.cstring.OutOfBounds (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 349040c15eeb83..7ce2b26a27dd27 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -459,6 +459,12 @@ def CStringModeling : Checker<"CStringModeling">,
Documentation<NotDocumented>,
Hidden;
+def CStringNotNullTerm : Checker<"NotNullTerminated">,
+ HelpText<"Check for arguments passed to C string functions which are not "
+ "null-terminated strings">,
+ Dependencies<[CStringModeling]>,
+ Documentation<HasDocumentation>;
+
def CStringNullArg : Checker<"NullArg">,
HelpText<"Check for null pointers being passed as arguments to C string "
"functions">,
@@ -485,11 +491,6 @@ def CStringBufferOverlap : Checker<"BufferOverlap">,
Dependencies<[CStringModeling]>,
Documentation<HasDocumentation>;
-def CStringNotNullTerm : Checker<"NotNullTerminated">,
- HelpText<"Check for arguments which are not null-terminating strings">,
- Dependencies<[CStringModeling]>,
- Documentation<HasDocumentation>;
-
def CStringUninitializedRead : Checker<"UninitializedRead">,
HelpText<"Checks if the string manipulation function would read uninitialized bytes">,
Dependencies<[CStringModeling]>,
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index e605c62a66ad0e..a84a0c2211fde0 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -53,6 +53,7 @@
// CHECK-NEXT: unix.StdCLibraryFunctions
// CHECK-NEXT: unix.Vfork
// CHECK-NEXT: unix.cstring.BadSizeArg
+// CHECK-NEXT: unix.cstring.NotNullTerminated
// CHECK-NEXT: unix.cstring.NullArg
int main() {
diff --git a/clang/test/Analysis/bstring.cpp b/clang/test/Analysis/bstring.cpp
index 1b6397c3455ebd..9c30bef15d407a 100644
--- a/clang/test/Analysis/bstring.cpp
+++ b/clang/test/Analysis/bstring.cpp
@@ -2,7 +2,7 @@
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,unix.Malloc,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,unix.Malloc,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,unix.cstring.NotNullTerminated,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
#include "Inputs/system-header-simulator-cxx.h"
#include "Inputs/system-header-simulator-for-malloc.h"
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..3d1d3c561a5580 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
@@ -61,6 +61,7 @@
// CHECK-NEXT: unix.StdCLibraryFunctions
// CHECK-NEXT: unix.Vfork
// CHECK-NEXT: unix.cstring.BadSizeArg
+// CHECK-NEXT: unix.cstring.NotNullTerminated
// CHECK-NEXT: unix.cstring.NullArg
int main() {
diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c
index 2e0a49d083b0b0..e017aff3b4a1db 100644
--- a/clang/test/Analysis/string.c
+++ b/clang/test/Analysis/string.c
@@ -38,7 +38,7 @@
// RUN: -analyzer-checker=unix.cstring \
// RUN: -analyzer-checker=unix.Malloc \
// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap \
-// RUN: -analyzer-checker=alpha.unix.cstring.NotNullTerminated \
+// RUN: -analyzer-checker=unix.cstring.NotNullTerminated \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false
diff --git a/clang/test/Analysis/string.cpp b/clang/test/Analysis/string.cpp
index c09422d1922369..e6cc950f30c9a0 100644
--- a/clang/test/Analysis/string.cpp
+++ b/clang/test/Analysis/string.cpp
@@ -53,3 +53,7 @@ struct TestNotNullTerm {
strlen((char *)&x); // expected-warning{{Argument to string length function is not a null-terminated string}}
}
};
+
+void test_notcstring_tempobject() {
+ strlen((char[]){'a', 0}); // expected-warning{{Argument to string length function is a C++ temp object of type char[2], which is not a null-terminated string}}
+}
>From 85e786ccb39a1440fb7e4134e81e6752f5fe8d7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 28 Oct 2024 16:23:47 +0100
Subject: [PATCH 2/3] fixed checkers.rst
---
clang/docs/analyzer/checkers.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 15c08081e174d4..cdf4ae0caeb4be 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1899,9 +1899,7 @@ Check the size argument passed into C string functions for common erroneous patt
// warn: potential buffer overflow
}
-.. _unix-cstring-NullArg:
-
-.. _alpha-unix-cstring-NotNullTerminated:
+.. _unix-cstring-NotNullTerminated:
unix.cstring.NotNullTerminated (C)
""""""""""""""""""""""""""""""""""
@@ -1924,6 +1922,8 @@ is missing.
int l = strlen((char *)&&label); // warn
}
+.. _unix-cstring-NullArg:
+
unix.cstring.NullArg (C)
""""""""""""""""""""""""
Check for null pointers being passed as arguments to C string functions:
>From 0d5c3dad561c9477250a7456eff1de0269cd492a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 25 Nov 2024 10:27:08 +0100
Subject: [PATCH 3/3] small test and documentation fixes
---
clang/docs/analyzer/checkers.rst | 2 +-
clang/test/Analysis/string.cpp | 3 +++
clang/test/Analysis/string_notnullterm.c | 10 ++++++++++
3 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Analysis/string_notnullterm.c
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index cdf4ae0caeb4be..9a6bb57557c972 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1914,7 +1914,7 @@ is missing.
.. code-block:: c
void test1() {
- int l = strlen((char *)&test); // warn
+ int l = strlen((char *)&test1); // warn
}
void test2() {
diff --git a/clang/test/Analysis/string.cpp b/clang/test/Analysis/string.cpp
index e6cc950f30c9a0..5e4580aa6ce1a6 100644
--- a/clang/test/Analysis/string.cpp
+++ b/clang/test/Analysis/string.cpp
@@ -55,5 +55,8 @@ struct TestNotNullTerm {
};
void test_notcstring_tempobject() {
+ // FIXME: This warning from cstring.NotNullTerminated is a false positive.
+ // Handling of similar cases is not perfect in other cstring checkers.
+ // The fix would be a larger change in CStringChecker and affect multiple checkers.
strlen((char[]){'a', 0}); // expected-warning{{Argument to string length function is a C++ temp object of type char[2], which is not a null-terminated string}}
}
diff --git a/clang/test/Analysis/string_notnullterm.c b/clang/test/Analysis/string_notnullterm.c
new file mode 100644
index 00000000000000..9c6a6713e615b0
--- /dev/null
+++ b/clang/test/Analysis/string_notnullterm.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=unix.cstring.NotNullTerminated \
+// RUN: -analyzer-checker=debug.ExprInspection
+
+char *strcpy(char *restrict s1, const char *restrict s2);
+
+void strcpy_fn(char *x) {
+ strcpy(x, (char*)&strcpy_fn); // expected-warning{{Argument to string copy function is the address of the function 'strcpy_fn', which is not a null-terminated string}}
+}
More information about the cfe-commits
mailing list