[clang][analyzer] Move StreamChecker out of the alpha package. #89247

balazske · 2024-04-18T15:10:46Z

No description provided.

llvmbot · 2024-04-18T15:11:19Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balázs Kéri (balazske)

Changes

Patch is 25.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89247.diff

20 Files Affected:

(modified) clang/docs/analyzer/checkers.rst (+93-93)
(modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+16-15)
(modified) clang/test/Analysis/analyzer-config.c (+1-1)
(modified) clang/test/Analysis/analyzer-enabled-checkers.c (+1)
(modified) clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c (+3-3)
(modified) clang/test/Analysis/std-c-library-functions-arg-weakdeps.c (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-vs-stream-checker.c (+4-4)
(modified) clang/test/Analysis/stream-errno-note.c (+2-2)
(modified) clang/test/Analysis/stream-errno.c (+2-2)
(modified) clang/test/Analysis/stream-error.c (+2-2)
(modified) clang/test/Analysis/stream-invalidate.c (+1-1)
(modified) clang/test/Analysis/stream-non-posix-function.c (+1-1)
(modified) clang/test/Analysis/stream-noopen.c (+1-1)
(modified) clang/test/Analysis/stream-note.c (+4-4)
(modified) clang/test/Analysis/stream-pedantic.c (+4-4)
(modified) clang/test/Analysis/stream-stdlibraryfunctionargs.c (+5-5)
(modified) clang/test/Analysis/stream.c (+8-8)
(modified) clang/test/Analysis/stream.cpp (+1-1)
(modified) clang/www/analyzer/alpha_checks.html (+2-2)
(modified) clang/www/analyzer/open_projects.html (+1-1)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index fb748d23a53d01..32c2a312962754 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1462,6 +1462,99 @@ checker).
 
 Default value of the option is ``true``.
 
