Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/93...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits