diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 93ec15a7f09596..157afd9e862915 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -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``. diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 43137f4b020f9f..40aa06724ccb75 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -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 +`_. +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 `_. + +.. 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) @@ -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** @@ -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 -`_. -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 `_. - -.. 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) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index b6e9f0fae1c7f4..9015872c7d94f1 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -517,6 +517,18 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">, Documentation, Hidden; +def ErrnoChecker : Checker<"Errno">, + HelpText<"Check for improper use of 'errno'">, + Dependencies<[ErrnoModeling]>, + CheckerOptions<[ + CmdLineOption, + ]>, + Documentation; + def MallocChecker: Checker<"Malloc">, HelpText<"Check for memory leaks, double free, and use-after-free problems. " "Traces memory managed by malloc()/free().">, @@ -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, - ]>, - Documentation; - def ChrootChecker : Checker<"Chroot">, HelpText<"Check improper use of chroot">, Documentation; diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 8abfbf84983d81..13b529c5d7a294 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -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 @@ -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 diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c index cf69a6b04c9792..2c4e34f4990bf5 100644 --- a/clang/test/Analysis/analyzer-enabled-checkers.c +++ b/clang/test/Analysis/analyzer-enabled-checkers.c @@ -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 diff --git a/clang/test/Analysis/errno-notes.c b/clang/test/Analysis/errno-notes.c index 711acc0f5db81a..5db96a4d016342 100644 --- a/clang/test/Analysis/errno-notes.c +++ b/clang/test/Analysis/errno-notes.c @@ -3,7 +3,7 @@ // 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 \ @@ -11,7 +11,7 @@ // 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" diff --git a/clang/test/Analysis/errno-options.c b/clang/test/Analysis/errno-options.c index 0449f22b8f7b4e..744a156d0f5bbc 100644 --- a/clang/test/Analysis/errno-options.c +++ b/clang/test/Analysis/errno-options.c @@ -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" diff --git a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c index 41f54664cfb4f3..8964454f40fc96 100644 --- a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c +++ b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c @@ -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" diff --git a/clang/test/Analysis/errno-stdlibraryfunctions.c b/clang/test/Analysis/errno-stdlibraryfunctions.c index fce5e5d6b0a471..dafda764af9f38 100644 --- a/clang/test/Analysis/errno-stdlibraryfunctions.c +++ b/clang/test/Analysis/errno-stdlibraryfunctions.c @@ -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" diff --git a/clang/test/Analysis/errno.c b/clang/test/Analysis/errno.c index 1f094cc741eb30..f3753a6fe74378 100644 --- a/clang/test/Analysis/errno.c +++ b/clang/test/Analysis/errno.c @@ -3,7 +3,7 @@ // 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 \ @@ -11,7 +11,7 @@ // 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" @@ -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; } @@ -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) { } } @@ -141,7 +141,7 @@ 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]}} } } } @@ -149,7 +149,7 @@ void testErrnoCheckUndefinedLoad() { 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) } } 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 7f5bfba6ff5683..d4721c0a59a3d0 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 @@ -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 diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c index d260eb2252f396..2531e26e200385 100644 --- a/clang/test/Analysis/stream-errno-note.c +++ b/clang/test/Analysis/stream-errno-note.c @@ -1,6 +1,6 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.unix.Stream \ -// RUN: -analyzer-checker=alpha.unix.Errno \ +// RUN: -analyzer-checker=unix.Errno \ // RUN: -analyzer-checker=unix.StdCLibraryFunctions \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \ // RUN: -analyzer-output text -verify %s @@ -15,7 +15,7 @@ void check_fopen(void) { // expected-note@+1{{Taking false branch}} if (!F) return; - 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]}} // expected-note@-1{{An undefined value may be read from 'errno'}} fclose(F); } @@ -27,7 +27,7 @@ void check_tmpfile(void) { // expected-note@+1{{Taking false branch}} if (!F) return; - 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]}} // expected-note@-1{{An undefined value may be read from 'errno'}} fclose(F); } @@ -136,7 +136,7 @@ void check_rewind_errnocheck(void) { return; errno = 0; rewind(F); // expected-note{{After calling 'rewind' reading 'errno' is required to find out if the call has failed}} - fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}} + fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [unix.Errno]}} // expected-note@-1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}} } diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c index cf4e2e3d781d9b..bf0a61db2424f9 100644 --- a/clang/test/Analysis/stream-errno.c +++ b/clang/test/Analysis/stream-errno.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,unix.StdCLibraryFunctions,debug.ExprInspection \ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.Errno,unix.StdCLibraryFunctions,debug.ExprInspection \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify %s #include "Inputs/system-header-simulator.h" @@ -15,7 +15,7 @@ void check_fopen(void) { if (errno) {} // no-warning return; } - 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 check_tmpfile(void) { @@ -25,7 +25,7 @@ void check_tmpfile(void) { if (errno) {} // no-warning return; } - 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 check_freopen(void) { diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c index cbeac276fdee23..2daf640c18a1d4 100644 --- a/clang/test/Analysis/stream-noopen.c +++ b/clang/test/Analysis/stream-noopen.c @@ -1,6 +1,6 @@ // RUN: %clang_analyze_cc1 -verify %s \ // RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=alpha.unix.Errno \ +// RUN: -analyzer-checker=unix.Errno \ // RUN: -analyzer-checker=alpha.unix.Stream \ // RUN: -analyzer-checker=unix.StdCLibraryFunctions \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \ @@ -9,7 +9,7 @@ // enable only StdCLibraryFunctions checker // RUN: %clang_analyze_cc1 -verify %s \ // RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=alpha.unix.Errno \ +// RUN: -analyzer-checker=unix.Errno \ // RUN: -analyzer-checker=unix.StdCLibraryFunctions \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \ // RUN: -analyzer-checker=debug.ExprInspection