From 6ef785c9517e8e44ddda8263e5f319b44f56cff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= Date: Mon, 3 Jun 2024 14:23:58 +0200 Subject: [PATCH] [clang][analyzer] Move unix.BlockInCriticalSection out of alpha (#93815) After recent improvements (#80029) and testing on open-source projects, the checker is ready to move out of the alpha package. --- clang/docs/analyzer/checkers.rst | 87 ++++++++++--------- .../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, 53 insertions(+), 83 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index bbc31832b9c3ca..1e75c3997a4750 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -1235,6 +1235,50 @@ 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, 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 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 + + 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 +3174,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 40f443047bd4bd..668e9f6cf07167 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 9543ba8ec02fc1..e605c62a66ad0e 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 1e174af541b183..36ecf9ac55f7df 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 403b7a16726a20..ee9a708f231a86 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 73d58479f4bf4a..2b5ec31568ba1f 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 14aca5a948bf4b..345a4e8f44efd1 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 2c8eece41fb2fc..411baae695b938 100644 --- a/clang/www/analyzer/alpha_checks.html +++ b/clang/www/analyzer/alpha_checks.html @@ -780,39 +780,6 @@

Unix Alpha Checkers

- - - -