+.. _unix-Stream:
+
+unix.Stream (C)
+"""""""""""""""""""""
+Check C stream handling functions:
+``fopen, fdopen, freopen, tmpfile, fclose, fread, fwrite, fgetc, fgets, fputc, fputs, fprintf, fscanf, ungetc, getdelim, getline, fseek, fseeko, ftell, ftello, fflush, rewind, fgetpos, fsetpos, clearerr, feof, ferror, fileno``.
+
+The checker maintains information about the C stream objects (``FILE *``) and
+can detect error conditions related to use of streams. The following conditions
+are detected:
+
+* The ``FILE *`` pointer passed to the function is NULL (the single exception is
+  ``fflush`` where NULL is allowed).
+* Use of stream after close.
+* Opened stream is not closed.
+* Read from a stream after end-of-file. (This is not a fatal error but reported
+  by the checker. Stream remains in EOF state and the read operation fails.)
+* Use of stream when the file position is indeterminate after a previous failed
+  operation. Some functions (like ``ferror``, ``clearerr``, ``fseek``) are
+  allowed in this state.
+* Invalid 3rd ("``whence``") argument to ``fseek``.
+
+The stream operations are by this checker usually split into two cases, a success
+and a failure case. However, in the case of write operations (like ``fwrite``,
+``fprintf`` and even ``fsetpos``) this behavior could produce a large amount of
+unwanted reports on projects that don't have error checks around the write
+operations, so by default the checker assumes that write operations always succeed.
+This behavior can be controlled by the ``Pedantic`` flag: With
+``-analyzer-config unix.Stream:Pedantic=true`` the checker will model the
+cases where a write operation fails and report situations where this leads to
+erroneous behavior. (The default is ``Pedantic=false``, where write operations
+are assumed to succeed.)
+
+.. code-block:: c
+
+ void test1() {
+   FILE *p = fopen("foo", "r");
+ } // warn: opened file is never closed
+
+ void test2() {
+   FILE *p = fopen("foo", "r");
+   fseek(p, 1, SEEK_SET); // warn: stream pointer might be NULL
+   fclose(p);
+ }
+
+ void test3() {
+   FILE *p = fopen("foo", "r");
+   if (p) {
+     fseek(p, 1, 3); // warn: third arg should be SEEK_SET, SEEK_END, or SEEK_CUR
+     fclose(p);
+   }
+ }
+
+ void test4() {
+   FILE *p = fopen("foo", "r");
+   if (!p)
+     return;
+
+   fclose(p);
+   fclose(p); // warn: stream already closed
+ }
+
+ void test5() {
+   FILE *p = fopen("foo", "r");
+   if (!p)
+     return;
+
+   fgetc(p);
+   if (!ferror(p))
+     fgetc(p); // warn: possible read after end-of-file
+
+   fclose(p);
+ }
+
+ void test6() {
+   FILE *p = fopen("foo", "r");
+   if (!p)
+     return;
+
+   fgetc(p);
+   if (!feof(p))
+     fgetc(p); // warn: file position may be indeterminate after I/O error
+
+   fclose(p);
+ }
+
+**Limitations**
+
+The checker does not track the correspondence between integer file descriptors
+and ``FILE *`` pointers. Operations on standard streams like ``stdin`` are not
+treated specially and are therefore often not recognized (because these streams
+are usually not opened explicitly by the program, and are global variables).
+
 .. _osx-checkers:
 
 osx
@@ -3116,99 +3209,6 @@ Check for misuses of stream APIs. Check for misuses of stream APIs: ``fopen, fcl
    fclose(F); // warn: closing a previously closed file stream
  }
 
-.. _alpha-unix-Stream:
-
-alpha.unix.Stream (C)
-"""""""""""""""""""""
-Check C stream handling functions:
-``fopen, fdopen, freopen, tmpfile, fclose, fread, fwrite, fgetc, fgets, fputc, fputs, fprintf, fscanf, ungetc, getdelim, getline, fseek, fseeko, ftell, ftello, fflush, rewind, fgetpos, fsetpos, clearerr, feof, ferror, fileno``.
-
-The checker maintains information about the C stream objects (``FILE *``) and
-can detect error conditions related to use of streams. The following conditions
-are detected:
-
-* The ``FILE *`` pointer passed to the function is NULL (the single exception is
-  ``fflush`` where NULL is allowed).
-* Use of stream after close.
-* Opened stream is not closed.
-* Read from a stream after end-of-file. (This is not a fatal error but reported
-  by the checker. Stream remains in EOF state and the read operation fails.)
-* Use of stream when the file position is indeterminate after a previous failed
-  operation. Some functions (like ``ferror``, ``clearerr``, ``fseek``) are
-  allowed in this state.
-* Invalid 3rd ("``whence``") argument to ``fseek``.
-
-The stream operations are by this checker usually split into two cases, a success
-and a failure case. However, in the case of write operations (like ``fwrite``,
-``fprintf`` and even ``fsetpos``) this behavior could produce a large amount of
-unwanted reports on projects that don't have error checks around the write
-operations, so by default the checker assumes that write operations always succeed.
-This behavior can be controlled by the ``Pedantic`` flag: With
-``-analyzer-config alpha.unix.Stream:Pedantic=true`` the checker will model the
-cases where a write operation fails and report situations where this leads to
-erroneous behavior. (The default is ``Pedantic=false``, where write operations
-are assumed to succeed.)
-
-.. code-block:: c
-
- void test1() {
-   FILE *p = fopen("foo", "r");
- } // warn: opened file is never closed
-
- void test2() {
-   FILE *p = fopen("foo", "r");
-   fseek(p, 1, SEEK_SET); // warn: stream pointer might be NULL
-   fclose(p);
- }
-
- void test3() {
-   FILE *p = fopen("foo", "r");
-   if (p) {
-     fseek(p, 1, 3); // warn: third arg should be SEEK_SET, SEEK_END, or SEEK_CUR
-     fclose(p);
-   }
- }
-
- void test4() {
-   FILE *p = fopen("foo", "r");
-   if (!p)
-     return;
-
-   fclose(p);
-   fclose(p); // warn: stream already closed
- }
-
- void test5() {
-   FILE *p = fopen("foo", "r");
-   if (!p)
-     return;
-
-   fgetc(p);
-   if (!ferror(p))
-     fgetc(p); // warn: possible read after end-of-file
-
-   fclose(p);
- }
-
- void test6() {
-   FILE *p = fopen("foo", "r");
-   if (!p)
-     return;
-
-   fgetc(p);
-   if (!feof(p))
-     fgetc(p); // warn: file position may be indeterminate after I/O error
-
-   fclose(p);
- }
-
-**Limitations**
-
-The checker does not track the correspondence between integer file descriptors
-and ``FILE *`` pointers. Operations on standard streams like ``stdin`` are not
-treated specially and are therefore often not recognized (because these streams
-are usually not opened explicitly by the program, and are global variables).
-
 .. _alpha-unix-cstring-BufferOverlap:
 
 alpha.unix.cstring.BufferOverlap (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 9aa1c6ddfe4492..d1e09fc136234d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -563,6 +563,20 @@ def MismatchedDeallocatorChecker : Checker<"MismatchedDeallocator">,
   Dependencies<[DynamicMemoryModeling]>,
   Documentation<HasDocumentation>;
 
+def StreamChecker : Checker<"Stream">,
+  HelpText<"Check stream handling functions">,
+  WeakDependencies<[NonNullParamChecker]>,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "Pedantic",
+                  "If false, assume that stream operations which are often not "
+                  "checked for error do not fail."
+                  "fail.",
+                  "false",
+                  InAlpha>
+  ]>,
+  Documentation<HasDocumentation>;
+
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Check for invalid arguments of C standard library functions, "
            "and apply relations between arguments and return value">,
