Patch to check for integer overflow. It has been

commented on and approved by Richard Smith.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173377 91177308-0d34-0410-b5e6-96231b3b80d8

10 files changed, 101 insertions, 38 deletions

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 715e6cbf9f..e28ef97579 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -570,6 +570,9 @@ public:
   /// integer.
   llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
                           SmallVectorImpl<PartialDiagnosticAt> *Diag=0) const;
+  
+  void EvaluateForOverflow(const ASTContext &Ctx,
+                           SmallVectorImpl<PartialDiagnosticAt> *Diag) const;
 
   /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an
   /// lvalue with link time known address, with no side-effects.
diff --git a/include/clang/Basic/DiagnosticASTKinds.td b/include/clang/Basic/DiagnosticASTKinds.td
index f3606b8ec4..a54f39e1ee 100644
--- a/include/clang/Basic/DiagnosticASTKinds.td
+++ b/include/clang/Basic/DiagnosticASTKinds.td
@@ -106,6 +106,9 @@ def note_constexpr_calls_suppressed : Note<
   "(skipping %0 call%s0 in backtrace; use -fconstexpr-backtrace-limit=0 to "
   "see all)">;
 def note_constexpr_call_here : Note<"in call to '%0'">;
+def warn_integer_constant_overflow : Warning<
+  "overflow in case constant expression results in value %0 of type %1">,
+  InGroup<DiagGroup<"integer-overflow">>;
 
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 74f9b48149..748ec15938 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5756,9 +5756,6 @@ def warn_bool_switch_condition : Warning<
 def warn_case_value_overflow : Warning<
   "overflow converting case value to switch condition type (%0 to %1)">,
   InGroup<Switch>;
-def warn_case_constant_overflow : Warning<
-  "overflow in case constant expression results in value %0">,
-  InGroup<Switch>;
 def err_duplicate_case : Error<"duplicate case value '%0'">;
 def err_duplicate_case_differing_expr : Error<
   "duplicate case value: '%0' and '%1' both equal '%2'">;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 8b826ad350..6c6d384a54 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -3993,7 +3993,8 @@ public:
                                           : SourceLocation());
   }
   ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
-                                 bool DiscardedValue = false);
+                                 bool DiscardedValue = false,
+                                 bool IsConstexpr = false);
   StmtResult ActOnFinishFullStmt(Stmt *Stmt);
 
   // Marks SS invalid if it represents an incomplete type.
@@ -7328,11 +7329,13 @@ private:
                             SourceLocation ReturnLoc);
   void CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr* RHS);
   void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
+  void CheckForIntOverflow(Expr *E);
   void CheckUnsequencedOperations(Expr *E);
 
   /// \brief Perform semantic checks on a completed expression. This will either
   /// be a full-expression or a default argument expression.
-  void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation());
+  void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation(),
+                          bool IsConstexpr = false);
 
   void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,
                                    Expr *Init);
diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index 71489cb03c..881182bba8 100644
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -384,13 +384,17 @@ namespace {
     /// expression is a potential constant expression? If so, some diagnostics
     /// are suppressed.
     bool CheckingPotentialConstantExpression;
+    
+    bool IntOverflowCheckMode;
 
-    EvalInfo(const ASTContext &C, Expr::EvalStatus &S)
+    EvalInfo(const ASTContext &C, Expr::EvalStatus &S,
+             bool OverflowCheckMode=false)
       : Ctx(const_cast<ASTContext&>(C)), EvalStatus(S), CurrentCall(0),
         CallStackDepth(0), NextCallIndex(1),
         BottomFrame(*this, SourceLocation(), 0, 0, 0),
         EvaluatingDecl(0), EvaluatingDeclValue(0), HasActiveDiagnostic(false),
-        CheckingPotentialConstantExpression(false) {}
+        CheckingPotentialConstantExpression(false),
+        IntOverflowCheckMode(OverflowCheckMode) {}
 
     void setEvaluatingDecl(const VarDecl *VD, APValue &Value) {
       EvaluatingDecl = VD;
@@ -474,6 +478,8 @@ namespace {
       return OptionalDiagnostic();
     }
 
+    bool getIntOverflowCheckMode() { return IntOverflowCheckMode; }
+    
     /// Diagnose that the evaluation does not produce a C++11 core constant
     /// expression.
     template<typename LocArg>
@@ -506,8 +512,12 @@ namespace {
     /// Should we continue evaluation as much as possible after encountering a
     /// construct which can't be folded?
     bool keepEvaluatingAfterFailure() {
-      return CheckingPotentialConstantExpression &&
-             EvalStatus.Diag && EvalStatus.Diag->empty();
+      // Should return true in IntOverflowCheckMode, so that we check for
+      // overflow even if some subexpressions can't be evaluated as constants.
+      // subexpressions can't be evaluated as constants.
+      return IntOverflowCheckMode ||
+             (CheckingPotentialConstantExpression &&
+              EvalStatus.Diag && EvalStatus.Diag->empty());
     }
   };
 
@@ -4418,8 +4428,14 @@ static APSInt CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
 
   APSInt Value(Op(LHS.extend(BitWidth), RHS.extend(BitWidth)), false);
   APSInt Result = Value.trunc(LHS.getBitWidth());
-  if (Result.extend(BitWidth) != Value)
-    HandleOverflow(Info, E, Value, E->getType());
+  if (Result.extend(BitWidth) != Value) {
+    if (Info.getIntOverflowCheckMode())
+      Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
+        diag::warn_integer_constant_overflow)
+          << Result.toString(10) << E->getType();
+    else
+      HandleOverflow(Info, E, Value, E->getType());
+  }
   return Result;
 }
 
