[clang] bd03ef1 - [analyzer] ApiModeling: Add buffer size arg constraint

Gabor Marton via cfe-commits cfe-commits at lists.llvm.org
Fri May 29 07:14:24 PDT 2020


Author: Gabor Marton
Date: 2020-05-29T16:13:57+02:00
New Revision: bd03ef19beb8a3476d5cd9f744c5fba5ca287c51

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

LOG: [analyzer] ApiModeling: Add buffer size arg constraint

Summary:
Introducing a new argument constraint to confine buffer sizes. It is typical in
C APIs that a parameter represents a buffer and another param holds the size of
the buffer (or the size of the data we want to handle from the buffer).

Reviewers: NoQ, Szelethus, Charusso, steakhal

Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
    clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
    clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
    clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
    clang/test/Analysis/std-c-library-functions-arg-constraints.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
index b48914c53d82..398f9b6ac33a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
@@ -32,6 +32,21 @@ DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
                                             SValBuilder &SVB,
                                             QualType ElementTy);
 
+/// Get the dynamic size for a symbolic value that represents a buffer. If
+/// there is an offsetting to the underlying buffer we consider that too.
+/// Returns with an SVal that represents the size, this is Unknown if the
+/// engine cannot deduce the size.
+/// E.g.
+///   char buf[3];
+///   (buf); // size is 3
+///   (buf + 1); // size is 2
+///   (buf + 3); // size is 0
+///   (buf + 4); // size is -1
+///
+///   char *bufptr;
+///   (bufptr) // size is unknown
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV);
+
 } // namespace ento
 } // namespace clang
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
index fec9fb59b2eb..dc9cd717be9e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -63,28 +63,8 @@ class PlacementNewChecker : public Checker<check::PreStmt<CXXNewExpr>> {
 
 SVal PlacementNewChecker::getExtentSizeOfPlace(const CXXNewExpr *NE,
                                                CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   const Expr *Place = NE->getPlacementArg(0);
-
-  const MemRegion *MRegion = C.getSVal(Place).getAsRegion();
-  if (!MRegion)
-    return UnknownVal();
-  RegionOffset Offset = MRegion->getAsOffset();
-  if (Offset.hasSymbolicOffset())
-    return UnknownVal();
-  const MemRegion *BaseRegion = MRegion->getBaseRegion();
-  if (!BaseRegion)
-    return UnknownVal();
-
-  SValBuilder &SvalBuilder = C.getSValBuilder();
-  NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
-      Offset.getOffset() / C.getASTContext().getCharWidth());
-  DefinedOrUnknownSVal ExtentInBytes =
-      getDynamicSize(State, BaseRegion, SvalBuilder);
-
-  return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
-                               ExtentInBytes, OffsetInBytes,
-                               SvalBuilder.getArrayIndexType());
+  return getDynamicSizeWithOffset(C.getState(), C.getSVal(Place));
 }
 
 SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index aefcad374596..f661f29948b6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -56,6 +56,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 
 using namespace clang;
 using namespace clang::ento;
@@ -108,7 +109,8 @@ class StdLibraryFunctionsChecker
     /// Apply the effects of the constraint on the given program state. If null
     /// is returned then the constraint is not feasible.
     virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
-                                  const Summary &Summary) const = 0;
+                                  const Summary &Summary,
+                                  CheckerContext &C) const = 0;
     virtual ValueConstraintPtr negate() const {
       llvm_unreachable("Not implemented");
     };
@@ -143,7 +145,8 @@ class StdLibraryFunctionsChecker
                                        const Summary &Summary) const;
   public:
     ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
-                          const Summary &Summary) const override {
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
       switch (Kind) {
       case OutOfRange:
         return applyAsOutOfRange(State, Call, Summary);
@@ -178,7 +181,8 @@ class StdLibraryFunctionsChecker
     ArgNo getOtherArgNo() const { return OtherArgN; }
     BinaryOperator::Opcode getOpcode() const { return Opcode; }
     ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
-                          const Summary &Summary) const override;
+                          const Summary &Summary,
+                          CheckerContext &C) const override;
   };
 
   class NotNullConstraint : public ValueConstraint {
@@ -188,7 +192,8 @@ class StdLibraryFunctionsChecker
 
   public:
     ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
-                          const Summary &Summary) const override {
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
       SVal V = getArgSVal(Call, getArgNo());
       if (V.isUndef())
         return State;
@@ -207,6 +212,51 @@ class StdLibraryFunctionsChecker
     }
   };
 
