[clang] d56a1c6 - [clang][analyzer] Errno modeling code refactor (NFC).

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 1 00:06:53 PDT 2022


Author: Balázs Kéri
Date: 2022-09-01T09:05:59+02:00
New Revision: d56a1c68247751e94c4fc46dda282643d3739689

URL: https://github.com/llvm/llvm-project/commit/d56a1c68247751e94c4fc46dda282643d3739689
DIFF: https://github.com/llvm/llvm-project/commit/d56a1c68247751e94c4fc46dda282643d3739689.diff

LOG: [clang][analyzer] Errno modeling code refactor (NFC).

Some of the code used in StdLibraryFunctionsChecker is applicable to
other checkers, this is put into common functions. Errno related
parts of the checker are simplified and renamed. Documentations in
errno_modeling functions are updated.

This change makes it available to have more checkers that perform
modeling of some standard functions. These can set the errno state
with common functions and the bug report messages (note tags) can
look similar.

Reviewed By: steakhal, martong

Differential Revision: https://reviews.llvm.org/D131879

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
    clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
    clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index 618f7e97f6e89..a9e249de38047 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -264,6 +264,12 @@ bool isErrno(const Decl *D) {
   return false;
 }
 
+const char *describeErrnoCheckState(ErrnoCheckState CS) {
+  assert(CS == errno_modeling::MustNotBeChecked &&
+         "Errno description not applicable.");
+  return "may be undefined after the call and should not be used";
+}
+
 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>();
@@ -275,6 +281,32 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message) {
   });
 }
 
+ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State,
+                                      CheckerContext &C) {
+  return setErrnoState(State, MustNotBeChecked);
+}
+
+ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
+                                      NonLoc ErrnoSym) {
+  SValBuilder &SVB = C.getSValBuilder();
+  NonLoc ZeroVal = SVB.makeZeroVal(C.getASTContext().IntTy).castAs<NonLoc>();
+  DefinedOrUnknownSVal Cond =
+      SVB.evalBinOp(State, BO_NE, ErrnoSym, ZeroVal, SVB.getConditionType())
+          .castAs<DefinedOrUnknownSVal>();
+  State = State->assume(Cond, true);
+  if (!State)
+    return nullptr;
+  return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
+}
+
+const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
+  return getErrnoNoteTag(
+      C, (Twine("Assuming that function '") + Twine(Fn) +
+          Twine("' is successful, in this case the value 'errno' ") +
+          Twine(describeErrnoCheckState(MustNotBeChecked)))
+             .str());
+}
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 3757e25e1afe6..d46a403d59a54 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -21,16 +21,23 @@ namespace clang {
 namespace ento {
 namespace errno_modeling {
 
+/// Describe how reads and writes of \c errno are handled by the checker.
 enum ErrnoCheckState : unsigned {
   /// We do not know anything about 'errno'.
+  /// Read and write is always allowed.
   Irrelevant = 0,
 
   /// Value of 'errno' should be checked to find out if a previous function call
   /// has failed.
+  /// When this state is set \c errno must be read by the program before a next
+  /// standard function call or other overwrite of \c errno follows, otherwise
+  /// a bug report is emitted.
   MustBeChecked = 1,
 
   /// Value of 'errno' is not allowed to be read, it can contain an unspecified
   /// value.
+  /// When this state is set \c errno is not allowed to be read by the program
+  /// until it is overwritten or invalidated.
   MustNotBeChecked = 2
 };
 
@@ -67,10 +74,38 @@ ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState);
 /// declaration.
 bool isErrno(const Decl *D);
 
+/// Produce a textual description about how \c errno is allowed to be used
+/// (in a \c ErrnoCheckState).
+/// The returned string is insertable into a longer warning message in the form
+/// "the value 'errno' <...>".
+/// Currently only the \c errno_modeling::MustNotBeChecked state is supported,
+/// others are not used by the clients.
+const char *describeErrnoCheckState(ErrnoCheckState CS);
+
 /// 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);
 
+/// Set errno state for the common case when a standard function is successful.
+/// Set \c ErrnoCheckState to \c MustNotBeChecked (the \c errno value is not
+/// affected). At the state transition a note tag created by
+/// \c getNoteTagForStdSuccess can be used.
+ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State, CheckerContext &C);
+
+/// Set errno state for the common case when a standard function fails.
+/// Set \c errno value to be not equal to zero and \c ErrnoCheckState to
+/// \c Irrelevant . The irrelevant errno state ensures that no related bug
+/// report is emitted later and no note tag is needed.
+/// \arg \c ErrnoSym Value to be used for \c errno and constrained to be
+/// non-zero.
+ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
+                                      NonLoc ErrnoSym);
+
+/// Generate the note tag that can be applied at the state generated by
+/// \c setErrnoForStdSuccess .
+/// \arg \c Fn Name of the (standard) function that is modeled.
+const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 5897e5096461a..25e80545154ee 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -57,19 +57,6 @@
 using namespace clang;
 using namespace clang::ento;
 