@@ -6260,26 +6276,39 @@ static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {
   return CheckConstantExpression(Info, E->getExprLoc(), E->getType(), Result);
 }
 
-/// EvaluateAsRValue - Return true if this is a constant which we can fold using
-/// any crazy technique (that has nothing to do with language standards) that
-/// we want to.  If this function returns true, it returns the folded constant
-/// in Result. If this expression is a glvalue, an lvalue-to-rvalue conversion
-/// will be applied to the result.
-bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const {
+static bool FastEvaluateAsRValue(const Expr *Exp, Expr::EvalResult &Result,
+                                 const ASTContext &Ctx, bool &IsConst) {
   // Fast-path evaluations of integer literals, since we sometimes see files
   // containing vast quantities of these.
-  if (const IntegerLiteral *L = dyn_cast<IntegerLiteral>(this)) {
+  if (const IntegerLiteral *L = dyn_cast<IntegerLiteral>(Exp)) {
     Result.Val = APValue(APSInt(L->getValue(),
                                 L->getType()->isUnsignedIntegerType()));
+    IsConst = true;
     return true;
   }
-
+  
   // FIXME: Evaluating values of large array and record types can cause
   // performance problems. Only do so in C++11 for now.
-  if (isRValue() && (getType()->isArrayType() || getType()->isRecordType()) &&
-      !Ctx.getLangOpts().CPlusPlus11)
-    return false;
+  if (Exp->isRValue() && (Exp->getType()->isArrayType() ||
+                          Exp->getType()->isRecordType()) &&
+      !Ctx.getLangOpts().CPlusPlus11) {
+    IsConst = false;
+    return true;
+  }
+  return false;
+}
+
 
+/// EvaluateAsRValue - Return true if this is a constant which we can fold using
+/// any crazy technique (that has nothing to do with language standards) that
+/// we want to.  If this function returns true, it returns the folded constant
+/// in Result. If this expression is a glvalue, an lvalue-to-rvalue conversion
+/// will be applied to the result.
+bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const {
+  bool IsConst;
+  if (FastEvaluateAsRValue(this, Result, Ctx, IsConst))
+    return IsConst;
+  
   EvalInfo Info(Ctx, Result);
   return ::EvaluateAsRValue(Info, this, Result.Val);
 }
@@ -6376,6 +6405,17 @@ APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx,
   return EvalResult.Val.getInt();
 }
 
+void Expr::EvaluateForOverflow(const ASTContext &Ctx,
+                    SmallVectorImpl<PartialDiagnosticAt> *Diags) const {
+  bool IsConst;
+  EvalResult EvalResult;
+  EvalResult.Diag = Diags;
+  if (!FastEvaluateAsRValue(this, EvalResult, Ctx, IsConst)) {
+    EvalInfo Info(Ctx, EvalResult, true);
+    (void)::EvaluateAsRValue(Info, this, EvalResult.Val);
+  }
+}
+
  bool Expr::EvalResult::isGlobalLValue() const {
    assert(Val.isLValue());
    return IsGlobalLValue(Val.getLValueBase());
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 0d8f764df5..9af16f3201 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -5183,6 +5183,18 @@ void Sema::CheckImplicitConversions(Expr *E, SourceLocation CC) {
   AnalyzeImplicitConversions(*this, E, CC);
 }
 
+/// Diagnose when expression is an integer constant expression and its evaluation
+/// results in integer overflow
+void Sema::CheckForIntOverflow (Expr *E) {
+  if (const BinaryOperator *BExpr = dyn_cast<BinaryOperator>(E->IgnoreParens())) {
+    unsigned Opc = BExpr->getOpcode();
+    if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Mul)
+      return;
+    llvm::SmallVector<PartialDiagnosticAt, 4> Diags;
+    E->EvaluateForOverflow(Context, &Diags);
+  }
+}
+
 namespace {
 /// \brief Visitor for expressions which looks for unsequenced operations on the
 /// same object.
@@ -5622,9 +5634,12 @@ void Sema::CheckUnsequencedOperations(Expr *E) {
   }
 }
 
-void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc) {
+void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
+                              bool IsConstexpr) {
   CheckImplicitConversions(E, CheckLoc);
   CheckUnsequencedOperations(E);
+  if (!IsConstexpr && !E->isValueDependent())
+    CheckForIntOverflow(E);
 }
 
 void Sema::CheckBitFieldInitialization(SourceLocation InitLoc,
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index ca9610b7d7..538434db00 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -7045,7 +7045,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init,
   //   struct T { S a, b; } t = { Temp(), Temp() }
   //
   // we should destroy the first Temp before constructing the second.
-  ExprResult Result = ActOnFinishFullExpr(Init, VDecl->getLocation());
+  ExprResult Result = ActOnFinishFullExpr(Init, VDecl->getLocation(),
+                                          false,
+                                          VDecl->isConstexpr());
   if (Result.isInvalid()) {
     VDecl->setInvalidDecl();
     return;
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 2658a3a2ce..c70041ba51 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -5469,7 +5469,8 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
 }
 
 ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
-                                     bool DiscardedValue) {
+                                     bool DiscardedValue,
+                                     bool IsConstexpr) {
   ExprResult FullExpr = Owned(FE);
 
   if (!FullExpr.get())
@@ -5497,7 +5498,7 @@ ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
       return ExprError();
   }
 