@@ -581,7 +595,7 @@ def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
                   "true",
                   InAlpha>
   ]>,
-  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
+  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
   Documentation<HasDocumentation>;
 
 def VforkChecker : Checker<"Vfork">,
@@ -601,20 +615,6 @@ def PthreadLockChecker : Checker<"PthreadLock">,
   Dependencies<[PthreadLockBase]>,
   Documentation<HasDocumentation>;
 
-def StreamChecker : Checker<"Stream">,
-  HelpText<"Check stream handling functions">,
-  WeakDependencies<[NonNullParamChecker]>,
-  CheckerOptions<[
-    CmdLineOption<Boolean,
-                  "Pedantic",
-                  "If false, assume that stream operations which are often not "
-                  "checked for error do not fail."
-                  "fail.",
-                  "false",
-                  InAlpha>
-  ]>,
-  Documentation<HasDocumentation>;
-
 def SimpleStreamChecker : Checker<"SimpleStream">,
   HelpText<"Check for misuses of stream APIs">,
   Documentation<HasDocumentation>;
@@ -1628,6 +1628,7 @@ def TaintTesterChecker : Checker<"TaintTest">,
 def StreamTesterChecker : Checker<"StreamTester">,
   HelpText<"Add test functions to StreamChecker for test and debugging "
            "purposes.">,
+  WeakDependencies<[StreamChecker]>,
   Documentation<NotDocumented>;
 
 def ErrnoTesterChecker : Checker<"ErrnoTest">,
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 23e37a856d09f7..fda920fa3951e0 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -12,7 +12,6 @@
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
-// CHECK-NEXT: alpha.unix.Stream:Pedantic = false
 // CHECK-NEXT: apply-fixits = false
 // CHECK-NEXT: assume-controlled-environment = false
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
@@ -131,6 +130,7 @@
 // CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
 // CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
 // CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = true
+// CHECK-NEXT: unix.Stream:Pedantic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: verbose-report-filename = false
 // CHECK-NEXT: widen-loops = false
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index 2c4e34f4990bf5..9543ba8ec02fc1 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -48,6 +48,7 @@
 // CHECK-NEXT: unix.Malloc
 // CHECK-NEXT: unix.MallocSizeof
 // CHECK-NEXT: unix.MismatchedDeallocator
+// CHECK-NEXT: unix.Stream
 // CHECK-NEXT: unix.StdCLibraryFunctions
 // CHECK-NEXT: unix.Vfork
 // CHECK-NEXT: unix.cstring.BadSizeArg
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 d4721c0a59a3d0..14aca5a948bf4b 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
@@ -6,7 +6,7 @@
 // RUN:   -Xclang -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -Xclang -analyzer-config \
 // RUN:      -Xclang unix.StdCLibraryFunctions:ModelPOSIX=true \
