[PATCH] Catch more cases when diagnosing integer-constant-expression overflows.

[Josh Magee]

jmagee added you to the CC list for the revision "Catch more cases when diagnosing integer-constant-expression overflows.".

Hi,

This patch improves integer-constant-expression overflow diagnostics.

Currently clang catches overflow in some cases:

int i;
...
switch (i) {
  case 4096 * 1024 * 1024:  // Generates overflow warning
  ...
}

if (4096 * 1024 * 1024) { // Generates overflow warning
}

But it misses other cases:

uint64_t x = 4096 * 1024 * 1024; // no warning

if ((x = 4096 * 1024 * 1024)) { // no warning
}

switch (i) {
  case (uint64_t)(4096 * 1024 * 1024): // no warning
...
}

void f1(int);
...
f1(4096 * 1024 * 1024); // no warning


The patch introduces the following changes:
 1) In Sema::CheckForIntOverflow, when checking for a BinaryOperator ignore
    any casts in addition to parentheses.
   Ignoring only parentheses was not enough because it missed cases like:
   x = 4608 * 1024 * 1024, which has an implicit cast:
    ImplicitCastExpr 0x5145858 'uint64_t':'unsigned long' <IntegralCast>
    `-BinaryOperator 0x5145830 'int' '*'
      |-BinaryOperator 0x51457e8 'int' '*'
      | |-IntegerLiteral 0x51457a8 'int' 4608
      | `-IntegerLiteral 0x51457c8 'int' 1024
      `-IntegerLiteral 0x5145810 'int' 1024

 2) In Sema::CheckForIntOverflow, check for CallExprs and recursively check for
    overflow on each argument.
  This handles cases like f1(4096 * 1024 * 1024).

 3) In IntExprEvaluator::VisitBinaryOperator(), bail out on AssignmentOps
   _after_ running the DataRecursiveIntBinOpEvaluator. In fact, I am not sure
   if check here for AssignmentOps is really necessary.  It seems like it is an
   early-exit optimization.  Presumably it was thought that assignment
   operators would not have any integer expression operands that need
   evaluating here, however this is not the case in "x = y = 4096 * 1024 * 1024"
   or "if ((y = 4096 * 1024 *1024))".  Running the DataRecursiveIntBinOpEvaluator
   on AssignmentOps catches these cases.  Note, this change was only needed to
   catch these cases in C mode.  In C++ mode an implicit LValueToRValue cast was
   added which changed the path taken through semantic analysis.


Thanks,
Josh

http://llvm-reviews.chandlerc.com/D1238

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/Sema/integer-overflow.c
  test/SemaCXX/integer-overflow.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1238.1.patch
Type: text/x-patch
Size: 20551 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130730/c583965e/attachment.bin>