[clang][analyzer] Move unix.BlockInCriticalSection out of alpha by gamesh411 · Pull Request #93815 · llvm/llvm-project (original) (raw)

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

@llvm/pr-subscribers-clang

Author: Endre Fülöp (gamesh411)

Changes

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 #93799 (comment) 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:

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

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; +def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">, + HelpText<"Check for calls to blocking functions inside a critical section">, + Documentation; + 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; -def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">, - HelpText<"Check for calls to blocking functions inside a critical section">, - Documentation;

} // 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 @@

Unix Alpha Checkers

-
-alpha.unix.BlockInCriticalSection -(C)
-Check for calls to blocking functions inside a critical section. Applies to: -
-lock
-unlock
-sleep
-getc
-fgets
-read
-revc
-pthread_mutex_lock
-pthread_mutex_unlock
-mtx_lock
-mtx_timedlock
-mtx_trylock
-mtx_unlock
-lock_guard
-unique_lock
-
 -
-
-void test() {
-  std::mutex m;
-  m.lock();
-  sleep(3); // warn: a blocking function sleep is called inside a critical
-            //       section
-  m.unlock();
-}
-