analyze all kinds of expressions for integer overflow

[nlewycky]

Summary

Remove clang::Sema::CheckForIntOverflow(E) by calling into E->EvaluateForOverflow instead. CheckForIntOverflow implemented a whitelist of top-level expressions to check, currently BinaryOperator, InitListExpr and ObjCBoxedExpr.

Two changes are made to avoid regression with the existing test suite.
test/SemaCXX/eval-sizeof-dependent-type.cpp has an example of a value-dependent InitListExpr. Handle value-dependent initializers in init list exprs.
test/Sema/integer-overflow.c tests that initializers are checked for overflow, which was conditionally disabled for performance. Allow checking for overflow of init list exprs in the performance check.

[rsmith]

If we're now catching integer overflow in more cases, please add some relevant testcases. If this is a pure refactoring that enables those additional diagnostics to be produced in future, then this is fine as-is. (For the benefit of other people looking at this, the test changes here are caused by a pre-existing bug: the notes for why a reference is not usable in a constant expression are only produced if we happen to produce diagnostics the first time we try to evaluate them.)

I have an unsubstantiated performance concern: we've seen this overflow checking having a visible effect on compile times in LNT before, but I'm happy to let you figure out whether there's anything else you can do to measure this before bouncing it off the LNT bots. (And any cases we're checking with this patch but not without it seem to be arbitrary oversights rather than something principled.)

[nlewycky]

Rebase. Now that ObjCBoxedExpr change is in, we can remove Sema::CheckForIntOverflow entirely.

[nlewycky]

If we're now catching integer overflow in more cases, please add some relevant testcases.

Both more and fewer. More because we no longer have a whitelist of three kinds of expressions that we recurse into. Fewer because we no longer call IgnoreParenCasts() on the full-expression so "(void)(4608 * 1024 * 1024);" used to get a warning but now doesn't. The plan to fix this is a patch to call EvaluateIgnoredValue on non-literal types, which will get us that warning back.

I've added two cases to test/Sema/integer-overflow.c and changed one to demonstrate this patch. It previously had an additional + on it just to trigger the int overflow checking.

I have an unsubstantiated performance concern: we've seen this overflow checking having a visible effect on compile times in LNT before

I haven't observed slowdown like I did with my previous attempt at this change in D31839, but yes we may need to back this patch out if it causes problems.

[nlewycky]

[No changes, just full context this time.]

include/clang/Sema/Sema.h

void CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
                           SourceLocation ReturnLoc,
                           bool isObjCMethod = false,
                           const AttrVec *Attrs = nullptr,
                           const FunctionDecl *FD = nullptr);
 
   void CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr* RHS);
   void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
   void CheckBoolLikeConversion(Expr *E, SourceLocation CC);
-  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(),
                           bool IsConstexpr = false);
 
   void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,

lib/AST/ExprConstant.cpp