-/// Produce a textual description of the state of \c errno (this describes the
-/// way how it is allowed to be used).
-/// The returned string is insertable into a longer warning message (in the form
-/// "the value 'errno' <...>").
-/// Currently only the \c errno_modeling::MustNotBeChecked state is supported.
-/// But later other kind of errno state may be needed if functions with special
-/// handling of \c errno are added.
-static const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS) {
-  assert(CS == errno_modeling::MustNotBeChecked &&
-         "Errno description not applicable.");
-  return "may be undefined after the call and should not be used";
-}
-
 namespace {
 class StdLibraryFunctionsChecker
     : public Checker<check::PreCall, check::PostCall, eval::Call> {
@@ -392,45 +379,42 @@ class StdLibraryFunctionsChecker
   using ConstraintSet = std::vector<ValueConstraintPtr>;
 
   /// Define how a function affects the system variable 'errno'.
-  /// This works together with the ErrnoModeling and ErrnoChecker classes.
+  /// This works together with the \c ErrnoModeling and \c ErrnoChecker classes.
+  /// Currently 3 use cases exist: success, failure, irrelevant.
+  /// In the future the failure case can be customized to set \c errno to a
+  /// more specific constraint (for example > 0), or new case can be added
+  /// for functions which require check of \c errno in both success and failure
+  /// case.
   class ErrnoConstraintBase {
   public:
     /// Apply specific state changes related to the errno variable.
     virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
                                   const Summary &Summary,
                                   CheckerContext &C) const = 0;
-    /// Get a description about what is applied to 'errno' and how is it allowed
-    /// to be used. If ErrnoChecker generates a bug then this message is
-    /// displayed as a note at the function call.
-    /// It may return empty string if no note tag is to be added.
-    virtual std::string describe(StringRef FunctionName) const { return ""; }
+    /// Get a NoteTag about the changes made to 'errno' and the possible bug.
+    /// It may return \c nullptr (if no bug report from \c ErrnoChecker is
+    /// expected).
+    virtual const NoteTag *describe(CheckerContext &C,
+                                    StringRef FunctionName) const {
+      return nullptr;
+    }
 
     virtual ~ErrnoConstraintBase() {}
 
   protected:
-    /// Many of the descendant classes use this value.
-    const errno_modeling::ErrnoCheckState CheckState;
-
-    ErrnoConstraintBase(errno_modeling::ErrnoCheckState CS) : CheckState(CS) {}
+    ErrnoConstraintBase() = default;
 
     /// This is used for conjure symbol for errno to 
diff erentiate from the
     /// original call expression (same expression is used for the errno symbol).
     static int Tag;
   };
 
-  /// Set value of 'errno' to be related to 0 in a specified way, with a
-  /// specified "errno check state". For example with \c BO_GT 'errno' is
-  /// constrained to be greater than 0. Use this for failure cases of functions.
-  class ZeroRelatedErrnoConstraint : public ErrnoConstraintBase {
-    BinaryOperatorKind Op;
-
+  /// Set errno constraint at failure cases of standard functions.
+  /// Failure case: 'errno' becomes not equal to 0 and may or may not be checked
+  /// by the program. \c ErrnoChecker does not emit a bug report after such a
+  /// function call.
+  class FailureErrnoConstraint : public ErrnoConstraintBase {
   public:
-    ZeroRelatedErrnoConstraint(clang::BinaryOperatorKind OpK,
-                               errno_modeling::ErrnoCheckState CS)
-        : ErrnoConstraintBase(CS), Op(OpK) {
-      assert(BinaryOperator::isComparisonOp(OpK));
-    }
-
     ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
                           const Summary &Summary,
                           CheckerContext &C) const override {
@@ -440,62 +424,36 @@ class StdLibraryFunctionsChecker
                                C.getLocationContext(), C.getASTContext().IntTy,
                                C.blockCount())
               .castAs<NonLoc>();
-      NonLoc ZeroVal =
-          SVB.makeZeroVal(C.getASTContext().IntTy).castAs<NonLoc>();
-      DefinedOrUnknownSVal Cond =
-          SVB.evalBinOp(State, Op, ErrnoSVal, ZeroVal, SVB.getConditionType())
-              .castAs<DefinedOrUnknownSVal>();
-      State = State->assume(Cond, true);
-      if (!State)
-        return State;
-      return errno_modeling::setErrnoValue(State, C.getLocationContext(),
-                                           ErrnoSVal, CheckState);
-    }
-
-    std::string describe(StringRef FunctionName) const override {
-      if (CheckState == errno_modeling::Irrelevant)
-        return "";
-      return (Twine("Assuming that function '") + FunctionName.str() +
-              "' fails, in this case the value 'errno' becomes " +
-              BinaryOperator::getOpcodeStr(Op).str() + " 0 and " +
-              describeErrnoCheckState(CheckState))
-          .str();
+      return errno_modeling::setErrnoForStdFailure(State, C, ErrnoSVal);
     }
   };
 