-// RUN:   -Xclang -analyzer-checker=alpha.unix.Stream \
+// RUN:   -Xclang -analyzer-checker=unix.Stream \
 // RUN:   -Xclang -analyzer-list-enabled-checkers \
 // RUN:   -Xclang -analyzer-display-progress \
 // RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
@@ -14,8 +14,6 @@
 
 // CHECK:      OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
-// CHECK-NEXT: core.NonNullParamChecker
-// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.Errno
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.TrustReturnsNonnull
@@ -26,6 +24,7 @@
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
+// CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
 // CHECK-NEXT: core.StackAddrEscapeBase
@@ -57,6 +56,7 @@
 // CHECK-NEXT: unix.Malloc
 // CHECK-NEXT: unix.MallocSizeof
 // CHECK-NEXT: unix.MismatchedDeallocator
+// CHECK-NEXT: unix.Stream
 // CHECK-NEXT: unix.StdCLibraryFunctions
 // CHECK-NEXT: unix.Vfork
 // CHECK-NEXT: unix.cstring.BadSizeArg
diff --git a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
index 5df5a770015b50..1f0d3627fae340 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
@@ -3,7 +3,7 @@
 
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core \
-// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-checker=unix.Stream \
 // RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -triple x86_64-unknown-linux-gnu \
