balazske updated this revision to Diff 453231.
balazske marked 4 inline comments as done.
balazske added a comment.
Small improvements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131879

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

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ 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 @@
   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 differentiate 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 @@
                                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 @@
   /// Usually if a failure return value exists for function, that function
   /// needs different cases for success and failure with different errno
   /// constraints (and different 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 @@
 
     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(
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -21,16 +21,23 @@
 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 @@
 /// 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
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -264,6 +264,12 @@
   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 @@
   });
 }
 
+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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to