+  // Represents a buffer argument with an additional size argument.
+  // E.g. the first two arguments here:
+  //   ctime_s(char *buffer, rsize_t bufsz, const time_t *time);
+  class BufferSizeConstraint : public ValueConstraint {
+    // The argument which holds the size of the buffer.
+    ArgNo SizeArgN;
+    // The operator we use in apply. This is negated in negate().
+    BinaryOperator::Opcode Op = BO_LE;
+
+  public:
+    BufferSizeConstraint(ArgNo Buffer, ArgNo BufSize)
+        : ValueConstraint(Buffer), SizeArgN(BufSize) {}
+
+    ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
+      // The buffer argument.
+      SVal BufV = getArgSVal(Call, getArgNo());
+      // The size argument.
+      SVal SizeV = getArgSVal(Call, SizeArgN);
+      // The dynamic size of the buffer argument, got from the analyzer engine.
+      SVal BufDynSize = getDynamicSizeWithOffset(State, BufV);
+
+      SValBuilder &SvalBuilder = C.getSValBuilder();
+      SVal Feasible = SvalBuilder.evalBinOp(State, Op, SizeV, BufDynSize,
+                                            SvalBuilder.getContext().BoolTy);
+      if (auto F = Feasible.getAs<DefinedOrUnknownSVal>())
+        return State->assume(*F, true);
+
+      // We can get here only if the size argument or the dynamic size is
+      // undefined. But the dynamic size should never be undefined, only
+      // unknown. So, here, the size of the argument is undefined, i.e. we
+      // cannot apply the constraint. Actually, other checkers like
+      // CallAndMessage should catch this situation earlier, because we call a
+      // function with an uninitialized argument.
+      llvm_unreachable("Size argument or the dynamic size is Undefined");
+    }
+
+    ValueConstraintPtr negate() const override {
+      BufferSizeConstraint Tmp(*this);
+      Tmp.Op = BinaryOperator::negateComparisonOp(Op);
+      return std::make_shared<BufferSizeConstraint>(Tmp);
+    }
+  };
+
   /// The complete list of constraints that defines a single branch.
   typedef std::vector<ValueConstraintPtr> ConstraintSet;
 
@@ -416,8 +466,8 @@ ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsWithinRange(
 }
 
 ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply(
-    ProgramStateRef State, const CallEvent &Call,
-    const Summary &Summary) const {
+    ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+    CheckerContext &C) const {
 
   ProgramStateManager &Mgr = State->getStateManager();
   SValBuilder &SVB = Mgr.getSValBuilder();
@@ -448,8 +498,8 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
 
   ProgramStateRef NewState = State;
   for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
-    ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
-    ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
+    ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary, C);
+    ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary, C);
     // The argument constraint is not satisfied.
     if (FailureSt && !SuccessSt) {
       if (ExplodedNode *N = C.generateErrorNode(NewState))
@@ -482,7 +532,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
   for (const auto &VRS : Summary.CaseConstraints) {
     ProgramStateRef NewState = State;
     for (const auto &VR: VRS) {
-      NewState = VR->apply(NewState, Call, Summary);
+      NewState = VR->apply(NewState, Call, Summary, C);
       if (!NewState)
         break;
     }
@@ -694,6 +744,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                               IntRangeVector Ranges) {
     return std::make_shared<RangeConstraint>(ArgN, Kind, Ranges);
   };
+  auto BufferSize = [](ArgNo BufArgN, ArgNo SizeArgN) {
+    return std::make_shared<BufferSizeConstraint>(BufArgN, SizeArgN);
+  };
   struct {
     auto operator()(RangeKind Kind, IntRangeVector Ranges) {
       return std::make_shared<RangeConstraint>(Ret, Kind, Ranges);
@@ -929,6 +982,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                                     RetType{IntTy}, EvalCallAsPure)
                                 .ArgConstraint(NotNull(ArgNo(0)))
                                 .ArgConstraint(NotNull(ArgNo(1))));
+    addToFunctionSummaryMap(
+        "__buf_size_arg_constraint",
+        Summary(ArgTypes{ConstVoidPtrTy, SizeTy}, RetType{IntTy},
+                EvalCallAsPure)
+            .ArgConstraint(
+                BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1))));
   }
 }
 

diff  --git a/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp b/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
index f90c29c52f0f..8b2172db445c 100644
--- a/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
+++ b/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
@@ -44,5 +44,28 @@ DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
   return DivisionV.castAs<DefinedOrUnknownSVal>();
 }
 
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV) {
+  SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();
+  const MemRegion *MRegion = BufV.getAsRegion();
+  if (!MRegion)
+    return UnknownVal();
+  RegionOffset Offset = MRegion->getAsOffset();
+  if (Offset.hasSymbolicOffset())
+    return UnknownVal();
+  const MemRegion *BaseRegion = MRegion->getBaseRegion();
+  if (!BaseRegion)
+    return UnknownVal();
+
+  NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
+      Offset.getOffset() /
+      MRegion->getMemRegionManager().getContext().getCharWidth());
+  DefinedOrUnknownSVal ExtentInBytes =
+      getDynamicSize(State, BaseRegion, SvalBuilder);
+
+  return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
+                               ExtentInBytes, OffsetInBytes,
+                               SvalBuilder.getArrayIndexType());
+}
+
 } // namespace ento
 } // namespace clang

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index 00960e42fb55..c59e4429f419 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -122,3 +122,30 @@ void test_arg_constraint_on_variadic_fun() {
   // bugpath-warning{{Function argument constraint is not satisfied}} \
   // bugpath-note{{Function argument constraint is not satisfied}}
 }
+
+int __buf_size_arg_constraint(const void *, size_t);
+void test_buf_size_concrete() {
+  char buf[3];                       // bugpath-note{{'buf' initialized here}}
+  __buf_size_arg_constraint(buf, 4); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+}
+void test_buf_size_symbolic(int s) {
+  char buf[3];
+  __buf_size_arg_constraint(buf, s);
+  clang_analyzer_eval(s <= 3); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{'s' is <= 3}}
+}
+void test_buf_size_symbolic_and_offset(int s) {
+  char buf[3];
+  __buf_size_arg_constraint(buf + 1, s);
+  clang_analyzer_eval(s <= 2); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{'s' is <= 2}}
+}


        


More information about the cfe-commits mailing list