[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki created this revision.
danielmarjamaki added reviewers: NoQ, xazax.hun.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
danielmarjamaki requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Diagnose signed integer overflows. The core.UndefResultChecker will warn.

This is a bit unfinished.. I wonder if you like the approach. It only checks 
addition right now. and not underflow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92634

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/integer-overflow.c


Index: clang/test/Analysis/integer-overflow.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/integer-overflow.c
@@ -0,0 +1,7 @@
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core.UndefinedBinaryOperatorResult -verify %s
+
+void f(int x) {
+    if (x > 0x7fffff00 && x + 32 < 0x7fffffff){}
+    if (x > 0x7ffffff0 && x + 32 < 0x7fffffff){} // expected-warning {{The result of the '+' expression is undefined}}
+}
+
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -42,6 +42,8 @@
   SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
                    Loc lhs, NonLoc rhs, QualType resultTy) override;
 
+  bool isGreater(ProgramStateRef State, const SymExpr *LHS, int64_t RHS);
+
   /// getKnownValue - evaluates a given SVal. If the SVal has only one possible
   ///  (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
@@ -51,7 +53,8 @@
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
 
   SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
-                     const llvm::APSInt &RHS, QualType resultTy);
+                     const llvm::APSInt &RHS, QualType resultTy,
+                     ProgramStateRef State);
 };
 } // end anonymous namespace
 
@@ -210,14 +213,29 @@
   }
 }
 
+bool SimpleSValBuilder::isGreater(ProgramStateRef State, const SymExpr *LHS,
+                                  int64_t RHS) {
+  ProgramStateManager &Mgr = State->getStateManager();
+  SValBuilder &Bldr = Mgr.getSValBuilder();
+  SVal Eval =
+      Bldr.evalBinOp(State, BO_GT, nonloc::SymbolVal(LHS),
+                     makeIntVal(RHS, LHS->getType()), Bldr.getConditionType());
+  if (Eval.isUnknownOrUndef())
+    return false;
+  ProgramStateRef StTrue, StFalse;
+  std::tie(StTrue, StFalse) = State->assume(Eval.castAs<DefinedSVal>());
+  return (StTrue && !StFalse);
+}
+
 //===----------------------------------------------------------------------===//
 // Transfer function for binary operators.
 //===----------------------------------------------------------------------===//
 
 SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
-                                    BinaryOperator::Opcode op,
-                                    const llvm::APSInt &RHS,
-                                    QualType resultTy) {
+                                      BinaryOperator::Opcode op,
+                                      const llvm::APSInt &RHS,
+                                      QualType resultTy,
+                                      ProgramStateRef State) {
   bool isIdempotent = false;
 
   // Check for a few special cases with known reductions first.
@@ -281,6 +299,15 @@
   if (isIdempotent)
       return evalCastFromNonLoc(nonloc::SymbolVal(LHS), resultTy);
 
+  // Is there a signed integer overflow
+  if (LHS->getType()->isSignedIntegerType() && op == BO_Add) {
+    int64_t ResultTyMaxVal =
+        (1ULL << (getContext().getTypeSize(resultTy) - 1)) - 1;
+    if (RHS > 0 && RHS < ResultTyMaxVal &&
+        isGreater(State, LHS, ResultTyMaxVal - RHS.getExtValue()))
+      return UndefinedVal();
+  }
+
   // If we reach this point, the expression cannot be simplified.
   // Make a SymbolVal for the entire expression, after converting the RHS.
   const llvm::APSInt *ConvertedRHS = &RHS;
@@ -748,7 +775,7 @@
           }
 
           // Otherwise, make a SymIntExpr out of the expression.
-          return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy);
+          return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy, state);
         }
       }
 
@@ -764,7 +791,7 @@
 
       // Is the RHS a constant?
       if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
-        return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
+        return MakeSymIntVal(Sym, op, *RHSValue, resultTy, state);
 
       if (Optional<NonLoc> V = tryRearrange(state, op, lhs, rhs, resultTy))
         return *V;
@@ -938,7 +965,7 @@
       // build an expression for use by the constraint manager.
       if (SymbolRef lSym = lhs.getAsLocSymbol(true)) {
         if (BinaryOperator::isComparisonOp(op))
-          return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy);
+          return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy, state);
         return UnknownVal();
       }
       // Special case comparisons to NULL.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits