[clang] [clang][analyzer] Move unix.BlockInCriticalSection out of alpha (PR #93815)

Endre Fülöp via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 3 03:37:04 PDT 2024


https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/93815

>From 1da3d4dc3c74f1240e7d921c97b0c7eea2a53e33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Wed, 29 May 2024 00:38:07 +0200
Subject: [PATCH 1/2] [clang][analyzer] Move unix.BlockInCriticalSection out of
 alpha

After recent improvements, and testing on open source projects, the
checker is ready to move out of the alpha package.
---
 clang/docs/analyzer/checkers.rst              | 86 +++++++++----------
 .../clang/StaticAnalyzer/Checkers/Checkers.td |  8 +-
 .../test/Analysis/analyzer-enabled-checkers.c |  1 +
 .../test/Analysis/block-in-critical-section.c |  2 +-
 .../Analysis/block-in-critical-section.cpp    |  2 +-
 .../test/Analysis/block-in-critical-section.m |  2 +-
 ...c-library-functions-arg-enabled-checkers.c |  1 +
 clang/www/analyzer/alpha_checks.html          | 33 -------
 8 files changed, 52 insertions(+), 83 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index bbc31832b9c3c..99a31bbdce283 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1235,6 +1235,49 @@ 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.
+Blocking functions detected by this checker: ``sleep, getc, fgets, read, recv``.
+Critical section handling functions modelled by this checker: ``lock, unlock, pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock, mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock, lock_guard, unique_lock``.
+
+.. code-block:: c
+
+ void pthread_lock_example(pthread_mutex_t *m) {
+   pthread_mutex_lock(m); // note: entering critical section here
+   sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
+   pthread_mutex_unlock(m);
+ }
+
+.. code-block:: cpp
+
+ void overlapping_critical_sections(mtx_t *m1, std::mutex &m2) {
+   std::lock_guard lg{m2}; // note: entering critical section here
+   mtx_lock(m1); // note: entering critical section here
+   sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
+   mtx_unlock(m1);
+   sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
+             // still inside of the critical section of the std::lock_guard
+ }
+
+**Limitations**
+
+* The ``trylock`` and ``timedlock`` versions of acquiring locks are currently assumed to always succeed.
+  This can lead to false positives.
+
+.. code-block:: c
+
+ void trylock_example(pthread_mutex_t *m) {
+   if (pthread_mutex_trylock(m) == 0) { // assume trylock always succeeds
+     sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
+     pthread_mutex_unlock(m);
+   } else {
+     sleep(10); // false positive: Incorrect warning about blocking function inside critical section.
+   }
+ }
+
 .. _unix-Errno:
 
 unix.Errno (C)
@@ -3130,49 +3173,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.
-Blocking functions detected by this checker: ``sleep, getc, fgets, read, recv``.
-Critical section handling functions modelled by this checker: ``lock, unlock, pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock, mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock, lock_guard, unique_lock``.
-
-.. code-block:: c
-
- void pthread_lock_example(pthread_mutex_t *m) {
-   pthread_mutex_lock(m); // note: entering critical section here
-   sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
-   pthread_mutex_unlock(m);
- }
-
-.. code-block:: cpp
-
- void overlapping_critical_sections(mtx_t *m1, std::mutex &m2) {
-   std::lock_guard lg{m2}; // note: entering critical section here
-   mtx_lock(m1); // note: entering critical section here
-   sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
-   mtx_unlock(m1);
-   sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
-             // still inside of the critical section of the std::lock_guard
- }
-
-**Limitations**
-
-* The ``trylock`` and ``timedlock`` versions of acquiring locks are currently assumed to always succeed.
-  This can lead to false positives.
-
-.. code-block:: c
-
- void trylock_example(pthread_mutex_t *m) {
-   if (pthread_mutex_trylock(m) == 0) { // assume trylock always succeeds
-     sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
-     pthread_mutex_unlock(m);
-   } else {
-     sleep(10); // false positive: Incorrect warning about blocking function inside critical section.
-   }
- }
-
 .. _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 403b7a16726a2..ee9a708f231a8 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">

>From 2e27ca30ccdeda647f275a501377f2029570f9e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 3 Jun 2024 12:36:31 +0200
Subject: [PATCH 2/2] steakhal review fix

---
 clang/docs/analyzer/checkers.rst | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 99a31bbdce283..1e75c3997a475 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1237,11 +1237,12 @@ Check calls to various UNIX/Posix functions: ``open, pthread_once, calloc, mallo
 
 .. _unix-BlockInCriticalSection:
 
-unix.BlockInCriticalSection (C)
-"""""""""""""""""""""""""""""""""""""
+unix.BlockInCriticalSection (C, C++)
+""""""""""""""""""""""""""""""""""""
 Check for calls to blocking functions inside a critical section.
 Blocking functions detected by this checker: ``sleep, getc, fgets, read, recv``.
-Critical section handling functions modelled by this checker: ``lock, unlock, pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock, mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock, lock_guard, unique_lock``.
+Critical section handling functions modeled by this checker:
+``lock, unlock, pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock, mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock, lock_guard, unique_lock``.
 
 .. code-block:: c
 
@@ -1259,7 +1260,7 @@ Critical section handling functions modelled by this checker: ``lock, unlock, pt
    sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
    mtx_unlock(m1);
    sleep(10); // warn: Call to blocking function 'sleep' inside of critical section
-             // still inside of the critical section of the std::lock_guard
+              // still inside of the critical section of the std::lock_guard
  }
 
 **Limitations**



More information about the cfe-commits mailing list