-  CheckCompletedExpr(FullExpr.get(), CC);
+  CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
   return MaybeCreateExprWithCleanups(FullExpr);
 }
 
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index a2daa18d45..fd3ee0d9f5 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -338,6 +338,12 @@ Sema::ActOnCaseStmt(SourceLocation CaseLoc, Expr *LHSVal,
       // Recover from an error by just forgetting about it.
     }
   }
+  
+  LHSVal = ActOnFinishFullExpr(LHSVal, LHSVal->getExprLoc(), false,
+                               getLangOpts().CPlusPlus11).take();
+  if (RHSVal)
+    RHSVal = ActOnFinishFullExpr(RHSVal, RHSVal->getExprLoc(), false,
+                                 getLangOpts().CPlusPlus11).take();
 
   CaseStmt *CS = new (Context) CaseStmt(LHSVal, RHSVal, CaseLoc, DotDotDotLoc,
                                         ColonLoc);
@@ -732,14 +738,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
       } else {
         // We already verified that the expression has a i-c-e value (C99
         // 6.8.4.2p3) - get that value now.
-        SmallVector<PartialDiagnosticAt, 8> Diags;
-        LoVal = Lo->EvaluateKnownConstInt(Context, &Diags);
-        if (Diags.size() == 1 && 
-            Diags[0].second.getDiagID() == diag::note_constexpr_overflow) {
-          Diag(Lo->getLocStart(), diag::warn_case_constant_overflow) <<
-            LoVal.toString(10);
-          Diag(Diags[0].first, Diags[0].second);
-        }
+        LoVal = Lo->EvaluateKnownConstInt(Context);
 
         // If the LHS is not the same type as the condition, insert an implicit
         // cast.
diff --git a/test/Sema/switch-1.c b/test/Sema/switch-1.c
index e029bc9ffb..bcf30a8d50 100644
--- a/test/Sema/switch-1.c
+++ b/test/Sema/switch-1.c
@@ -4,12 +4,12 @@
 
 int f(int i) {
   switch (i) {
-    case 2147483647 + 2: // expected-note {{value 2147483649 is outside the range of representable values of type 'int'}}  \
-                      // expected-warning {{overflow in case constant expression results in value -2147483647}} 
+    case 2147483647 + 2: // expected-warning {{overflow in case constant expression results in value -2147483647 of type 'int'}}
       return 1;
-    case 9223372036854775807L * 4 : // expected-note {{value 36893488147419103228 is outside the range of representable values of type 'long'}}   \
-                        // expected-warning {{overflow in case constant expression results in value -4}} 
+    case 9223372036854775807L * 4: // expected-warning {{overflow in case constant expression results in value -4 of type 'long'}}
       return 2;
+    case (123456 *789012) + 1:  // expected-warning {{overflow in case constant expression results in value -1375982336 of type 'int'}}
+      return 3;
     case 2147483647:
       return 0;
   }