Skip to content

Commit 72d3bf2

Browse files
balazskebalazske
authored
[clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (#69469)
1 parent 521277c commit 72d3bf2

14 files changed

+115
-109
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,10 @@ Static Analyzer
938938
- Added a new checker ``core.BitwiseShift`` which reports situations where
939939
bitwise shift operators produce undefined behavior (because some operand is
940940
negative or too large).
941+
942+
- Move checker ``alpha.unix.Errno`` out of the ``alpha`` package
943+
to ``unix.Errno``.
944+
941945
- Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
942946
to ``unix.StdCLibraryFunctions``.
943947

‎clang/docs/analyzer/checkers.rst

Lines changed: 71 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,76 @@ Check calls to various UNIX/Posix functions: ``open, pthread_once, calloc, mallo
934934
.. literalinclude:: checkers/unix_api_example.c
935935
:language: c
936936
937+
.. _unix-Errno:
938+
939+
unix.Errno (C)
940+
""""""""""""""
941+
942+
Check for improper use of ``errno``.
943+
This checker implements partially CERT rule
944+
`ERR30-C. Set errno to zero before calling a library function known to set errno,
945+
and check errno only after the function returns a value indicating failure
946+
<https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152351>`_.
947+
The checker can find the first read of ``errno`` after successful standard
948+
function calls.
949+
950+
The C and POSIX standards often do not define if a standard library function
951+
may change value of ``errno`` if the call does not fail.
952+
Therefore, ``errno`` should only be used if it is known from the return value
953+
of a function that the call has failed.
954+
There are exceptions to this rule (for example ``strtol``) but the affected
955+
functions are not yet supported by the checker.
956+
The return values for the failure cases are documented in the standard Linux man
957+
pages of the functions and in the `POSIX standard <https://pubs.opengroup.org/onlinepubs/9699919799/>`_.
958+
959+
.. code-block:: c
960+
961+
int unsafe_errno_read(int sock, void *data, int data_size) {
962+
if (send(sock, data, data_size, 0) != data_size) {
963+
// 'send' can be successful even if not all data was sent
964+
if (errno == 1) { // An undefined value may be read from 'errno'
965+
return 0;
966+
}
967+
}
968+
return 1;
969+
}
970+
971+
The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
972+
warnings from this checker. The supported functions are the same as by
973+
:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
974+
checker affects the set of checked functions.
975+
976+
**Parameters**
977+
978+
The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
979+
errno value if the value is not used in a condition (in ``if`` statements,
980+
loops, conditional expressions, ``switch`` statements). For example ``errno``
981+
can be stored into a variable without getting a warning by the checker.
982+
983+
.. code-block:: c
984+
985+
int unsafe_errno_read(int sock, void *data, int data_size) {
986+
if (send(sock, data, data_size, 0) != data_size) {
987+
int err = errno;
988+
// warning if 'AllowErrnoReadOutsideConditionExpressions' is false
989+
// no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
990+
}
991+
return 1;
992+
}
993+
994+
Default value of this option is ``true``. This allows save of the errno value
995+
for possible later error handling.
996+
997+
**Limitations**
998+
999+
- Only the very first usage of ``errno`` is checked after an affected function
1000+
call. Value of ``errno`` is not followed when it is stored into a variable
1001+
or returned from a function.
1002+
- Documentation of function ``lseek`` is not clear about what happens if the
1003+
function returns different value than the expected file position but not -1.
1004+
To avoid possible false-positives ``errno`` is allowed to be used in this
1005+
case.
1006+
9371007
.. _unix-Malloc:
9381008
9391009
unix.Malloc (C)
@@ -1098,7 +1168,7 @@ state of the value ``errno`` if applicable to the analysis. Many system
10981168
functions set the ``errno`` value only if an error occurs (together with a
10991169
specific return value of the function), otherwise it becomes undefined. This
11001170
checker changes the analysis state to contain such information. This data is
1101-
used by other checkers, for example :ref:`alpha-unix-Errno`.
1171+
used by other checkers, for example :ref:`unix-Errno`.
11021172
11031173
**Limitations**
11041174
@@ -2826,76 +2896,6 @@ Check improper use of chroot.
28262896
f(); // warn: no call of chdir("/") immediately after chroot
28272897
}
28282898
2829-
.. _alpha-unix-Errno:
2830-
2831-
alpha.unix.Errno (C)
2832-
""""""""""""""""""""
2833-
2834-
Check for improper use of ``errno``.
2835-
This checker implements partially CERT rule
2836-
`ERR30-C. Set errno to zero before calling a library function known to set errno,
2837-
and check errno only after the function returns a value indicating failure
2838-
<https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152351>`_.
2839-
The checker can find the first read of ``errno`` after successful standard
2840-
function calls.
2841-
2842-
The C and POSIX standards often do not define if a standard library function
2843-
may change value of ``errno`` if the call does not fail.
2844-
Therefore, ``errno`` should only be used if it is known from the return value
2845-
of a function that the call has failed.
2846-
There are exceptions to this rule (for example ``strtol``) but the affected
2847-
functions are not yet supported by the checker.
2848-
The return values for the failure cases are documented in the standard Linux man
2849-
pages of the functions and in the `POSIX standard <https://pubs.opengroup.org/onlinepubs/9699919799/>`_.
2850-
2851-
.. code-block:: c
2852-
2853-
int unsafe_errno_read(int sock, void *data, int data_size) {
2854-
if (send(sock, data, data_size, 0) != data_size) {
2855-
// 'send' can be successful even if not all data was sent
2856-
if (errno == 1) { // An undefined value may be read from 'errno'
2857-
return 0;
2858-
}
2859-
}
2860-
return 1;
2861-
}
2862-
2863-
The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
2864-
warnings from this checker. The supported functions are the same as by
2865-
:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
2866-
checker affects the set of checked functions.
2867-
2868-
**Parameters**
2869-
2870-
The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
2871-
errno value if the value is not used in a condition (in ``if`` statements,
2872-
loops, conditional expressions, ``switch`` statements). For example ``errno``
2873-
can be stored into a variable without getting a warning by the checker.
2874-
2875-
.. code-block:: c
2876-
2877-
int unsafe_errno_read(int sock, void *data, int data_size) {
2878-
if (send(sock, data, data_size, 0) != data_size) {
2879-
int err = errno;
2880-
// warning if 'AllowErrnoReadOutsideConditionExpressions' is false
2881-
// no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
2882-
}
2883-
return 1;
2884-
}
2885-
2886-
Default value of this option is ``true``. This allows save of the errno value
2887-
for possible later error handling.
2888-
2889-
**Limitations**
2890-
2891-
- Only the very first usage of ``errno`` is checked after an affected function
2892-
call. Value of ``errno`` is not followed when it is stored into a variable
2893-
or returned from a function.
2894-
- Documentation of function ``lseek`` is not clear about what happens if the
2895-
function returns different value than the expected file position but not -1.
2896-
To avoid possible false-positives ``errno`` is allowed to be used in this
2897-
case.
2898-
28992899
.. _alpha-unix-PthreadLock:
29002900
29012901
alpha.unix.PthreadLock (C)

‎clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,18 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">,
517517
Documentation<NotDocumented>,
518518
Hidden;
519519

520+
def ErrnoChecker : Checker<"Errno">,
521+
HelpText<"Check for improper use of 'errno'">,
522+
Dependencies<[ErrnoModeling]>,
523+
CheckerOptions<[
524+
CmdLineOption<Boolean,
525+
"AllowErrnoReadOutsideConditionExpressions",
526+
"Allow read of undefined value from errno outside of conditions",
527+
"true",
528+
InAlpha>,
529+
]>,
530+
Documentation<HasDocumentation>;
531+
520532
def MallocChecker: Checker<"Malloc">,
521533
HelpText<"Check for memory leaks, double free, and use-after-free problems. "
522534
"Traces memory managed by malloc()/free().">,
@@ -561,18 +573,6 @@ def VforkChecker : Checker<"Vfork">,
561573

562574
let ParentPackage = UnixAlpha in {
563575

564-
def ErrnoChecker : Checker<"Errno">,
565-
HelpText<"Check for improper use of 'errno'">,
566-
Dependencies<[ErrnoModeling]>,
567-
CheckerOptions<[
568-
CmdLineOption<Boolean,
569-
"AllowErrnoReadOutsideConditionExpressions",
570-
"Allow read of undefined value from errno outside of conditions",
571-
"true",
572-
InAlpha>,
573-
]>,
574-
Documentation<HasDocumentation>;
575-
576576
def ChrootChecker : Checker<"Chroot">,
577577
HelpText<"Check improper use of chroot">,
578578
Documentation<HasDocumentation>;

‎clang/test/Analysis/analyzer-config.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
1414
// CHECK-NEXT: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv = false
1515
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
16-
// CHECK-NEXT: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
1716
// CHECK-NEXT: apply-fixits = false
1817
// CHECK-NEXT: assume-controlled-environment = false
1918
// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
@@ -128,6 +127,7 @@
128127
// CHECK-NEXT: track-conditions-debug = false
129128
// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = true
130129
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
130+
// CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
131131
// CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
132132
// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false
133133
// CHECK-NEXT: unroll-loops = false

‎clang/test/Analysis/analyzer-enabled-checkers.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
// CHECK-NEXT: unix.API
4545
// CHECK-NEXT: unix.cstring.CStringModeling
4646
// CHECK-NEXT: unix.DynamicMemoryModeling
47+
// CHECK-NEXT: unix.Errno
4748
// CHECK-NEXT: unix.Malloc
4849
// CHECK-NEXT: unix.MallocSizeof
4950
// CHECK-NEXT: unix.MismatchedDeallocator

‎clang/test/Analysis/errno-notes.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
// RUN: -analyzer-checker=apiModeling.Errno \
44
// RUN: -analyzer-checker=debug.ExprInspection \
55
// RUN: -analyzer-checker=debug.ErrnoTest \
6-
// RUN: -analyzer-checker=alpha.unix.Errno \
6+
// RUN: -analyzer-checker=unix.Errno \
77
// RUN: -DERRNO_VAR
88

99
// RUN: %clang_analyze_cc1 -verify -analyzer-output text %s \
1010
// RUN: -analyzer-checker=core \
1111
// RUN: -analyzer-checker=apiModeling.Errno \
1212
// RUN: -analyzer-checker=debug.ExprInspection \
1313
// RUN: -analyzer-checker=debug.ErrnoTest \
14-
// RUN: -analyzer-checker=alpha.unix.Errno \
14+
// RUN: -analyzer-checker=unix.Errno \
1515
// RUN: -DERRNO_FUNC
1616

1717
#include "Inputs/errno_var.h"

‎clang/test/Analysis/errno-options.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@
22
// RUN: -analyzer-checker=core \
33
// RUN: -analyzer-checker=apiModeling.Errno \
44
// RUN: -analyzer-checker=debug.ErrnoTest \
5-
// RUN: -analyzer-checker=alpha.unix.Errno \
6-
// RUN: -analyzer-config alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
5+
// RUN: -analyzer-checker=unix.Errno \
6+
// RUN: -analyzer-config unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
77
// RUN: -DERRNO_VAR
88

99
// RUN: %clang_analyze_cc1 -verify %s \
1010
// RUN: -analyzer-checker=core \
1111
// RUN: -analyzer-checker=apiModeling.Errno \
1212
// RUN: -analyzer-checker=debug.ErrnoTest \
13-
// RUN: -analyzer-checker=alpha.unix.Errno \
14-
// RUN: -analyzer-config alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
13+
// RUN: -analyzer-checker=unix.Errno \
14+
// RUN: -analyzer-config unix.Errno:AllowErrnoReadOutsideConditionExpressions=false \
1515
// RUN: -DERRNO_FUNC
1616

1717
#include "Inputs/system-header-simulator.h"

‎clang/test/Analysis/errno-stdlibraryfunctions-notes.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// RUN: -analyzer-checker=debug.ExprInspection \
44
// RUN: -analyzer-checker=unix.StdCLibraryFunctions \
55
// RUN: -analyzer-checker=apiModeling.Errno \
6-
// RUN: -analyzer-checker=alpha.unix.Errno \
6+
// RUN: -analyzer-checker=unix.Errno \
77
// RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true
88

99
#include "Inputs/errno_var.h"

‎clang/test/Analysis/errno-stdlibraryfunctions.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// RUN: -analyzer-checker=debug.ExprInspection \
44
// RUN: -analyzer-checker=unix.StdCLibraryFunctions \
55
// RUN: -analyzer-checker=apiModeling.Errno \
6-
// RUN: -analyzer-checker=alpha.unix.Errno \
6+
// RUN: -analyzer-checker=unix.Errno \
77
// RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true
88

99
#include "Inputs/errno_var.h"

‎clang/test/Analysis/errno.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
// RUN: -analyzer-checker=apiModeling.Errno \
44
// RUN: -analyzer-checker=debug.ExprInspection \
55
// RUN: -analyzer-checker=debug.ErrnoTest \
6-
// RUN: -analyzer-checker=alpha.unix.Errno \
6+
// RUN: -analyzer-checker=unix.Errno \
77
// RUN: -DERRNO_VAR
88

99
// RUN: %clang_analyze_cc1 -verify %s \
1010
// RUN: -analyzer-checker=core \
1111
// RUN: -analyzer-checker=apiModeling.Errno \
1212
// RUN: -analyzer-checker=debug.ExprInspection \
1313
// RUN: -analyzer-checker=debug.ErrnoTest \
14-
// RUN: -analyzer-checker=alpha.unix.Errno \
14+
// RUN: -analyzer-checker=unix.Errno \
1515
// RUN: -DERRNO_FUNC
1616

1717
#include "Inputs/system-header-simulator.h"
@@ -72,14 +72,14 @@ void testErrnoCheck0() {
7272
// The function did not promise to not change 'errno' if no failure happens.
7373
int X = ErrnoTesterChecker_setErrnoCheckState();
7474
if (X == 0) {
75-
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
75+
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [unix.Errno]}}
7676
}
7777
if (errno) { // no warning for second time (analysis stops at the first warning)
7878
}
7979
}
8080
X = ErrnoTesterChecker_setErrnoCheckState();
8181
if (X == 0) {
82-
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
82+
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [unix.Errno]}}
8383
}
8484
errno = 0;
8585
}
@@ -113,12 +113,12 @@ void testErrnoCheck2() {
113113
// change of 'errno'.
114114
int X = ErrnoTesterChecker_setErrnoCheckState();
115115
if (X == 2) {
116-
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
116+
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [unix.Errno]}}
117117
errno = 0;
118118
}
119119
X = ErrnoTesterChecker_setErrnoCheckState();
120120
if (X == 2) {
121-
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
121+
errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [unix.Errno]}}
122122
if (errno) {
123123
}
124124
}
@@ -141,15 +141,15 @@ void testErrnoCheck3() {
141141
void testErrnoCheckUndefinedLoad() {
142142
int X = ErrnoTesterChecker_setErrnoCheckState();
143143
if (X == 0) {
144-
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
144+
if (errno) { // expected-warning{{An undefined value may be read from 'errno' [unix.Errno]}}
145145
}
146146
}
147147
}
148148

149149
void testErrnoNotCheckedAtSystemCall() {
150150
int X = ErrnoTesterChecker_setErrnoCheckState();
151151
if (X == 2) {
152-
printf("%i", 1); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'printf' [alpha.unix.Errno]}}
152+
printf("%i", 1); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'printf' [unix.Errno]}}
153153
printf("%i", 1); // no warning ('printf' does not change errno state)
154154
}
155155
}

‎clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
// CHECK-NEXT: unix.API
5454
// CHECK-NEXT: unix.cstring.CStringModeling
5555
// CHECK-NEXT: unix.DynamicMemoryModeling
56+
// CHECK-NEXT: unix.Errno
5657
// CHECK-NEXT: unix.Malloc
5758
// CHECK-NEXT: unix.MallocSizeof
5859
// CHECK-NEXT: unix.MismatchedDeallocator

0 commit comments

Comments
 (0)