for (const auto *Field : RD->fields()) {
     if (!HandleLValueMember(Info, HaveInit ? E->getInit(ElementNo) : E,
                             Subobject, Field, &Layout))
       return false;
 
     // Perform an implicit value-initialization for members beyond the end of
     // the initializer list.
     ImplicitValueInitExpr VIE(HaveInit ? Info.Ctx.IntTy : Field->getType());
     const Expr *Init = HaveInit ? E->getInit(ElementNo++) : &VIE;
+    if (Init->isValueDependent()) {
+      Success = false;
+      continue;
+    }
 
     // Temporarily override This, in case there's a CXXDefaultInitExpr in here.
     ThisOverrideRAII ThisOverride(*Info.CurrentCall, &This,
                                   isa<CXXDefaultInitExpr>(Init));
 
     APValue &FieldVal = Result.getStructField(Field->getFieldIndex());
     if (!EvaluateInPlace(FieldVal, Info, Subobject, Init) ||
         (Field->isBitField() && !truncateBitfieldValue(Info, Init,
if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
       return false;
   }
 
   // Check this core constant expression is a constant expression.
   return CheckConstantExpression(Info, E->getExprLoc(), E->getType(), Result);
 }
 
 static bool FastEvaluateAsRValue(const Expr *Exp, Expr::EvalResult &Result,
-                                 const ASTContext &Ctx, bool &IsConst) {
+                                 const ASTContext &Ctx, bool &IsConst,
+                                 bool IsCheckingForOverflow) {
   // Fast-path evaluations of integer literals, since we sometimes see files
   // containing vast quantities of these.
   if (const IntegerLiteral *L = dyn_cast<IntegerLiteral>(Exp)) {
     Result.Val = APValue(APSInt(L->getValue(),
                                 L->getType()->isUnsignedIntegerType()));
     IsConst = true;
     return true;
   }
 
   // This case should be rare, but we need to check it before we check on
   // the type below.
   if (Exp->getType().isNull()) {
     IsConst = false;
     return true;
   }
   
   // FIXME: Evaluating values of large array and record types can cause
   // performance problems. Only do so in C++11 for now.
   if (Exp->isRValue() && (Exp->getType()->isArrayType() ||
                           Exp->getType()->isRecordType()) &&
-      !Ctx.getLangOpts().CPlusPlus11) {
+      !Ctx.getLangOpts().CPlusPlus11 && !IsCheckingForOverflow) {
     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))
+  if (FastEvaluateAsRValue(this, Result, Ctx, IsConst, false))
     return IsConst;
   
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
   return ::EvaluateAsRValue(Info, this, Result.Val);
 }
 
 bool Expr::EvaluateAsBooleanCondition(bool &Result,
                                       const ASTContext &Ctx) const {
APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx,
   assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
 
   return EvalResult.Val.getInt();
 }
 
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
-  if (!FastEvaluateAsRValue(this, EvalResult, Ctx, IsConst)) {
+  if (!FastEvaluateAsRValue(this, EvalResult, Ctx, IsConst, true)) {
     EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
     (void)::EvaluateAsRValue(Info, this, EvalResult.Val);
   }
 }
 
 bool Expr::EvalResult::isGlobalLValue() const {
   assert(Val.isLValue());
   return IsGlobalLValue(Val.getLValueBase());

lib/Sema/SemaChecking.cpp

 }
 
 /// CheckBoolLikeConversion - Check conversion of given expression to boolean.
 /// Input argument E is a logical expression.
 void Sema::CheckBoolLikeConversion(Expr *E, SourceLocation CC) {
   ::CheckBoolLikeConversion(*this, E, CC);
 }
 
-/// Diagnose when expression is an integer constant expression and its evaluation
-/// results in integer overflow
-void Sema::CheckForIntOverflow (Expr *E) {
-  // Use a work list to deal with nested struct initializers.
-  SmallVector<Expr *, 2> Exprs(1, E);
-
-  do {
-    Expr *E = Exprs.pop_back_val();
-
-    if (isa<BinaryOperator>(E->IgnoreParenCasts())) {
-      E->IgnoreParenCasts()->EvaluateForOverflow(Context);
-      continue;
-    }
-
-    if (auto InitList = dyn_cast<InitListExpr>(E))
-      Exprs.append(InitList->inits().begin(), InitList->inits().end());
-
-    if (isa<ObjCBoxedExpr>(E))
-      E->IgnoreParenCasts()->EvaluateForOverflow(Context);
-  } while (!Exprs.empty());
-}
-
 namespace {
 /// \brief Visitor for expressions which looks for unsequenced operations on the
 /// same object.
 class SequenceChecker : public EvaluatedExprVisitor<SequenceChecker> {
   typedef EvaluatedExprVisitor<SequenceChecker> Base;
 
   /// \brief A tree of sequenced regions within an expression. Two regions are
   /// unsequenced if one is an ancestor or a descendent of the other. When we

 }
 
 void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
                               bool IsConstexpr) {
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
     CheckUnsequencedOperations(E);
   if (!IsConstexpr && !E->isValueDependent())
-    CheckForIntOverflow(E);
+    E->EvaluateForOverflow(Context);
   DiagnoseMisalignedMembers();
 }
 
 void Sema::CheckBitFieldInitialization(SourceLocation InitLoc,
                                        FieldDecl *BitField,
                                        Expr *Init) {
   (void) AnalyzeBitFieldAssignment(*this, BitField, Init, InitLoc);
 }

test/OpenMP/distribute_parallel_for_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
 
 #pragma omp target
 #pragma omp teams
 #pragma omp distribute parallel for simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
 
 #pragma omp target

test/OpenMP/distribute_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
 
 #pragma omp target
 #pragma omp teams
 #pragma omp distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
 
 #pragma omp target

test/OpenMP/for_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
   #pragma omp for simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp for simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp for simd aligned () // expected-error {{expected expression}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp for simd aligned (argc // expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/parallel_for_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
   #pragma omp parallel for simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp parallel for simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp parallel for simd aligned () // expected-error {{expected expression}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp parallel for simd aligned (argc // expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
   #pragma omp simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp simd aligned () // expected-error {{expected expression}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp simd aligned (argc // expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/target_parallel_for_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
   #pragma omp target parallel for simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp target parallel for simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp target parallel for simd aligned () // expected-error {{expected expression}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp target parallel for simd aligned (argc // expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/target_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
   #pragma omp target simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp target simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp target simd aligned () // expected-error {{expected expression}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp target simd aligned (argc // expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
 
 #pragma omp target teams distribute parallel for simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
 
 #pragma omp target teams distribute parallel for simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
 

test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
 
 #pragma omp target teams distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
 
 #pragma omp target teams distribute simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
 

test/OpenMP/taskloop_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
   #pragma omp taskloop simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp taskloop simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp taskloop simd aligned () // expected-error {{expected expression}}
   for (I k = 0; k < argc; ++k) ++k;
   #pragma omp taskloop simd aligned (argc // expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
 
 #pragma omp target
 #pragma omp teams distribute parallel for simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
 #pragma omp teams distribute parallel for simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}

test/OpenMP/teams_distribute_simd_aligned_messages.cpp

 };
 
 S3 h; // expected-note 2 {{'h' defined here}}
 #pragma omp threadprivate(h)
 
 template<class I, class C> int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int &j = i;
 
 #pragma omp target
 #pragma omp teams distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
 #pragma omp teams distribute simd aligned ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}

test/Sema/integer-overflow.c

// expected-note@+1 {{array 'a' declared here}}
   uint64_t a[10];
   a[4608 * 1024 * 1024] = 1i;
 
 // expected-warning@+2 {{overflow in expression; result is 536870912 with type 'int'}}
   uint64_t *b;
   uint64_t b2 = b[4608 * 1024 * 1024] + 1;
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
-  (void)((i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024)) + 1);
+  int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
+
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  int j2 = -(4608 * 1024 * 1024);
+
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  uint64_t j3 = b[4608 * 1024 * 1024];
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
 
 struct s {
   unsigned x;
   unsigned y;