This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
balazske marked an inline comment as done.
Closed by commit rG60f3b071185b: [clang][analyzer] Add checker for bad use of
'errno'. (authored by balazske).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122150/new/
https://reviews.llvm.org/D122150
Files:
clang/docs/ReleaseNotes.rst
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/errno-notes.c
clang/test/Analysis/errno-options.c
clang/test/Analysis/errno.c
Index: clang/test/Analysis/errno.c
===================================================================
--- clang/test/Analysis/errno.c
+++ clang/test/Analysis/errno.c
@@ -3,6 +3,7 @@
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=debug.ErrnoTest \
+// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -DERRNO_VAR
// RUN: %clang_analyze_cc1 -verify %s \
@@ -10,8 +11,10 @@
// RUN: -analyzer-checker=apiModeling.Errno \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=debug.ErrnoTest \
+// RUN: -analyzer-checker=alpha.unix.Errno \
// RUN: -DERRNO_FUNC
+#include "Inputs/system-header-simulator.h"
#ifdef ERRNO_VAR
#include "Inputs/errno_var.h"
#endif
@@ -24,6 +27,7 @@
int ErrnoTesterChecker_getErrno();
int ErrnoTesterChecker_setErrnoIfError();
int ErrnoTesterChecker_setErrnoIfErrorRange();
+int ErrnoTesterChecker_setErrnoCheckState();
void something();
@@ -61,3 +65,199 @@
clang_analyzer_eval(errno == 1); // expected-warning{{FALSE}} expected-warning{{TRUE}}
}
}
+
+void testErrnoCheck0() {
+ // If the function returns a success result code, value of 'errno'
+ // is unspecified and it is unsafe to make any decision with it.
+ // 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) { // 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]}}
+ }
+ errno = 0;
+ }
+ X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 0) {
+ errno = 0;
+ if (errno) { // no warning after overwritten 'errno'
+ }
+ }
+}
+
+void testErrnoCheck1() {
+ // If the function returns error result code that is out-of-band (not a valid
+ // non-error return value) the value of 'errno' can be checked but it is not
+ // required to do so.
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 1) {
+ if (errno) { // no warning
+ }
+ }
+ X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 1) {
+ errno = 0; // no warning
+ }
+}
+
+void testErrnoCheck2() {
+ // If the function returns an in-band error result the value of 'errno' is
+ // required to be checked to verify if error happened.
+ // The same applies to other functions that can indicate failure only by
+ // 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;
+ }
+ X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 2) {
+ errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
+ if (errno) {
+ }
+ }
+}
+
+void testErrnoCheck3() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 2) {
+ if (errno) {
+ }
+ errno = 0; // no warning after 'errno' was read
+ }
+ X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 2) {
+ int A = errno;
+ errno = 0; // no warning after 'errno' was read
+ }
+}
+
+void testErrnoCheckUndefinedLoad() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 0) {
+ if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.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); // no warning ('printf' does not change errno state)
+ }
+}
+
+void testErrnoCheckStateInvalidate() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 0) {
+ something();
+ if (errno) { // no warning after an invalidating function call
+ }
+ }
+ X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 0) {
+ printf("%i", 1);
+ if (errno) { // no warning after an invalidating standard function call
+ }
+ }
+}
+
+void testErrnoCheckStateInvalidate1() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ if (X == 2) {
+ clang_analyzer_eval(errno); // expected-warning{{TRUE}}
+ something();
+ clang_analyzer_eval(errno); // expected-warning{{UNKNOWN}}
+ errno = 0; // no warning after invalidation
+ }
+}
+
+void test_if_cond_in_expr() {
+ ErrnoTesterChecker_setErrnoIfError();
+ if (errno + 10 > 2) {
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+ }
+}
+
+void test_for_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ for (; errno != 0;) {
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+ }
+}
+
+void test_do_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ do {
+ } while (errno != 0);
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_while_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ while (errno != 0) {
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+ }
+}
+
+void test_switch_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ switch (errno) {}
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_conditional_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int A = errno ? 1 : 2;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_binary_conditional_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int A = errno ?: 2;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_errno_store_into_variable() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int a = errno; // AllowNonConditionErrnoRead is on by default, no warning
+}
+
+void test_errno_store_into_variable_in_expr() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int a = errno > 1; // AllowNonConditionErrnoRead is on by default, no warning
+}
+
+int test_errno_return() {
+ ErrnoTesterChecker_setErrnoIfError();
+ return errno;
+}
+
+void test_errno_pointer1() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int *ErrnoP = &errno;
+ int A = errno ? 1 : 2;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_errno_pointer2() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int *ErrnoP = &errno;
+ int A = (*ErrnoP) ? 1 : 2;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+int f(int);
+
+void test_errno_in_condition_in_function_call() {
+ ErrnoTesterChecker_setErrnoIfError();
+ if (f(errno) != 0) {
+ }
+}
Index: clang/test/Analysis/errno-options.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/errno-options.c
@@ -0,0 +1,55 @@
+// 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: -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: -DERRNO_FUNC
+
+#include "Inputs/system-header-simulator.h"
+#ifdef ERRNO_VAR
+#include "Inputs/errno_var.h"
+#endif
+#ifdef ERRNO_FUNC
+#include "Inputs/errno_func.h"
+#endif
+
+int ErrnoTesterChecker_setErrnoIfError();
+
+void test_cond() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int A = errno ? 1 : 2;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_errno_store_into_variable() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int a = errno;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+void test_errno_store_into_variable_in_expr() {
+ ErrnoTesterChecker_setErrnoIfError();
+ int a = 4 + errno;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+int test_errno_return() {
+ ErrnoTesterChecker_setErrnoIfError();
+ return errno;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
+
+int test_errno_return_expr() {
+ ErrnoTesterChecker_setErrnoIfError();
+ return errno > 10;
+ // expected-warning@-1{{An undefined value may be read from 'errno'}}
+}
Index: clang/test/Analysis/errno-notes.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/errno-notes.c
@@ -0,0 +1,62 @@
+// 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: -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: -DERRNO_FUNC
+
+#include "Inputs/errno_var.h"
+#include "Inputs/system-header-simulator.h"
+#ifdef ERRNO_VAR
+#include "Inputs/errno_var.h"
+#endif
+#ifdef ERRNO_FUNC
+#include "Inputs/errno_func.h"
+#endif
+
+int ErrnoTesterChecker_setErrnoCheckState();
+
+void something();
+
+void testErrnoCheckUndefRead() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ something();
+ X = ErrnoTesterChecker_setErrnoCheckState(); // expected-note{{Assuming that this function succeeds but sets 'errno' to an unspecified value}}
+ if (X == 0) { // expected-note{{'X' is equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ if (errno) {
+ } // expected-warning@-1{{An undefined value may be read from 'errno'}}
+ // expected-note@-2{{An undefined value may be read from 'errno'}}
+ }
+}
+
+void testErrnoCheckOverwrite() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ something();
+ X = ErrnoTesterChecker_setErrnoCheckState(); // expected-note{{Assuming that this function returns 2. 'errno' should be checked to test for failure}}
+ if (X == 2) { // expected-note{{'X' is equal to 2}}
+ // expected-note@-1{{Taking true branch}}
+ errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here}}
+ // expected-note@-1{{Value of 'errno' was not checked and is overwritten here}}
+ }
+}
+
+void testErrnoCheckOverwriteStdCall() {
+ int X = ErrnoTesterChecker_setErrnoCheckState();
+ something();
+ X = ErrnoTesterChecker_setErrnoCheckState(); // expected-note{{Assuming that this function returns 2. 'errno' should be checked to test for failure}}
+ if (X == 2) { // expected-note{{'X' is equal to 2}}
+ // expected-note@-1{{Taking true branch}}
+ printf(""); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'printf'}}
+ // expected-note@-1{{Value of 'errno' was not checked and may be overwritten by function 'printf'}}
+ }
+}
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -12,6 +12,7 @@
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
+// CHECK-NEXT: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries = false
// CHECK-NEXT: apiModeling.StdCLibraryFunctions:ModelPOSIX = false
// CHECK-NEXT: apply-fixits = false
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
@@ -20,6 +20,7 @@
using namespace clang;
using namespace ento;
+using namespace errno_modeling;
namespace {
@@ -28,11 +29,43 @@
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
private:
+ /// Evaluate function \code void ErrnoTesterChecker_setErrno(int) \endcode.
+ /// Set value of \c errno to the argument.
static void evalSetErrno(CheckerContext &C, const CallEvent &Call);
+ /// Evaluate function \code int ErrnoTesterChecker_getErrno() \endcode.
+ /// Return the value of \c errno.
static void evalGetErrno(CheckerContext &C, const CallEvent &Call);
+ /// Evaluate function \code int ErrnoTesterChecker_setErrnoIfError() \endcode.
+ /// Simulate a standard library function tha returns 0 on success and 1 on
+ /// failure. On the success case \c errno is not allowed to be used (may be
+ /// undefined). On the failure case \c errno is set to a fixed value 11 and
+ /// is not needed to be checked.
static void evalSetErrnoIfError(CheckerContext &C, const CallEvent &Call);
+ /// Evaluate function \code int ErrnoTesterChecker_setErrnoIfErrorRange()
+ /// \endcode. Same as \c ErrnoTesterChecker_setErrnoIfError but \c errno is
+ /// set to a range (to be nonzero) at the failure case.
static void evalSetErrnoIfErrorRange(CheckerContext &C,
const CallEvent &Call);
+ /// Evaluate function \code int ErrnoTesterChecker_setErrnoCheckState()
+ /// \endcode. This function simulates the following:
+ /// - Return 0 and leave \c errno with undefined value.
+ /// This is the case of a successful standard function call.
+ /// For example if \c ftell returns not -1.
+ /// - Return 1 and sets \c errno to a specific error code (1).
+ /// This is the case of a failed standard function call.
+ /// The function indicates the failure by a special return value
+ /// that is returned only at failure.
+ /// \c errno can be checked but it is not required.
+ /// For example if \c ftell returns -1.
+ /// - Return 2 and may set errno to a value (actually it does not set it).
+ /// This is the case of a standard function call where the failure can only
+ /// be checked by reading from \c errno. The value of \c errno is changed by
+ /// the function only at failure, the user should set \c errno to 0 before
+ /// the call (\c ErrnoChecker does not check for this rule).
+ /// \c strtol is an example of this case, if it returns \c LONG_MIN (or
+ /// \c LONG_MAX). This case applies only if \c LONG_MIN or \c LONG_MAX is
+ /// returned, otherwise the first case in this list applies.
+ static void evalSetErrnoCheckState(CheckerContext &C, const CallEvent &Call);
using EvalFn = std::function<void(CheckerContext &, const CallEvent &)>;
const CallDescriptionMap<EvalFn> TestCalls{
@@ -41,22 +74,24 @@
{{"ErrnoTesterChecker_setErrnoIfError", 0},
&ErrnoTesterChecker::evalSetErrnoIfError},
{{"ErrnoTesterChecker_setErrnoIfErrorRange", 0},
- &ErrnoTesterChecker::evalSetErrnoIfErrorRange}};
+ &ErrnoTesterChecker::evalSetErrnoIfErrorRange},
+ {{"ErrnoTesterChecker_setErrnoCheckState", 0},
+ &ErrnoTesterChecker::evalSetErrnoCheckState}};
};
} // namespace
void ErrnoTesterChecker::evalSetErrno(CheckerContext &C,
const CallEvent &Call) {
- C.addTransition(errno_modeling::setErrnoValue(
- C.getState(), C.getLocationContext(), Call.getArgSVal(0)));
+ C.addTransition(setErrnoValue(C.getState(), C.getLocationContext(),
+ Call.getArgSVal(0), Irrelevant));
}
void ErrnoTesterChecker::evalGetErrno(CheckerContext &C,
const CallEvent &Call) {
ProgramStateRef State = C.getState();
- Optional<SVal> ErrnoVal = errno_modeling::getErrnoValue(State);
+ Optional<SVal> ErrnoVal = getErrnoValue(State);
assert(ErrnoVal && "Errno value should be available.");
State =
State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), *ErrnoVal);
@@ -71,10 +106,11 @@
ProgramStateRef StateSuccess = State->BindExpr(
Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(0, true));
+ StateSuccess = setErrnoState(StateSuccess, MustNotBeChecked);
ProgramStateRef StateFailure = State->BindExpr(
Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(1, true));
- StateFailure = errno_modeling::setErrnoValue(StateFailure, C, 11);
+ StateFailure = setErrnoValue(StateFailure, C, 11, Irrelevant);
C.addTransition(StateSuccess);
C.addTransition(StateFailure);
@@ -87,6 +123,7 @@
ProgramStateRef StateSuccess = State->BindExpr(
Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(0, true));
+ StateSuccess = setErrnoState(StateSuccess, MustNotBeChecked);
ProgramStateRef StateFailure = State->BindExpr(
Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(1, true));
@@ -94,13 +131,40 @@
nullptr, Call.getOriginExpr(), C.getLocationContext(), C.blockCount());
StateFailure = StateFailure->assume(ErrnoVal, true);
assert(StateFailure && "Failed to assume on an initial value.");
- StateFailure = errno_modeling::setErrnoValue(
- StateFailure, C.getLocationContext(), ErrnoVal);
+ StateFailure =
+ setErrnoValue(StateFailure, C.getLocationContext(), ErrnoVal, Irrelevant);
C.addTransition(StateSuccess);
C.addTransition(StateFailure);
}
+void ErrnoTesterChecker::evalSetErrnoCheckState(CheckerContext &C,
+ const CallEvent &Call) {
+ ProgramStateRef State = C.getState();
+ SValBuilder &SVB = C.getSValBuilder();
+
+ ProgramStateRef StateSuccess = State->BindExpr(
+ Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(0, true));
+ StateSuccess = setErrnoState(StateSuccess, MustNotBeChecked);
+
+ ProgramStateRef StateFailure1 = State->BindExpr(
+ Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(1, true));
+ StateFailure1 = setErrnoValue(StateFailure1, C, 1, Irrelevant);
+
+ ProgramStateRef StateFailure2 = State->BindExpr(
+ Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(2, true));
+ StateFailure2 = setErrnoValue(StateFailure2, C, 2, MustBeChecked);
+
+ C.addTransition(StateSuccess,
+ getErrnoNoteTag(C, "Assuming that this function succeeds but "
+ "sets 'errno' to an unspecified value."));
+ C.addTransition(StateFailure1);
+ C.addTransition(
+ StateFailure2,
+ getErrnoNoteTag(C, "Assuming that this function returns 2. 'errno' "
+ "should be checked to test for failure."));
+}
+
bool ErrnoTesterChecker::evalCall(const CallEvent &Call,
CheckerContext &C) const {
const EvalFn *Fn = TestCalls.lookup(Call);
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -21,16 +21,55 @@
namespace ento {
namespace errno_modeling {
+enum ErrnoCheckState : unsigned {
+ /// We do not know anything about 'errno'.
+ Irrelevant = 0,
+
+ /// Value of 'errno' should be checked to find out if a previous function call
+ /// has failed.
+ MustBeChecked = 1,
+
+ /// Value of 'errno' is not allowed to be read, it can contain an unspecified
+ /// value.
+ MustNotBeChecked = 2
+};
+
/// Returns the value of 'errno', if 'errno' was found in the AST.
llvm::Optional<SVal> getErrnoValue(ProgramStateRef State);
+/// Returns the errno check state, \c Errno_Irrelevant if 'errno' was not found
+/// (this is not the only case for that value).
+ErrnoCheckState getErrnoState(ProgramStateRef State);
+
+/// Returns the location that points to the \c MemoryRegion where the 'errno'
+/// value is stored. Returns \c None if 'errno' was not found. Otherwise it
+/// always returns a valid memory region in the system global memory space.
+llvm::Optional<Loc> getErrnoLoc(ProgramStateRef State);
+
/// Set value of 'errno' to any SVal, if possible.
+/// The errno check state is set always when the 'errno' value is set.
ProgramStateRef setErrnoValue(ProgramStateRef State,
- const LocationContext *LCtx, SVal Value);
+ const LocationContext *LCtx, SVal Value,
+ ErrnoCheckState EState);
/// Set value of 'errno' to a concrete (signed) integer, if possible.
+/// The errno check state is set always when the 'errno' value is set.
ProgramStateRef setErrnoValue(ProgramStateRef State, CheckerContext &C,
- uint64_t Value);
+ uint64_t Value, ErrnoCheckState EState);
+
+/// Set the errno check state, do not modify the errno value.
+ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState);
+
+/// Determine if a `Decl` node related to 'errno'.
+/// This is true if the declaration is the errno variable or a function
+/// that returns a pointer to the 'errno' value (usually the 'errno' macro is
+/// defined with this function). \p D is not required to be a canonical
+/// declaration.
+bool isErrno(const Decl *D);
+
+/// Create a NoteTag that displays the message if the 'errno' memory region is
+/// marked as interesting, and resets the interestingness.
+const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message);
} // namespace errno_modeling
} // namespace ento
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -70,6 +70,8 @@
/// The value is null if the 'errno' value was not recognized in the AST.
REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoRegion, const MemRegion *)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoState, errno_modeling::ErrnoCheckState)
+
/// Search for a variable called "errno" in the AST.
/// Return nullptr if not found.
static const VarDecl *getErrnoVar(ASTContext &ACtx) {
@@ -141,7 +143,8 @@
State->getRegion(ErrnoVar, C.getLocationContext());
assert(ErrnoR && "Memory region should exist for the 'errno' variable.");
State = State->set<ErrnoRegion>(ErrnoR);
- State = errno_modeling::setErrnoValue(State, C, 0);
+ State =
+ errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant);
C.addTransition(State);
} else if (ErrnoDecl) {
assert(isa<FunctionDecl>(ErrnoDecl) && "Invalid errno location function.");
@@ -168,7 +171,8 @@
ACtx.IntTy, SVB.makeZeroArrayIndex(),
RMgr.getSymbolicRegion(Sym, GlobalSystemSpace), C.getASTContext());
State = State->set<ErrnoRegion>(ErrnoR);
- State = errno_modeling::setErrnoValue(State, C, 0);
+ State =
+ errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant);
C.addTransition(State);
}
}
@@ -195,7 +199,7 @@
void ErrnoModeling::checkLiveSymbols(ProgramStateRef State,
SymbolReaper &SR) const {
// The special errno region should never garbage collected.
- if (const MemRegion *ErrnoR = State->get<ErrnoRegion>())
+ if (const auto *ErrnoR = State->get<ErrnoRegion>())
SR.markLive(ErrnoR);
}
@@ -212,22 +216,63 @@
}
ProgramStateRef setErrnoValue(ProgramStateRef State,
- const LocationContext *LCtx, SVal Value) {
+ const LocationContext *LCtx, SVal Value,
+ ErrnoCheckState EState) {
const MemRegion *ErrnoR = State->get<ErrnoRegion>();
if (!ErrnoR)
return State;
- return State->bindLoc(loc::MemRegionVal{ErrnoR}, Value, LCtx);
+ // First set the errno value, the old state is still available at 'checkBind'
+ // or 'checkLocation' for errno value.
+ State = State->bindLoc(loc::MemRegionVal{ErrnoR}, Value, LCtx);
+ return State->set<ErrnoState>(EState);
}
ProgramStateRef setErrnoValue(ProgramStateRef State, CheckerContext &C,
- uint64_t Value) {
+ uint64_t Value, ErrnoCheckState EState) {
const MemRegion *ErrnoR = State->get<ErrnoRegion>();
if (!ErrnoR)
return State;
- return State->bindLoc(
+ State = State->bindLoc(
loc::MemRegionVal{ErrnoR},
C.getSValBuilder().makeIntVal(Value, C.getASTContext().IntTy),
C.getLocationContext());
+ return State->set<ErrnoState>(EState);
+}
+
+Optional<Loc> getErrnoLoc(ProgramStateRef State) {
+ const MemRegion *ErrnoR = State->get<ErrnoRegion>();
+ if (!ErrnoR)
+ return {};
+ return loc::MemRegionVal{ErrnoR};
+}
+
+ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState) {
+ return State->set<ErrnoState>(EState);
+}
+
+ErrnoCheckState getErrnoState(ProgramStateRef State) {
+ return State->get<ErrnoState>();
+}
+
+bool isErrno(const Decl *D) {
+ if (const auto *VD = dyn_cast_or_null<VarDecl>(D))
+ if (const IdentifierInfo *II = VD->getIdentifier())
+ return II->getName() == ErrnoVarName;
+ if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
+ if (const IdentifierInfo *II = FD->getIdentifier())
+ return llvm::is_contained(ErrnoLocationFuncNames, II->getName());
+ return false;
+}
+
+const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message) {
+ return C.getNoteTag([Message](PathSensitiveBugReport &BR) -> std::string {
+ const MemRegion *ErrnoR = BR.getErrorNode()->getState()->get<ErrnoRegion>();
+ if (ErrnoR && BR.isInteresting(ErrnoR)) {
+ BR.markNotInteresting(ErrnoR);
+ return Message;
+ }
+ return "";
+ });
}
} // namespace errno_modeling
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
@@ -0,0 +1,249 @@
+//=== ErrnoChecker.cpp ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines an "errno checker" that can detect some invalid use of the
+// system-defined value 'errno'. This checker works together with the
+// ErrnoModeling checker and other checkers like StdCLibraryFunctions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ErrnoModeling.h"
+#include "clang/AST/ParentMapContext.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang;
+using namespace ento;
+using namespace errno_modeling;
+
+namespace {
+
+class ErrnoChecker
+ : public Checker<check::Location, check::PreCall, check::RegionChanges> {
+public:
+ void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext &) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+ ProgramStateRef
+ checkRegionChanges(ProgramStateRef State,
+ const InvalidatedSymbols *Invalidated,
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions,
+ const LocationContext *LCtx, const CallEvent *Call) const;
+ void checkBranchCondition(const Stmt *Condition, CheckerContext &Ctx) const;
+
+ /// Indicates if a read (load) of \c errno is allowed in a non-condition part
+ /// of \c if, \c switch, loop and conditional statements when the errno
+ /// value may be undefined.
+ bool AllowErrnoReadOutsideConditions = true;
+
+private:
+ void generateErrnoNotCheckedBug(CheckerContext &C, ProgramStateRef State,
+ const MemRegion *ErrnoRegion,
+ const CallEvent *CallMayChangeErrno) const;
+
+ BugType BT_InvalidErrnoRead{this, "Value of 'errno' could be undefined",
+ "Error handling"};
+ BugType BT_ErrnoNotChecked{this, "Value of 'errno' was not checked",
+ "Error handling"};
+};
+
+} // namespace
+
+static ProgramStateRef setErrnoStateIrrelevant(ProgramStateRef State) {
+ return setErrnoState(State, Irrelevant);
+}
+
+/// Check if a statement (expression) or an ancestor of it is in a condition
+/// part of a (conditional, loop, switch) statement.
+static bool isInCondition(const Stmt *S, CheckerContext &C) {
+ ParentMapContext &ParentCtx = C.getASTContext().getParentMapContext();
+ bool CondFound = false;
+ while (S && !CondFound) {
+ const DynTypedNodeList Parents = ParentCtx.getParents(*S);
+ if (Parents.empty())
+ break;
+ const auto *ParentS = Parents[0].get<Stmt>();
+ if (!ParentS || isa<CallExpr>(ParentS))
+ break;
+ switch (ParentS->getStmtClass()) {
+ case Expr::IfStmtClass:
+ CondFound = (S == cast<IfStmt>(ParentS)->getCond());
+ break;
+ case Expr::ForStmtClass:
+ CondFound = (S == cast<ForStmt>(ParentS)->getCond());
+ break;
+ case Expr::DoStmtClass:
+ CondFound = (S == cast<DoStmt>(ParentS)->getCond());
+ break;
+ case Expr::WhileStmtClass:
+ CondFound = (S == cast<WhileStmt>(ParentS)->getCond());
+ break;
+ case Expr::SwitchStmtClass:
+ CondFound = (S == cast<SwitchStmt>(ParentS)->getCond());
+ break;
+ case Expr::ConditionalOperatorClass:
+ CondFound = (S == cast<ConditionalOperator>(ParentS)->getCond());
+ break;
+ case Expr::BinaryConditionalOperatorClass:
+ CondFound = (S == cast<BinaryConditionalOperator>(ParentS)->getCommon());
+ break;
+ default:
+ break;
+ }
+ S = ParentS;
+ }
+ return CondFound;
+}
+
+void ErrnoChecker::generateErrnoNotCheckedBug(
+ CheckerContext &C, ProgramStateRef State, const MemRegion *ErrnoRegion,
+ const CallEvent *CallMayChangeErrno) const {
+ if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
+ SmallString<100> StrBuf;
+ llvm::raw_svector_ostream OS(StrBuf);
+ if (CallMayChangeErrno) {
+ OS << "Value of 'errno' was not checked and may be overwritten by "
+ "function '";
+ const auto *CallD =
+ dyn_cast_or_null<FunctionDecl>(CallMayChangeErrno->getDecl());
+ assert(CallD && CallD->getIdentifier());
+ OS << CallD->getIdentifier()->getName() << "'";
+ } else {
+ OS << "Value of 'errno' was not checked and is overwritten here";
+ }
+ auto BR = std::make_unique<PathSensitiveBugReport>(BT_ErrnoNotChecked,
+ OS.str(), N);
+ BR->markInteresting(ErrnoRegion);
+ C.emitReport(std::move(BR));
+ }
+}
+
+void ErrnoChecker::checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const {
+ Optional<ento::Loc> ErrnoLoc = getErrnoLoc(C.getState());
+ if (!ErrnoLoc)
+ return;
+
+ auto L = Loc.getAs<ento::Loc>();
+ if (!L || *ErrnoLoc != *L)
+ return;
+
+ ProgramStateRef State = C.getState();
+ ErrnoCheckState EState = getErrnoState(State);
+
+ if (IsLoad) {
+ switch (EState) {
+ case MustNotBeChecked:
+ // Read of 'errno' when it may have undefined value.
+ if (!AllowErrnoReadOutsideConditions || isInCondition(S, C)) {
+ if (ExplodedNode *N = C.generateErrorNode()) {
+ auto BR = std::make_unique<PathSensitiveBugReport>(
+ BT_InvalidErrnoRead,
+ "An undefined value may be read from 'errno'", N);
+ BR->markInteresting(ErrnoLoc->getAsRegion());
+ C.emitReport(std::move(BR));
+ }
+ }
+ break;
+ case MustBeChecked:
+ // 'errno' has to be checked. A load is required for this, with no more
+ // information we can assume that it is checked somehow.
+ // After this place 'errno' is allowed to be read and written.
+ State = setErrnoStateIrrelevant(State);
+ C.addTransition(State);
+ break;
+ default:
+ break;
+ }
+ } else {
+ switch (EState) {
+ case MustBeChecked:
+ // 'errno' is overwritten without a read before but it should have been
+ // checked.
+ generateErrnoNotCheckedBug(C, setErrnoStateIrrelevant(State),
+ ErrnoLoc->getAsRegion(), nullptr);
+ break;
+ case MustNotBeChecked:
+ // Write to 'errno' when it is not allowed to be read.
+ // After this place 'errno' is allowed to be read and written.
+ State = setErrnoStateIrrelevant(State);
+ C.addTransition(State);
+ break;
+ default:
+ break;
+ }
+ }
+}
+
+void ErrnoChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ const auto *CallF = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+ if (!CallF)
+ return;
+
+ CallF = CallF->getCanonicalDecl();
+ // If 'errno' must be checked, it should be done as soon as possible, and
+ // before any other call to a system function (something in a system header).
+ // To avoid use of a long list of functions that may change 'errno'
+ // (which may be different with standard library versions) assume that any
+ // function can change it.
+ // A list of special functions can be used that are allowed here without
+ // generation of diagnostic. For now the only such case is 'errno' itself.
+ // Probably 'strerror'?
+ if (CallF->isExternC() && CallF->isGlobal() &&
+ C.getSourceManager().isInSystemHeader(CallF->getLocation()) &&
+ !isErrno(CallF)) {
+ if (getErrnoState(C.getState()) == MustBeChecked) {
+ Optional<ento::Loc> ErrnoLoc = getErrnoLoc(C.getState());
+ assert(ErrnoLoc && "ErrnoLoc should exist if an errno state is set.");
+ generateErrnoNotCheckedBug(C, setErrnoStateIrrelevant(C.getState()),
+ ErrnoLoc->getAsRegion(), &Call);
+ }
+ }
+}
+
+ProgramStateRef ErrnoChecker::checkRegionChanges(
+ ProgramStateRef State, const InvalidatedSymbols *Invalidated,
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions, const LocationContext *LCtx,
+ const CallEvent *Call) const {
+ Optional<ento::Loc> ErrnoLoc = getErrnoLoc(State);
+ if (!ErrnoLoc)
+ return State;
+ const MemRegion *ErrnoRegion = ErrnoLoc->getAsRegion();
+
+ // If 'errno' is invalidated we can not know if it is checked or written into,
+ // allow read and write without bug reports.
+ if (llvm::is_contained(Regions, ErrnoRegion))
+ return setErrnoStateIrrelevant(State);
+
+ // Always reset errno state when the system memory space is invalidated.
+ // The ErrnoRegion is not always found in the list in this case.
+ if (llvm::is_contained(Regions, ErrnoRegion->getMemorySpace()))
+ return setErrnoStateIrrelevant(State);
+
+ return State;
+}
+
+void ento::registerErrnoChecker(CheckerManager &mgr) {
+ const AnalyzerOptions &Opts = mgr.getAnalyzerOptions();
+ auto *Checker = mgr.registerChecker<ErrnoChecker>();
+ Checker->AllowErrnoReadOutsideConditions = Opts.getCheckerBooleanOption(
+ Checker, "AllowErrnoReadOutsideConditionExpressions");
+}
+
+bool ento::shouldRegisterErrnoChecker(const CheckerManager &mgr) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -40,6 +40,7 @@
DynamicTypePropagation.cpp
DynamicTypeChecker.cpp
EnumCastOutOfRangeChecker.cpp
+ ErrnoChecker.cpp
ErrnoModeling.cpp
ErrnoTesterChecker.cpp
ExprInspectionChecker.cpp
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -546,6 +546,18 @@
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>;
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -2520,6 +2520,75 @@
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 supported functions are the same that are modeled by checker
+:ref:`alpha-unix-StdCLibraryFunctionArgs`.
+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)
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -581,6 +581,10 @@
`strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``strsep`` and many more. Although
this checker currently is in list of alpha checkers due to a false positive.
+- Added a new checker ``alpha.unix.Errno``. This can find the first read
+ of ``errno`` after successful standard function calls, such use of ``errno``
+ could be unsafe.
+
- Deprecate the ``-analyzer-store region`` and
``-analyzer-opt-analyze-nested-blocks`` analyzer flags.
These flags are still accepted, but a warning will be displayed.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