@@ -57,5 +57,5 @@ void test_notnull_arg(FILE *F) {
 
 void test_notnull_stream_arg(void) {
   fileno(0); // \
-  // expected-warning{{Stream pointer might be NULL [alpha.unix.Stream]}}
+  // expected-warning{{Stream pointer might be NULL [unix.Stream]}}
 }
diff --git a/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c b/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
index cac3fe5c5151cd..b99cc30149c912 100644
--- a/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
+++ b/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
@@ -1,7 +1,7 @@
 // Check the case when only the StreamChecker is enabled.
 // RUN: %clang_analyze_cc1 %s \
-// RUN:   -analyzer-checker=core,alpha.unix.Stream \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN:   -analyzer-checker=core,unix.Stream \
+// RUN:   -analyzer-config unix.Stream:Pedantic=true \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config eagerly-assume=false \
 // RUN:   -triple x86_64-unknown-linux \
@@ -19,8 +19,8 @@
 // Check the case when both the StreamChecker and the
 // StdLibraryFunctionsChecker are enabled.
 // RUN: %clang_analyze_cc1 %s \
-// RUN:   -analyzer-checker=core,alpha.unix.Stream \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN:   -analyzer-checker=core,unix.Stream \
+// RUN:   -analyzer-config unix.Stream:Pedantic=true \
 // RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:DisplayLoadedSummaries=true \
 // RUN:   -analyzer-checker=debug.ExprInspection \
diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c
index fb12f0bace937f..71ea026ed4de33 100644
--- a/clang/test/Analysis/stream-errno-note.c
+++ b/clang/test/Analysis/stream-errno-note.c
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core \
-// RUN:   -analyzer-checker=alpha.unix.Stream \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN:   -analyzer-checker=unix.Stream \
+// RUN:   -analyzer-config unix.Stream:Pedantic=true \
 // RUN:   -analyzer-checker=unix.Errno \
 // RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \
diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c
index 08382eaf6abf9f..b28cc301a4ec25 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/clang/test/Analysis/stream-errno.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.Errno,unix.StdCLibraryFunctions,debug.ExprInspection \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Stream,unix.Errno,unix.StdCLibraryFunctions,debug.ExprInspection \
+// RUN:   -analyzer-config unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify %s
 
 #include "Inputs/system-header-simulator.h"
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 2abf4b900a047f..3f791d13346419 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -verify %s \
 // RUN: -analyzer-checker=core \
-// RUN: -analyzer-checker=alpha.unix.Stream \
-// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN: -analyzer-checker=unix.Stream \
+// RUN: -analyzer-config unix.Stream:Pedantic=true \
 // RUN: -analyzer-checker=debug.StreamTester \
 // RUN: -analyzer-checker=debug.ExprInspection
 
diff --git a/clang/test/Analysis/stream-invalidate.c b/clang/test/Analysis/stream-invalidate.c
index 5046a356d0583d..749c53d164fb5c 100644
--- a/clang/test/Analysis/stream-invalidate.c
+++ b/clang/test/Analysis/stream-invalidate.c
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -verify %s \
 // RUN: -analyzer-checker=core \
-// RUN: -analyzer-checker=alpha.unix.Stream \
+// RUN: -analyzer-checker=unix.Stream \
 // RUN: -analyzer-checker=debug.ExprInspection
 
 #include "Inputs/system-header-simulator.h"
diff --git a/clang/test/Analysis/stream-non-posix-function.c b/clang/test/Analysis/stream-non-posix-function.c
index 091d95a573ddf9..b9ece31cde1568 100644
--- a/clang/test/Analysis/stream-non-posix-function.c
+++ b/clang/test/Analysis/stream-non-posix-function.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -fno-builtin -analyzer-checker=core,alpha.unix.Stream -verify %s
+// RUN: %clang_analyze_cc1 -fno-builtin -analyzer-checker=core,unix.Stream -verify %s
 // RUN: %clang_analyze_cc1 -fno-builtin -analyzer-checker=core,alpha.unix.SimpleStream -verify %s
 
 // expected-no-diagnostics
diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c
index 644c699d05e246..87761b3afb76b4 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix.Errno \
-// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-checker=unix.Stream \
 // RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=debug.ExprInspection
diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index 03a8ff4e468f66..3aef707d50056e 100644
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -1,8 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -analyzer-output text \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Stream -analyzer-output text \
+// RUN:   -analyzer-config unix.Stream:Pedantic=true \
 // RUN:   -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.StdCLibraryFunctions -analyzer-output text \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Stream,unix.StdCLibraryFunctions -analyzer-output text \
+// RUN:   -analyzer-config unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=expected,stdargs %s
 
 #include "Inputs/system-header-simulator.h"
diff --git a/clang/test/Analysis/stream-pedantic.c b/clang/test/Analysis/stream-pedantic.c
index 2bbea81d47ef60..2a3dff86789030 100644
--- a/clang/test/Analysis/stream-pedantic.c
+++ b/clang/test/Analysis/stream-pedantic.c
@@ -1,8 +1,8 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
-// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=false -verify=nopedantic %s
+// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config unix.Stream:Pedantic=false -verify=nopedantic %s
 
-// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
-// RUN:   -analyze...
[truncated]

balazske · 2024-04-18T15:22:16Z

The checker is usable enough to move to non-alpha state.
This table contains some links to the results after the "Pedantic" option was added. The "new reports" are the ones that got removed if the option is turned on. At some projects there are still many results, for example at vim. The reason is that often read operations are not checked for error (or EOF) too, and the pedantic mode removes such warnings for the write operations only. But if read operations would be included, the checker almost loses its purpose to find unchecked operations. I think that probably this is a checker that must not be turned on by default, but still is not an alpha checker.

Project	New Reports	Resolved Reports
codechecker	14 new reports	No reports
vim	119 new reports	No reports
sqlite	74 new reports	No reports
libwebm	No reports	No reports
xerces	2 new reports	No reports
bitcoin	4 new reports	No reports

balazske · 2024-04-18T15:24:49Z

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

+                  InAlpha>
+  ]>,
+  Documentation<HasDocumentation>;
+


This would come after StdCLibraryFunctionsChecker if the original alphabetic order is used, but because StdreamChecker is a weak dependency of StdCLibraryFunctionsChecker it must be included before it to have the checker name available.

Perhaps add this remark as a comment in the Checkers.td file to help those who would notice the inconsistent ordering and try to restore the alphabetical order.

balazske · 2024-04-18T15:25:54Z

clang/www/analyzer/alpha_checks.html

-<tr><td><a id="alpha.unix.Stream"><div class="namedescr expandable"><span class="name">
-alpha.unix.Stream</span><span class="lang">
+<tr><td><a id="unix.Stream"><div class="namedescr expandable"><span class="name">
+unix.Stream</span><span class="lang">


Probably the checker must be removed entirely from this file?

Remove it from this file entirely.

In fact, I strongly suspect that the entire HTML documentation is obsolete and it should be deleted from the project (to avoid redundancy). I'm planning to review its contents in the foreseeable future and open a discussion if it's indeed superfluous, but until then let's do the bare minimum to update it.

