[clang] [llvm] [clang][analyzer] Move unix.BlockInCriticalSection out of alpha (PR #93815)
Endre Fülöp via llvm-commits
llvm-commits at lists.llvm.org
Fri May 31 21:32:20 PDT 2024
https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/93815
>From 429cbcfb14295aaf07997082fed0820b6f79f4f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Thu, 30 May 2024 11:28:12 +0200
Subject: [PATCH 1/2] [workflow] Add AI review for PRs
---
.github/workflows/ai-code-review.yml | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 .github/workflows/ai-code-review.yml
diff --git a/.github/workflows/ai-code-review.yml b/.github/workflows/ai-code-review.yml
new file mode 100644
index 0000000000000..c5003cddb84b2
--- /dev/null
+++ b/.github/workflows/ai-code-review.yml
@@ -0,0 +1,22 @@
+name: AI Code Reviewer
+
+on:
+ pull_request:
+ types:
+ - opened
+ - synchronize
+permissions: write-all
+jobs:
+ review:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout Repo
+ uses: actions/checkout at v3
+
+ - name: AI Code Reviewer
+ uses: freeedcom/ai-codereviewer at v2.7.0
+ with:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # The GITHUB_TOKEN is there by default so you just need to keep it like it is and not necessarily need to add it as secret as it will throw an error. [More Details](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret)
+ OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
+ OPENAI_API_MODEL: "gpt-4" # Optional: defaults to "gpt-4"
+ exclude: "**/*.json, **/*.md" # Optional: exclude patterns separated by commas
>From 898940a64d0ff9f7d87b732529d8f4bc79b0701c 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 2/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">
More information about the llvm-commits
mailing list