Merged @luckyluke's change for detecting overflow of unsigned add, sub

and mul operations.  Refactored tests into two main cases, and
disabled them on LLVM 2.9, which does not support -fsanitized=*signed-integer-overflow.

6 files changed, 176 insertions, 7 deletions

diff --git a/lib/Core/SpecialFunctionHandler.cpp b/lib/Core/SpecialFunctionHandler.cpp
index 59e269cb..43185511 100644
--- a/lib/Core/SpecialFunctionHandler.cpp
+++ b/lib/Core/SpecialFunctionHandler.cpp
@@ -108,6 +108,12 @@ static SpecialFunctionHandler::HandlerInfo handlerInfo[] = {
   // operator new(unsigned long)
   add("_Znwm", handleNew, true),
 
+  // clang -fsanitize=unsigned-integer-overflow
+  add("__ubsan_handle_add_overflow", handleAddOverflow, false),
+  add("__ubsan_handle_sub_overflow", handleSubOverflow, false),
+  add("__ubsan_handle_mul_overflow", handleMulOverflow, false),
+  add("__ubsan_handle_divrem_overflow", handleDivRemOverflow, false),
+
 #undef addDNR
 #undef add  
 };
@@ -707,3 +713,36 @@ void SpecialFunctionHandler::handleMarkGlobal(ExecutionState &state,
     mo->isGlobal = true;
   }
 }
+
+void SpecialFunctionHandler::handleAddOverflow(ExecutionState &state,
+                                               KInstruction *target,
+                                               std::vector<ref<Expr> > &arguments) {
+  executor.terminateStateOnError(state,
+                                 "overflow on unsigned addition",
+                                 "overflow.err");
+}
+
+void SpecialFunctionHandler::handleSubOverflow(ExecutionState &state,
+                                               KInstruction *target,
+                                               std::vector<ref<Expr> > &arguments) {
+  executor.terminateStateOnError(state,
+                                 "overflow on unsigned subtraction",
+                                 "overflow.err");
+}
+
+void SpecialFunctionHandler::handleMulOverflow(ExecutionState &state,
+                                               KInstruction *target,
+                                               std::vector<ref<Expr> > &arguments) {
+  executor.terminateStateOnError(state,
+                                 "overflow on unsigned multiplication",
+                                 "overflow.err");
+}
+
+void SpecialFunctionHandler::handleDivRemOverflow(ExecutionState &state,
+                                               KInstruction *target,
+                                               std::vector<ref<Expr> > &arguments) {
+  executor.terminateStateOnError(state,
+                                 "overflow on division or remainder",
+                                 "overflow.err");
+}
+
diff --git a/lib/Core/SpecialFunctionHandler.h b/lib/Core/SpecialFunctionHandler.h
index f68c6edb..d52b8fc5 100644
--- a/lib/Core/SpecialFunctionHandler.h
+++ b/lib/Core/SpecialFunctionHandler.h
@@ -132,6 +132,10 @@ namespace klee {
     HANDLER(handleUnderConstrained);
     HANDLER(handleWarning);
     HANDLER(handleWarningOnce);
+    HANDLER(handleAddOverflow);
+    HANDLER(handleMulOverflow);
+    HANDLER(handleSubOverflow);
+    HANDLER(handleDivRemOverflow);
 #undef HANDLER
   };
 } // End klee namespace
