balazske updated this revision to Diff 432929.
balazske added a comment.

added option to allow read-only of `errno`
other small change in the test checker


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122150/new/

https://reviews.llvm.org/D122150

Files:
  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/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,154 @@
     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) {
+    if (errno) {
+    }
+    errno = 0; // no warning after 'errno' was read
+  }
+  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 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_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_errno_store_into_variable() {
+  ErrnoTesterChecker_setErrnoIfError();
+  int a = errno; // AllowNonConditionErrnoRead is on by default, no warning
+}
+
+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,37 @@
+// 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:AllowNonConditionErrnoRead=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:AllowNonConditionErrnoRead=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'}}
+}
Index: clang/test/Analysis/errno-notes.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/errno-notes.c
@@ -0,0 +1,47 @@
+// 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
+
+#include "Inputs/errno_var.h"
+#include "Inputs/system-header-simulator.h"
+
+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{{An undefined value may be read from 'errno'}}
+      // expected-note@-1{{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/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), Errno_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, Errno_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, Errno_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, Errno_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,
+                               Errno_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, Errno_MustNotBeChecked);
+
+  ProgramStateRef StateFailure1 = State->BindExpr(
+      Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(1, true));
+  StateFailure1 = setErrnoValue(StateFailure1, C, 1, Errno_Irrelevant);
+
+  ProgramStateRef StateFailure2 = State->BindExpr(
+      Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(2, true));
+  StateFailure2 = setErrnoValue(StateFailure2, C, 2, Errno_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'.
+  Errno_Irrelevant = 0,
+
+  /// Value of 'errno' should be checked to find out if a previous function call
+  /// has failed.
+  Errno_MustBeChecked = 1,
+
+  /// Value of 'errno' is not allowed to be read, it can contain an unspecified
+  /// value.
+  Errno_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
@@ -76,6 +76,8 @@
   return reinterpret_cast<const MemRegion *>(State->get<ErrnoRegion>());
 }
 
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoState, unsigned)
+
 /// Search for a variable called "errno" in the AST.
 /// Return nullptr if not found.
 static const VarDecl *getErrnoVar(ASTContext &ACtx) {
@@ -147,7 +149,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::Errno_Irrelevant);
     C.addTransition(State);
   } else if (ErrnoDecl) {
     assert(isa<FunctionDecl>(ErrnoDecl) && "Invalid errno location function.");
@@ -174,7 +177,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::Errno_Irrelevant);
     C.addTransition(State);
   }
 }
@@ -218,22 +222,66 @@
 }
 
 ProgramStateRef setErrnoValue(ProgramStateRef State,
-                              const LocationContext *LCtx, SVal Value) {
+                              const LocationContext *LCtx, SVal Value,
+                              ErrnoCheckState EState) {
   const MemRegion *ErrnoR = getErrnoRegion(State);
   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 = getErrnoRegion(State);
   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 = getErrnoRegion(State);
+  if (!ErrnoR)
+    return {};
+  return loc::MemRegionVal{ErrnoR};
+}
+
+ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState) {
+  return State->set<ErrnoState>(EState);
+}
+
+ErrnoCheckState getErrnoState(ProgramStateRef State) {
+  if (unsigned EState = State->get<ErrnoState>())
+    return static_cast<ErrnoCheckState>(EState);
+  return Errno_Irrelevant;
+}
+
+bool isErrno(const Decl *D) {
+  if (const auto *VD = dyn_cast_or_null<VarDecl>(D))
+    return VD->getIdentifier() &&
+           VD->getIdentifier()->getName() == ErrnoVarName;
+  if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
+    if (IdentifierInfo *II = FD->getIdentifier())
+      return llvm::find(ErrnoLocationFuncNames, II->getName()) !=
+             std::end(ErrnoLocationFuncNames);
+  return false;
+}
+
+const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message) {
+  return C.getNoteTag([Message](PathSensitiveBugReport &BR) {
+    const MemRegion *ErrnoR = getErrnoRegion(BR.getErrorNode()->getState());
+    if (ErrnoR && BR.isInteresting(ErrnoR)) {
+      BR.markNotInteresting(ErrnoR);
+      return Message.c_str();
+    }
+    return "";
+  });
 }
 
 } // namespace errno_modeling
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
@@ -0,0 +1,246 @@
+//=== 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.
+//
+//===----------------------------------------------------------------------===//
+
+#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 'errno' is allowed in a non-conditional
+  /// statement.
+  bool AllowNonConditionErrnoRead = 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, Errno_Irrelevant);
+}
+
+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)->getCond());
+      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 Errno_MustNotBeChecked:
+      // Read of 'errno' when it may have undefined value.
+      if (!AllowNonConditionErrnoRead || 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 Errno_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 Errno_MustBeChecked:
+      // 'errno' is overwritten without a read before but it should have been
+      // checked.
+      generateErrnoNotCheckedBug(C, setErrnoStateIrrelevant(State),
+                                 ErrnoLoc->getAsRegion(), nullptr);
+      break;
+    case Errno_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()) == Errno_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->AllowNonConditionErrnoRead =
+      Opts.getCheckerBooleanOption(Checker, "AllowNonConditionErrnoRead");
+}
+
+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,19 @@
 
 let ParentPackage = UnixAlpha in {
 
+def ErrnoChecker : Checker<"Errno">,
+  HelpText<"Check not recommended uses of 'errno'">,
+  Dependencies<[ErrnoModeling]>,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "AllowNonConditionErrnoRead",
+                  "Allow read of undefined value from errno outside of conditions",
+                  "true",
+                  InAlpha,
+                  Hide>, // ??
+  ]>,
+  Documentation<HasDocumentation>;
+
 def ChrootChecker : Checker<"Chroot">,
   HelpText<"Check improper use of chroot">,
   Documentation<HasDocumentation>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to