balazske · 2024-04-18T15:28:48Z

clang/www/analyzer/open_projects.html

@@ -48,7 +48,7 @@ <h1>Open Projects</h1>
      <p><i>(Difficulty: Medium)</i></p></p>
      </li>

-      <li><code>alpha.unix.StreamChecker</code>
+      <li><code>unix.StreamChecker</code>


This section should be removed too. The problem is still not solved in the mentioned way ("delayed split"). I do not see how this could work to find problems where the result of an operation is not checked at all. Or if a split is done only when the result is used in any way, there may be still many checker warnings for read operations.

Yes, remove it from here.

steakhal

I have no particular interest of this change.
Let me know if I should do a proper review.

clang/docs/analyzer/checkers.rst

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

NagyDonat

LGTM, the diff is clean and the results on open source projects are good enough for a non-alpha checker.

Note that the direct codechecker links show the pedantic-only results which are... well, pedantic. Turning off the comparison and filtering for "alpha.unix.Stream" shows the default behavior of the checker, which is reasonable.

On vim even non-pedantic mode produces dozens of reports, but that's because the code assumes that the input files are well-formed (no EOF in the middle of a record) -- and this checker should definitely report that kind of assumption. (If the developers don't want to see this kind of warning, they can disable this checker.)

I think that this PR can be merged as it is now, but let's wait a week and see whether @haoNoQ or @Xazax-hun have any suggestions or objections.

NagyDonat · 2024-04-22T10:46:19Z

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

+                  InAlpha>
+  ]>,
+  Documentation<HasDocumentation>;
+


Perhaps add this remark as a comment in the Checkers.td file to help those who would notice the inconsistent ordering and try to restore the alphabetical order.

[clang][analyzer] Move StreamChecker out of the alpha package.

7138f02

llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Apr 18, 2024

balazske commented Apr 18, 2024

View reviewed changes

balazske requested review from NagyDonat and steakhal April 18, 2024 15:29

steakhal reviewed Apr 18, 2024

View reviewed changes

clang/docs/analyzer/checkers.rst Outdated Show resolved Hide resolved

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td Outdated Show resolved Hide resolved

balazske added 3 commits April 19, 2024 09:38

fixed documentation issues

719567b

fixed Checkers.td

78e3c39

deleted obsolete documentations

33903c2

NagyDonat approved these changes Apr 22, 2024

View reviewed changes

NagyDonat requested review from haoNoQ and Xazax-hun April 22, 2024 12:11

add comment to Checkers.td

dc465e7

balazske merged commit 09f160c into llvm:main Apr 30, 2024
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang][analyzer] Move StreamChecker out of the alpha package. #89247

[clang][analyzer] Move StreamChecker out of the alpha package. #89247

balazske commented Apr 18, 2024

llvmbot commented Apr 18, 2024 •

edited

balazske commented Apr 18, 2024

balazske Apr 18, 2024

NagyDonat Apr 22, 2024

balazske Apr 18, 2024

NagyDonat Apr 19, 2024

balazske Apr 18, 2024

NagyDonat Apr 19, 2024

steakhal left a comment

NagyDonat left a comment

NagyDonat Apr 22, 2024

[clang][analyzer] Move StreamChecker out of the alpha package. #89247

[clang][analyzer] Move StreamChecker out of the alpha package. #89247

Conversation

balazske commented Apr 18, 2024

llvmbot commented Apr 18, 2024 • edited

balazske commented Apr 18, 2024

balazske Apr 18, 2024

Choose a reason for hiding this comment

NagyDonat Apr 22, 2024

Choose a reason for hiding this comment

balazske Apr 18, 2024

Choose a reason for hiding this comment

NagyDonat Apr 19, 2024

Choose a reason for hiding this comment

balazske Apr 18, 2024

Choose a reason for hiding this comment

NagyDonat Apr 19, 2024

Choose a reason for hiding this comment

steakhal left a comment

Choose a reason for hiding this comment

NagyDonat left a comment

Choose a reason for hiding this comment

NagyDonat Apr 22, 2024

Choose a reason for hiding this comment

llvmbot commented Apr 18, 2024 •

edited