diff --git a/lib/Module/IntrinsicCleaner.cpp b/lib/Module/IntrinsicCleaner.cpp
index 0f095269..da97621a 100644
--- a/lib/Module/IntrinsicCleaner.cpp
+++ b/lib/Module/IntrinsicCleaner.cpp
@@ -66,13 +66,15 @@ bool IntrinsicCleanerPass::runOnModule(Module &M) {
 
 bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b, Module &M) {
   bool dirty = false;
+  bool block_split=false;
   
 #if LLVM_VERSION_CODE <= LLVM_VERSION(3, 1)
   unsigned WordSize = TargetData.getPointerSizeInBits() / 8;
 #else
   unsigned WordSize = DataLayout.getPointerSizeInBits() / 8;
 #endif
-  for (BasicBlock::iterator i = b.begin(), ie = b.end(); i != ie;) {     
+  for (BasicBlock::iterator i = b.begin(), ie = b.end();
+       (i != ie) && (block_split == false);) {
     IntrinsicInst *ii = dyn_cast<IntrinsicInst>(&*i);
     // increment now since LowerIntrinsic deletion makes iterator invalid.
     ++i;  
@@ -116,7 +118,11 @@ bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b, Module &M) {
         break;
       }
 
+      case Intrinsic::sadd_with_overflow:
+      case Intrinsic::ssub_with_overflow:
+      case Intrinsic::smul_with_overflow:
       case Intrinsic::uadd_with_overflow:
+      case Intrinsic::usub_with_overflow:
       case Intrinsic::umul_with_overflow: {
         IRBuilder<> builder(ii->getParent(), ii);
 
@@ -124,13 +130,71 @@ bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b, Module &M) {
         Value *op2 = ii->getArgOperand(1);
         
         Value *result = 0;
-        if (ii->getIntrinsicID() == Intrinsic::uadd_with_overflow)
-          result = builder.CreateAdd(op1, op2);
-        else
-          result = builder.CreateMul(op1, op2);
+        Value *result_ext = 0;
+        Value *overflow = 0;
+
+        unsigned int bw = op1->getType()->getPrimitiveSizeInBits();
+        unsigned int bw2 = op1->getType()->getPrimitiveSizeInBits()*2;
+
+        if ((ii->getIntrinsicID() == Intrinsic::uadd_with_overflow) ||
+            (ii->getIntrinsicID() == Intrinsic::usub_with_overflow) ||
+            (ii->getIntrinsicID() == Intrinsic::umul_with_overflow)) {
+
+          Value *op1ext =
+            builder.CreateZExt(op1, IntegerType::get(M.getContext(), bw2));
+          Value *op2ext =
+            builder.CreateZExt(op2, IntegerType::get(M.getContext(), bw2));
+          Value *int_max_s =
+            ConstantInt::get(op1->getType(), APInt::getMaxValue(bw));
+          Value *int_max =
+            builder.CreateZExt(int_max_s, IntegerType::get(M.getContext(), bw2));
+
+          if (ii->getIntrinsicID() == Intrinsic::uadd_with_overflow){
+            result_ext = builder.CreateAdd(op1ext, op2ext);
+          } else if (ii->getIntrinsicID() == Intrinsic::usub_with_overflow){
+            result_ext = builder.CreateSub(op1ext, op2ext);
+          } else if (ii->getIntrinsicID() == Intrinsic::umul_with_overflow){
+            result_ext = builder.CreateMul(op1ext, op2ext);
+          }
+          overflow = builder.CreateICmpUGT(result_ext, int_max);
+
+        } else if ((ii->getIntrinsicID() == Intrinsic::sadd_with_overflow) ||
+                   (ii->getIntrinsicID() == Intrinsic::ssub_with_overflow) ||
+                   (ii->getIntrinsicID() == Intrinsic::smul_with_overflow)) {
+
+          Value *op1ext =
+            builder.CreateSExt(op1, IntegerType::get(M.getContext(), bw2));
+          Value *op2ext =
+            builder.CreateSExt(op2, IntegerType::get(M.getContext(), bw2));
+          Value *int_max_s =
+            ConstantInt::get(op1->getType(), APInt::getSignedMaxValue(bw));
+          Value *int_min_s =
+            ConstantInt::get(op1->getType(), APInt::getSignedMinValue(bw));
+          Value *int_max =
+            builder.CreateSExt(int_max_s, IntegerType::get(M.getContext(), bw2));
+          Value *int_min =
+            builder.CreateSExt(int_min_s, IntegerType::get(M.getContext(), bw2));
+
+          if (ii->getIntrinsicID() == Intrinsic::sadd_with_overflow){
+            result_ext = builder.CreateAdd(op1ext, op2ext);
+          } else if (ii->getIntrinsicID() == Intrinsic::ssub_with_overflow){
+            result_ext = builder.CreateSub(op1ext, op2ext);
+          } else if (ii->getIntrinsicID() == Intrinsic::smul_with_overflow){
+            result_ext = builder.CreateMul(op1ext, op2ext);
+          }
+          overflow = builder.CreateOr(builder.CreateICmpSGT(result_ext, int_max),
+                                      builder.CreateICmpSLT(result_ext, int_min));
+        }
 
-        Value *overflow = builder.CreateICmpULT(result, op1);
-        
+        // This trunc cound be replaced by a more general trunc replacement
+        // that allows to detect also undefined behavior in assignments or
+        // overflow in operation with integers whose dimension is smaller than
+        // int's dimension, e.g.
+        //     uint8_t = uint8_t + uint8_t;
+        // if one desires the wrapping should write
+        //     uint8_t = (uint8_t + uint8_t) & 0xFF;
+        // before this, must check if it has side effects on other operations
+        result = builder.CreateTrunc(result_ext, op1->getType());
         Value *resultStruct =
           builder.CreateInsertValue(UndefValue::get(ii->getType()), result, 0);
         resultStruct = builder.CreateInsertValue(resultStruct, overflow, 1);
diff --git a/test/Feature/ubsan_signed_overflow.c b/test/Feature/ubsan_signed_overflow.c
new file mode 100644
index 00000000..9816d496
--- /dev/null
+++ b/test/Feature/ubsan_signed_overflow.c
@@ -0,0 +1,29 @@
+// RUN: %llvmgcc %s -fsanitize=signed-integer-overflow -emit-llvm -g -O0 -c -o %t.bc
+// RUN: rm -rf %t.klee-out
+// RUN: %klee --output-dir=%t.klee-out %t.bc 2>&1 | FileCheck %s
+
+// llvm-gcc 2.9 does not support -fsanitize=signed-integer-overflow
+// REQUIRES: not-llvm-2.9
+
+#include "klee/klee.h"
+
+int main()
+{
+  signed int x;
+  signed int y;
+  volatile signed int result;
+
+  klee_make_symbolic(&x, sizeof(x), "x");
+  klee_make_symbolic(&y, sizeof(y), "y");
+
+  // CHECK: ubsan_signed_overflow.c:20: overflow on unsigned addition
+  result = x + y;
+
+  // CHECK: ubsan_signed_overflow.c:23: overflow on unsigned subtraction
+  result = x - y;
+
+  // CHECK: ubsan_signed_overflow.c:26: overflow on unsigned multiplication
+  result = x * y;
+
+  return 0;
+}
diff --git a/test/Feature/ubsan_unsigned_overflow.c b/test/Feature/ubsan_unsigned_overflow.c
new file mode 100644
index 00000000..82eacdd7
--- /dev/null
+++ b/test/Feature/ubsan_unsigned_overflow.c
@@ -0,0 +1,29 @@
+// RUN: %llvmgcc %s -fsanitize=unsigned-integer-overflow -emit-llvm -g -O0 -c -o %t.bc
+// RUN: rm -rf %t.klee-out
+// RUN: %klee --output-dir=%t.klee-out %t.bc 2>&1 | FileCheck %s
+
+// llvm-gcc 2.9 does not support -fsanitize=unsigned-integer-overflow
+// REQUIRES: not-llvm-2.9
+
+#include "klee/klee.h"
+
+int main()
+{
+  unsigned int x;
+  unsigned int y;
+  volatile unsigned int result;
+
+  klee_make_symbolic(&x, sizeof(x), "x");
+  klee_make_symbolic(&y, sizeof(y), "y");
+
+  // CHECK: ubsan_unsigned_overflow.c:20: overflow on unsigned addition
+  result = x + y;
+
+  // CHECK: ubsan_unsigned_overflow.c:23: overflow on unsigned subtraction
+  result = x - y;
+
+  // CHECK: ubsan_unsigned_overflow.c:26: overflow on unsigned multiplication
+  result = x * y;
+
+  return 0;
+}
diff --git a/tools/klee/main.cpp b/tools/klee/main.cpp
index 23c07f03..8a246685 100644
--- a/tools/klee/main.cpp
+++ b/tools/klee/main.cpp
@@ -765,6 +765,10 @@ static const char *modelledExternals[] = {
   "_Znwj", 
   "_Znam", 
   "_Znwm", 
+  "__ubsan_handle_add_overflow",
+  "__ubsan_handle_sub_overflow",
+  "__ubsan_handle_mul_overflow",
+  "__ubsan_handle_divrem_overflow",
 };
 // Symbols we aren't going to warn about
 static const char *dontCareExternals[] = {