-  /// Applies the constraints to 'errno' for a common case when a standard
-  /// function is successful. The value of 'errno' after the call is not
-  /// specified by the standard (it may change or not). The \c ErrnoChecker can
-  /// generate a bug if 'errno' is read afterwards.
+  /// Set errno constraint at success cases of standard functions.
+  /// Success case: 'errno' is not allowed to be used.
+  /// \c ErrnoChecker can emit bug report after such a function call if errno
+  /// is used.
   class SuccessErrnoConstraint : public ErrnoConstraintBase {
   public:
-    SuccessErrnoConstraint()
-        : ErrnoConstraintBase(errno_modeling::MustNotBeChecked) {}
-
     ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
                           const Summary &Summary,
                           CheckerContext &C) const override {
-      return errno_modeling::setErrnoState(State, CheckState);
+      return errno_modeling::setErrnoForStdSuccess(State, C);
     }
 
-    std::string describe(StringRef FunctionName) const override {
-      return (Twine("Assuming that function '") + FunctionName.str() +
-              "' is successful, in this case the value 'errno' " +
-              describeErrnoCheckState(CheckState))
-          .str();
+    const NoteTag *describe(CheckerContext &C,
+                            StringRef FunctionName) const override {
+      return errno_modeling::getNoteTagForStdSuccess(C, FunctionName);
     }
   };
 
-  /// Set errno constraints if use of 'errno' is completely irrelevant to the
+  /// Set errno constraints if use of 'errno' is irrelevant to the
   /// modeled function or modeling is not possible.
   class NoErrnoConstraint : public ErrnoConstraintBase {
   public:
-    NoErrnoConstraint() : ErrnoConstraintBase(errno_modeling::Irrelevant) {}
-
     ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
                           const Summary &Summary,
                           CheckerContext &C) const override {
-      return errno_modeling::setErrnoState(State, CheckState);
+      return errno_modeling::setErrnoState(State, errno_modeling::Irrelevant);
     }
   };
 
@@ -758,10 +716,9 @@ class StdLibraryFunctionsChecker
   /// Usually if a failure return value exists for function, that function
   /// needs 
diff erent cases for success and failure with 
diff erent errno
   /// constraints (and 
diff erent return value constraints).
-  const NoErrnoConstraint ErrnoIrrelevant;
-  const SuccessErrnoConstraint ErrnoMustNotBeChecked;
-  const ZeroRelatedErrnoConstraint ErrnoNEZeroIrrelevant{
-      clang::BinaryOperatorKind::BO_NE, errno_modeling::Irrelevant};
+  const NoErrnoConstraint ErrnoIrrelevant{};
+  const SuccessErrnoConstraint ErrnoMustNotBeChecked{};
+  const FailureErrnoConstraint ErrnoNEZeroIrrelevant{};
 };
 
 int StdLibraryFunctionsChecker::ErrnoConstraintBase::Tag = 0;
@@ -1017,13 +974,10 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
 
     if (NewState && NewState != State) {
       if (Case.getNote().empty()) {
-        std::string Note;
+        const NoteTag *NT = nullptr;
         if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl()))
-          Note = Case.getErrnoConstraint().describe(D->getNameAsString());
-        if (Note.empty())
-          C.addTransition(NewState);
-        else
-          C.addTransition(NewState, errno_modeling::getErrnoNoteTag(C, Note));
+          NT = Case.getErrnoConstraint().describe(C, D->getNameAsString());
+        C.addTransition(NewState, NT);
       } else {
         StringRef Note = Case.getNote();
         const NoteTag *Tag = C.getNoteTag(


        


More information about the cfe-commits mailing list