[clang] [clang][analyzer] Move unix.BlockInCriticalSection out of alpha (PR #93815)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 31 03:58:15 PDT 2024
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/93815 at github.com>
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
@llvm/pr-subscribers-clang
Author: Endre Fülöp (gamesh411)
<details>
<summary>Changes</summary>
After recent improvements, and testing on open source projects, the
checker is ready to move out of the alpha package.
I would like to land #<!-- -->93799 and #<!-- -->93799 first, then this modification.
I have ran this checker on multiple OS projects, and found no false positives, and only 10 true ones.
The complete set of tested projects:
codechecker, memcached, tmux, curl, twin, vim, openssl, sqlite, ffmpeg, postgres, tinyxml2, libwebm, xerces, bitcoin, protobuf, qtbase, openrct2, llvm-project.
The results for this checker:
https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_alpha.unix.BlockInCriticalSection-evaluation&is-unique=on&diff-type=New&checker-name=alpha.unix.BlockInCriticalSection
https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_alpha.unix.BlockInCriticalSection-evaluation&is-unique=on&diff-type=New&checker-name=alpha.unix.BlockInCriticalSection
https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openrct2_v0.4.8_alpha.unix.BlockInCriticalSection-evaluation&is-unique=on&diff-type=New&checker-name=alpha.unix.BlockInCriticalSection
Also note that even if I just merged a commit admitting to one of this checker's limitations (that can lead to false positives), these OS project runs did not show any, so it currently does not seem to be a very prominent limitation.
Please see this comment https://github.com/llvm/llvm-project/pull/93799#issuecomment-2141496323 for my plans to deal with this limitation and reduce code duplication.
---
Full diff: https://github.com/llvm/llvm-project/pull/93815.diff
8 Files Affected:
- (modified) clang/docs/analyzer/checkers.rst (+19-18)
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+4-4)
- (modified) clang/test/Analysis/analyzer-enabled-checkers.c (+1)
- (modified) clang/test/Analysis/block-in-critical-section.c (+1-1)
- (modified) clang/test/Analysis/block-in-critical-section.cpp (+1-1)
- (modified) clang/test/Analysis/block-in-critical-section.m (+1-1)
- (modified) clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c (+1)
- (modified) clang/www/analyzer/alpha_checks.html (-33)
``````````diff
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 3a31708a1e9de..86412bd3b9294 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1235,6 +1235,25 @@ Check calls to various UNIX/Posix functions: ``open, pthread_once, calloc, mallo
.. literalinclude:: checkers/unix_api_example.c
:language: c
+.. _unix-BlockInCriticalSection:
+
+unix.BlockInCriticalSection (C)
+"""""""""""""""""""""""""""""""""""""
+
+Check for calls to blocking functions inside a critical section.
+Applies to: ``lock, unlock, sleep, getc, fgets, read, recv, pthread_mutex_lock,``
+`` pthread_mutex_unlock, mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock, lock_guard, unique_lock``
+
+.. code-block:: c
+
+ void test() {
+ std::mutex m;
+ m.lock();
+ sleep(3); // warn: a blocking function sleep is called inside a critical
+ // section
+ m.unlock();
+ }
+
.. _unix-Errno:
unix.Errno (C)
@@ -3130,24 +3149,6 @@ For a more detailed description of configuration options, please see the
alpha.unix
^^^^^^^^^^
-.. _alpha-unix-BlockInCriticalSection:
-
-alpha.unix.BlockInCriticalSection (C)
-"""""""""""""""""""""""""""""""""""""
-Check for calls to blocking functions inside a critical section.
-Applies to: ``lock, unlock, sleep, getc, fgets, read, recv, pthread_mutex_lock,``
-`` pthread_mutex_unlock, mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock, lock_guard, unique_lock``
-
-.. code-block:: c
-
- void test() {
- std::mutex m;
- m.lock();
- sleep(3); // warn: a blocking function sleep is called inside a critical
- // section
- m.unlock();
- }
-
.. _alpha-unix-Chroot:
alpha.unix.Chroot (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 40f443047bd4b..668e9f6cf0716 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -509,6 +509,10 @@ def UnixAPIMisuseChecker : Checker<"API">,
HelpText<"Check calls to various UNIX/Posix functions">,
Documentation<HasDocumentation>;
+def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
+ HelpText<"Check for calls to blocking functions inside a critical section">,
+ Documentation<HasDocumentation>;
+
def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">,
HelpText<"The base of several malloc() related checkers. On it's own it "
"emits no reports, but adds valuable information to the analysis "
@@ -619,10 +623,6 @@ def SimpleStreamChecker : Checker<"SimpleStream">,
HelpText<"Check for misuses of stream APIs">,
Documentation<HasDocumentation>;
-def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
- HelpText<"Check for calls to blocking functions inside a critical section">,
- Documentation<HasDocumentation>;
-
} // end "alpha.unix"
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index 9543ba8ec02fc..e605c62a66ad0 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -42,6 +42,7 @@
// CHECK-NEXT: security.insecureAPI.mktemp
// CHECK-NEXT: security.insecureAPI.vfork
// CHECK-NEXT: unix.API
+// CHECK-NEXT: unix.BlockInCriticalSection
// CHECK-NEXT: unix.cstring.CStringModeling
// CHECK-NEXT: unix.DynamicMemoryModeling
// CHECK-NEXT: unix.Errno
diff --git a/clang/test/Analysis/block-in-critical-section.c b/clang/test/Analysis/block-in-critical-section.c
index 1e174af541b18..36ecf9ac55f7d 100644
--- a/clang/test/Analysis/block-in-critical-section.c
+++ b/clang/test/Analysis/block-in-critical-section.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.BlockInCriticalSection -verify %s
// expected-no-diagnostics
// This should not crash
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index 87c26b9f1b520..238d9a84f5f77 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=alpha.unix.BlockInCriticalSection \
+// RUN: -analyzer-checker=unix.BlockInCriticalSection \
// RUN: -std=c++11 \
// RUN: -analyzer-output text \
// RUN: -verify %s
diff --git a/clang/test/Analysis/block-in-critical-section.m b/clang/test/Analysis/block-in-critical-section.m
index 73d58479f4bf4..2b5ec31568ba1 100644
--- a/clang/test/Analysis/block-in-critical-section.m
+++ b/clang/test/Analysis/block-in-critical-section.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.BlockInCriticalSection -verify -Wno-objc-root-class %s
// expected-no-diagnostics
@interface SomeClass
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 14aca5a948bf4..345a4e8f44efd 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
@@ -50,6 +50,7 @@
// CHECK-NEXT: security.insecureAPI.mktemp
// CHECK-NEXT: security.insecureAPI.vfork
// CHECK-NEXT: unix.API
+// CHECK-NEXT: unix.BlockInCriticalSection
// CHECK-NEXT: unix.cstring.CStringModeling
// CHECK-NEXT: unix.DynamicMemoryModeling
// CHECK-NEXT: unix.Errno
diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html
index 2c8eece41fb2f..411baae695b93 100644
--- a/clang/www/analyzer/alpha_checks.html
+++ b/clang/www/analyzer/alpha_checks.html
@@ -780,39 +780,6 @@ <h3 id="unix_alpha_checkers">Unix Alpha Checkers</h3>
<tbody>
-<tr><td><a id="alpha.unix.BlockInCriticalSection"><div class="namedescr expandable"><span class="name">
-alpha.unix.BlockInCriticalSection</span><span class="lang">
-(C)</span><div class="descr">
-Check for calls to blocking functions inside a critical section. Applies to:
-<div class=functions>
-lock<br>
-unlock<br>
-sleep<br>
-getc<br>
-fgets<br>
-read<br>
-revc<br>
-pthread_mutex_lock<br>
-pthread_mutex_unlock<br>
-mtx_lock<br>
-mtx_timedlock<br>
-mtx_trylock<br>
-mtx_unlock<br>
-lock_guard<br>
-unique_lock</div>
-</div></div></a></td>
-<td><div class="exampleContainer expandable">
-<div class="example"><pre>
-void test() {
- std::mutex m;
- m.lock();
- sleep(3); // warn: a blocking function sleep is called inside a critical
- // section
- m.unlock();
-}
-</pre></div></div></td></tr>
-
-
<tr><td><a id="alpha.unix.Chroot"><div class="namedescr expandable"><span class="name">
alpha.unix.Chroot</span><span class="lang">
(C)</span><div class="descr">
``````````
</details>
https://github.com/llvm/llvm-project/pull/93815
More information about the cfe-commits
mailing list