vsk created this revision.
Teach ubsan to diagnose remainder operations which have undefined behavior due
to signed overflow.
My copy of the C11 spec draft (6.5.5.6) says that: if the quotient a/b is
representable, (a/b)*b + a%b shall equal a; otherwise, the behavior of both a/b
and a%b is undefined. I take this to mean INT_MIN % -1 has signed overflow UB,
so we should check for that.
Loosely depends on: https://reviews.llvm.org/D29369
https://reviews.llvm.org/D29437
Files:
lib/CodeGen/CGExprScalar.cpp
test/CodeGen/ubsan-promoted-arith.cpp
Index: test/CodeGen/ubsan-promoted-arith.cpp
===================================================================
--- test/CodeGen/ubsan-promoted-arith.cpp
+++ test/CodeGen/ubsan-promoted-arith.cpp
@@ -101,12 +101,14 @@
// CHECK-NOT: ubsan_handle_divrem_overflow
uchar rem2(uchar uc) { return uc % uc; }
-// FIXME: This is a long-standing false negative.
-//
// CHECK-LABEL: define signext i8 @_Z4rem3
-// rdar30301609: ubsan_handle_divrem_overflow
+// CHECK: ubsan_handle_divrem_overflow
char rem3(int i, char c) { return i % c; }
+// CHECK-LABEL: define signext i8 @_Z4rem4
+// CHECK-NOT: ubsan_handle_divrem_overflow
+char rem4(char c, int i) { return c % i; }
+
// CHECK-LABEL: define signext i8 @_Z4inc1
// CHECK-NOT: sadd.with.overflow
char inc1(char c) { return c++ + (char)0; }
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2375,12 +2375,12 @@
Value *ScalarExprEmitter::EmitRem(const BinOpInfo &Ops) {
// Rem in C can't be a floating point type: C99 6.5.5p2.
- if (CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero)) {
+ if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
+ CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
+ Ops.Ty->isIntegerType()) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
-
- if (Ops.Ty->isIntegerType())
- EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
+ EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
}
if (Ops.Ty->hasUnsignedIntegerRepresentation())
Index: test/CodeGen/ubsan-promoted-arith.cpp
===================================================================
--- test/CodeGen/ubsan-promoted-arith.cpp
+++ test/CodeGen/ubsan-promoted-arith.cpp
@@ -101,12 +101,14 @@
// CHECK-NOT: ubsan_handle_divrem_overflow
uchar rem2(uchar uc) { return uc % uc; }
-// FIXME: This is a long-standing false negative.
-//
// CHECK-LABEL: define signext i8 @_Z4rem3
-// rdar30301609: ubsan_handle_divrem_overflow
+// CHECK: ubsan_handle_divrem_overflow
char rem3(int i, char c) { return i % c; }
+// CHECK-LABEL: define signext i8 @_Z4rem4
+// CHECK-NOT: ubsan_handle_divrem_overflow
+char rem4(char c, int i) { return c % i; }
+
// CHECK-LABEL: define signext i8 @_Z4inc1
// CHECK-NOT: sadd.with.overflow
char inc1(char c) { return c++ + (char)0; }
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2375,12 +2375,12 @@
Value *ScalarExprEmitter::EmitRem(const BinOpInfo &Ops) {
// Rem in C can't be a floating point type: C99 6.5.5p2.
- if (CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero)) {
+ if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
+ CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
+ Ops.Ty->isIntegerType()) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
-
- if (Ops.Ty->isIntegerType())
- EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
+ EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
}
if (Ops.Ty->hasUnsignedIntegerRepresentation())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
