Skip to content

Commit

Permalink
[clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (l…
Browse files Browse the repository at this point in the history
  • Loading branch information
balazske authored Nov 21, 2023
1 parent 521277c commit 72d3bf2
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 109 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,10 @@ Static Analyzer
- Added a new checker ``core.BitwiseShift`` which reports situations where
bitwise shift operators produce undefined behavior (because some operand is
negative or too large).

- Move checker ``alpha.unix.Errno`` out of the ``alpha`` package
to ``unix.Errno``.

- Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
to ``unix.StdCLibraryFunctions``.

Expand Down
142 changes: 71 additions & 71 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,76 @@ Check calls to various UNIX/Posix functions: ``open, pthread_once, calloc, mallo
.. literalinclude:: checkers/unix_api_example.c
:language: c
.. _unix-Errno:
unix.Errno (C)
""""""""""""""
Check for improper use of ``errno``.
This checker implements partially CERT rule
`ERR30-C. Set errno to zero before calling a library function known to set errno,
and check errno only after the function returns a value indicating failure
<https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152351>`_.
The checker can find the first read of ``errno`` after successful standard
function calls.
The C and POSIX standards often do not define if a standard library function
may change value of ``errno`` if the call does not fail.
Therefore, ``errno`` should only be used if it is known from the return value
of a function that the call has failed.
There are exceptions to this rule (for example ``strtol``) but the affected
functions are not yet supported by the checker.
The return values for the failure cases are documented in the standard Linux man
pages of the functions and in the `POSIX standard <https://pubs.opengroup.org/onlinepubs/9699919799/>`_.
.. code-block:: c
int unsafe_errno_read(int sock, void *data, int data_size) {
if (send(sock, data, data_size, 0) != data_size) {
// 'send' can be successful even if not all data was sent
if (errno == 1) { // An undefined value may be read from 'errno'
return 0;
}
}
return 1;
}
The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
warnings from this checker. The supported functions are the same as by
:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
checker affects the set of checked functions.
**Parameters**
The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
errno value if the value is not used in a condition (in ``if`` statements,
loops, conditional expressions, ``switch`` statements). For example ``errno``
can be stored into a variable without getting a warning by the checker.
.. code-block:: c
int unsafe_errno_read(int sock, void *data, int data_size) {
if (send(sock, data, data_size, 0) != data_size) {
int err = errno;
// warning if 'AllowErrnoReadOutsideConditionExpressions' is false
// no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
}
return 1;
}
Default value of this option is ``true``. This allows save of the errno value
for possible later error handling.
**Limitations**
- Only the very first usage of ``errno`` is checked after an affected function
call. Value of ``errno`` is not followed when it is stored into a variable
or returned from a function.
- Documentation of function ``lseek`` is not clear about what happens if the
function returns different value than the expected file position but not -1.
To avoid possible false-positives ``errno`` is allowed to be used in this
case.
.. _unix-Malloc:
unix.Malloc (C)
Expand Down Expand Up @@ -1098,7 +1168,7 @@ state of the value ``errno`` if applicable to the analysis. Many system
functions set the ``errno`` value only if an error occurs (together with a
specific return value of the function), otherwise it becomes undefined. This
checker changes the analysis state to contain such information. This data is
used by other checkers, for example :ref:`alpha-unix-Errno`.
used by other checkers, for example :ref:`unix-Errno`.
**Limitations**
Expand Down Expand Up @@ -2826,76 +2896,6 @@ Check improper use of chroot.
f(); // warn: no call of chdir("/") immediately after chroot
}
.. _alpha-unix-Errno:
alpha.unix.Errno (C)
""""""""""""""""""""
Check for improper use of ``errno``.
This checker implements partially CERT rule
`ERR30-C. Set errno to zero before calling a library function known to set errno,
and check errno only after the function returns a value indicating failure
<https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152351>`_.
The checker can find the first read of ``errno`` after successful standard
function calls.
The C and POSIX standards often do not define if a standard library function
may change value of ``errno`` if the call does not fail.
Therefore, ``errno`` should only be used if it is known from the return value
of a function that the call has failed.
There are exceptions to this rule (for example ``strtol``) but the affected
functions are not yet supported by the checker.
The return values for the failure cases are documented in the standard Linux man
pages of the functions and in the `POSIX standard <https://pubs.opengroup.org/onlinepubs/9699919799/>`_.
.. code-block:: c
int unsafe_errno_read(int sock, void *data, int data_size) {
if (send(sock, data, data_size, 0) != data_size) {
// 'send' can be successful even if not all data was sent
if (errno == 1) { // An undefined value may be read from 'errno'
return 0;
}
}
return 1;
}
The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
warnings from this checker. The supported functions are the same as by
:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
checker affects the set of checked functions.
**Parameters**
The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
errno value if the value is not used in a condition (in ``if`` statements,
loops, conditional expressions, ``switch`` statements). For example ``errno``
can be stored into a variable without getting a warning by the checker.
.. code-block:: c
int unsafe_errno_read(int sock, void *data, int data_size) {
if (send(sock, data, data_size, 0) != data_size) {
int err = errno;
// warning if 'AllowErrnoReadOutsideConditionExpressions' is false
// no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
}
return 1;
}
Default value of this option is ``true``. This allows save of the errno value
for possible later error handling.
**Limitations**
- Only the very first usage of ``errno`` is checked after an affected function
call. Value of ``errno`` is not followed when it is stored into a variable
or returned from a function.
- Documentation of function ``lseek`` is not clear about what happens if the
function returns different value than the expected file position but not -1.
To avoid possible false-positives ``errno`` is allowed to be used in this
case.
.. _alpha-unix-PthreadLock:
alpha.unix.PthreadLock (C)
Expand Down
24 changes: 12 additions & 12 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,18 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">,
Documentation<NotDocumented>,
Hidden;

def ErrnoChecker : Checker<"Errno">,
HelpText<"Check for improper use of 'errno'">,
Dependencies<[ErrnoModeling]>,
CheckerOptions<[
CmdLineOption<Boolean,
"AllowErrnoReadOutsideConditionExpressions",
"Allow read of undefined value from errno outside of conditions",
"true",
InAlpha>,
]>,
Documentation<HasDocumentation>;

def MallocChecker: Checker<"Malloc">,
HelpText<"Check for memory leaks, double free, and use-after-free problems. "
"Traces memory managed by malloc()/free().">,
Expand Down Expand Up @@ -561,18 +573,6 @@ def VforkChecker : Checker<"Vfork">,

let ParentPackage = UnixAlpha in {

def ErrnoChecker : Checker<"Errno">,
HelpText<"Check for improper use of 'errno'">,
Dependencies<[ErrnoModeling]>,
CheckerOptions<[
CmdLineOption<Boolean,
"AllowErrnoReadOutsideConditionExpressions",
"Allow read of undefined value from errno outside of conditions",
"true",
InAlpha>,
]>,
Documentation<HasDocumentation>;

def ChrootChecker : Checker<"Chroot">,
HelpText<"Check improper use of chroot">,
Documentation<HasDocumentation>;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/analyzer-config.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
// CHECK-NEXT: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv = false
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
// CHECK-NEXT: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: apply-fixits = false
// CHECK-NEXT: assume-controlled-environment = false
// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
Expand Down Expand Up @@ -128,6 +127,7 @@
// CHECK-NEXT: track-conditions-debug = false
// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = true
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
// CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false
// CHECK-NEXT: unroll-loops = false
Expand Down
1 change: 1 addition & 0 deletions clang/test/Analysis/analyzer-enabled-checkers.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
// CHECK-NEXT: unix.API
// CHECK-NEXT: unix.cstring.CStringModeling
// CHECK-NEXT: unix.DynamicMemoryModeling
// CHECK-NEXT: unix.Errno
// CHECK-NEXT: unix.Malloc
// CHECK-NEXT: unix.MallocSizeof
// CHECK-NEXT: unix.MismatchedDeallocator
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Analysis/errno-notes.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=debug.ErrnoTest \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -DERRNO_VAR

// RUN: %clang_analyze_cc1 -verify -analyzer-output text %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=debug.ErrnoTest \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -DERRNO_FUNC

#include "Inputs/errno_var.h"
Expand Down
8 changes: 4 additions & 4 deletions clang/test/Analysis/errno-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ErrnoTest \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-config alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -analyzer-config unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
// RUN: -DERRNO_VAR

// RUN: %clang_analyze_cc1 -verify %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ErrnoTest \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-config alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -analyzer-config unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
// RUN: -DERRNO_FUNC

#include "Inputs/system-header-simulator.h"
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/errno-stdlibraryfunctions-notes.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=unix.StdCLibraryFunctions \
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true

#include "Inputs/errno_var.h"
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/errno-stdlibraryfunctions.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=unix.StdCLibraryFunctions \
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true

#include "Inputs/errno_var.h"
Expand Down
16 changes: 8 additions & 8 deletions clang/test/Analysis/errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=debug.ErrnoTest \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -DERRNO_VAR

// RUN: %clang_analyze_cc1 -verify %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=debug.ErrnoTest \
// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -analyzer-checker=unix.Errno \
// RUN: -DERRNO_FUNC

#include "Inputs/system-header-simulator.h"
Expand Down Expand Up @@ -72,14 +72,14 @@ void testErrnoCheck0() {
// The function did not promise to not change 'errno' if no failure happens.
int X = ErrnoTesterChecker_setErrnoCheckState();
if (X == 0) {
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [unix.Errno]}}
}
if (errno) { // no warning for second time (analysis stops at the first warning)
}
}
X = ErrnoTesterChecker_setErrnoCheckState();
if (X == 0) {
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [unix.Errno]}}
}
errno = 0;
}
Expand Down Expand Up @@ -113,12 +113,12 @@ void testErrnoCheck2() {
// change of 'errno'.
int X = ErrnoTesterChecker_setErrnoCheckState();
if (X == 2) {
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [unix.Errno]}}
errno = 0;
}
X = ErrnoTesterChecker_setErrnoCheckState();
if (X == 2) {
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [unix.Errno]}}
if (errno) {
}
}
Expand All @@ -141,15 +141,15 @@ void testErrnoCheck3() {
void testErrnoCheckUndefinedLoad() {
int X = ErrnoTesterChecker_setErrnoCheckState();
if (X == 0) {
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [unix.Errno]}}
}
}
}

void testErrnoNotCheckedAtSystemCall() {
int X = ErrnoTesterChecker_setErrnoCheckState();
if (X == 2) {
printf("%i", 1); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'printf' [alpha.unix.Errno]}}
printf("%i", 1); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'printf' [unix.Errno]}}
printf("%i", 1); // no warning ('printf' does not change errno state)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
// CHECK-NEXT: unix.API
// CHECK-NEXT: unix.cstring.CStringModeling
// CHECK-NEXT: unix.DynamicMemoryModeling
// CHECK-NEXT: unix.Errno
// CHECK-NEXT: unix.Malloc
// CHECK-NEXT: unix.MallocSizeof
// CHECK-NEXT: unix.MismatchedDeallocator
Expand Down
Loading

0 comments on commit 72d3bf2

Please sign in to comment.