jfb created this revision.
jfb added a reviewer: vsk.
Herald added subscribers: Sanitizers, cfe-commits, ributzka, dexonsmith,
jkorous.
Herald added projects: clang, Sanitizers.
jfb requested review of this revision.
It's not undefined behavior for an unsigned left shift to overflow (i.e. to
shift bits out), but it has been the source of bugs and exploits in certain
codebases in the past. As we do in other parts of UBSan, this patch adds a
dynamic checker which acts beyond UBSan and checks other sources of errors. The
option is enabled completely separately from other checkers since it's unlikely
that folks who have currently adopted other checkers will want this one.
The flag is named: -fsanitize=unsigned-shift-base
This matches shift-base and shift-exponent flags.
rdar://problem/46129047
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86000
Files:
clang/docs/UndefinedBehaviorSanitizer.rst
clang/include/clang/Basic/Sanitizers.def
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChain.cpp
clang/test/CodeGen/unsigned-shift-base.c
clang/test/Driver/fsanitize.c
compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
Index: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
===================================================================
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
@@ -0,0 +1,52 @@
+// RUN: %clangxx -fsanitize=unsigned-shift-base %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+// RUN: %clangxx -fsanitize=unsigned-shift-base,shift-exponent %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+
+#define shift(val, amount) ({ \
+ volatile unsigned _v = (val); \
+ volatile unsigned _a = (amount); \
+ unsigned res = _v << _a; \
+ res; \
+})
+
+int main() {
+
+ shift(0b00000000'00000000'00000000'00000000, 31);
+ shift(0b00000000'00000000'00000000'00000001, 31);
+ shift(0b00000000'00000000'00000000'00000010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000000'00000100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000000'00001000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000000'00010000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000000'00100000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000000'01000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 64 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000000'10000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 128 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000001'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 256 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000010'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 512 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00000100'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1024 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00001000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2048 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00010000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4096 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'00100000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8192 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'01000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16384 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000000'10000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32768 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000001'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 65536 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000010'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 131072 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00000100'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 262144 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00001000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 524288 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00010000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1048576 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'00100000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2097152 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'01000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4194304 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000000'10000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8388608 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000001'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16777216 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000010'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 33554432 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00000100'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 67108864 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00001000'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 134217728 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00010000'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 268435456 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b00100000'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 536870912 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b01000000'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1073741824 by 31 places cannot be represented in type 'unsigned int'
+ shift(0b10000000'00000000'00000000'00000000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2147483648 by 31 places cannot be represented in type 'unsigned int'
+
+ shift(0b10000000'00000000'00000000'00000000, 00);
+ shift(0b10000000'00000000'00000000'00000000, 01); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2147483648 by 1 places cannot be represented in type 'unsigned int'
+
+ shift(0xffff'ffff, 0);
+ shift(0xffff'ffff, 1); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4294967295 by 1 places cannot be represented in type 'unsigned int'
+}
Index: clang/test/Driver/fsanitize.c
===================================================================
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -899,3 +899,18 @@
// RUN: %clang -fsanitize=undefined,float-divide-by-zero %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DIVBYZERO-UBSAN
// CHECK-DIVBYZERO-UBSAN: "-fsanitize={{.*}},float-divide-by-zero,{{.*}}"
+
+// RUN: %clang -fsanitize=unsigned-shift-base %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-unsigned-shift-base,CHECK-unsigned-shift-base-RECOVER
+// RUN: %clang -fsanitize=unsigned-shift-base -fsanitize-recover=unsigned-shift-base %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-unsigned-shift-base,CHECK-unsigned-shift-base-RECOVER
+// RUN: %clang -fsanitize=unsigned-shift-base -fno-sanitize-recover=unsigned-shift-base %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-unsigned-shift-base,CHECK-unsigned-shift-base-NORECOVER
+// RUN: %clang -fsanitize=unsigned-shift-base -fsanitize-trap=unsigned-shift-base %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-unsigned-shift-base,CHECK-unsigned-shift-base-TRAP
+// CHECK-unsigned-shift-base: "-fsanitize=unsigned-shift-base"
+// CHECK-unsigned-shift-base-RECOVER: "-fsanitize-recover=unsigned-shift-base"
+// CHECK-unsigned-shift-base-RECOVER-NOT: "-fno-sanitize-recover=unsigned-shift-base"
+// CHECK-unsigned-shift-base-RECOVER-NOT: "-fsanitize-trap=unsigned-shift-base"
+// CHECK-unsigned-shift-base-NORECOVER-NOT: "-fno-sanitize-recover=unsigned-shift-base"
+// CHECK-unsigned-shift-base-NORECOVER-NOT: "-fsanitize-recover=unsigned-shift-base"
+// CHECK-unsigned-shift-base-NORECOVER-NOT: "-fsanitize-trap=unsigned-shift-base"
+// CHECK-unsigned-shift-base-TRAP: "-fsanitize-trap=unsigned-shift-base"
+// CHECK-unsigned-shift-base-TRAP-NOT: "-fsanitize-recover=unsigned-shift-base"
+// CHECK-unsigned-shift-base-TRAP-NOT: "-fno-sanitize-recover=unsigned-shift-base"
Index: clang/test/CodeGen/unsigned-shift-base.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/unsigned-shift-base.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsanitize=unsigned-shift-base,shift-exponent %s -emit-llvm -o - | FileCheck %s
+
+// CHECK-LABEL: lsh_overflow(
+unsigned lsh_overflow(unsigned a, unsigned b) {
+ // CHECK: %[[RHS_INBOUNDS:.*]] = icmp ule i32 %[[RHS:.*]], 31
+ // CHECK-NEXT: br i1 %[[RHS_INBOUNDS]], label %[[CHECK_BB:.*]], label %[[CONT_BB:.*]],
+
+ // CHECK: [[CHECK_BB]]:
+ // CHECK-NEXT: %[[SHIFTED_OUT_WIDTH:.*]] = sub nuw nsw i32 31, %[[RHS]]
+ // CHECK-NEXT: %[[SHIFTED_OUT:.*]] = lshr i32 %[[LHS:.*]], %[[SHIFTED_OUT_WIDTH]]
+
+ // CHECK-NEXT: %[[SHIFTED_OUT_NOT_SIGN:.*]] = lshr i32 %[[SHIFTED_OUT]], 1
+
+ // CHECK-NEXT: %[[NO_OVERFLOW:.*]] = icmp eq i32 %[[SHIFTED_OUT_NOT_SIGN]], 0
+ // CHECK-NEXT: br label %[[CONT_BB]]
+
+ // CHECK: [[CONT_BB]]:
+ // CHECK-NEXT: %[[VALID_BASE:.*]] = phi i1 [ true, {{.*}} ], [ %[[NO_OVERFLOW]], %[[CHECK_BB]] ]
+ // CHECK-NEXT: %[[VALID:.*]] = and i1 %[[RHS_INBOUNDS]], %[[VALID_BASE]]
+ // CHECK-NEXT: br i1 %[[VALID]]
+
+ // CHECK: call void @__ubsan_handle_shift_out_of_bounds
+ // CHECK-NOT: call void @__ubsan_handle_shift_out_of_bounds
+
+ // CHECK: %[[RET:.*]] = shl i32 %[[LHS]], %[[RHS]]
+ // CHECK-NEXT: ret i32 %[[RET]]
+ return a << b;
+}
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1016,14 +1016,14 @@
// Return sanitizers which don't require runtime support and are not
// platform dependent.
- SanitizerMask Res = (SanitizerKind::Undefined & ~SanitizerKind::Vptr &
- ~SanitizerKind::Function) |
- (SanitizerKind::CFI & ~SanitizerKind::CFIICall) |
- SanitizerKind::CFICastStrict |
- SanitizerKind::FloatDivideByZero |
- SanitizerKind::UnsignedIntegerOverflow |
- SanitizerKind::ImplicitConversion |
- SanitizerKind::Nullability | SanitizerKind::LocalBounds;
+ SanitizerMask Res =
+ (SanitizerKind::Undefined & ~SanitizerKind::Vptr &
+ ~SanitizerKind::Function) |
+ (SanitizerKind::CFI & ~SanitizerKind::CFIICall) |
+ SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero |
+ SanitizerKind::UnsignedIntegerOverflow |
+ SanitizerKind::UnsignedShiftBase | SanitizerKind::ImplicitConversion |
+ SanitizerKind::Nullability | SanitizerKind::LocalBounds;
if (getTriple().getArch() == llvm::Triple::x86 ||
getTriple().getArch() == llvm::Triple::x86_64 ||
getTriple().getArch() == llvm::Triple::arm || getTriple().isWasm() ||
Index: clang/lib/Driver/SanitizerArgs.cpp
===================================================================
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -26,9 +26,9 @@
static const SanitizerMask NeedsUbsanRt =
SanitizerKind::Undefined | SanitizerKind::Integer |
- SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
- SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
- SanitizerKind::ObjCCast;
+ SanitizerKind::UnsignedShiftBase | SanitizerKind::ImplicitConversion |
+ SanitizerKind::Nullability | SanitizerKind::CFI |
+ SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
static const SanitizerMask NeedsUbsanCxxRt =
SanitizerKind::Vptr | SanitizerKind::CFI;
static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
@@ -44,7 +44,8 @@
SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress |
SanitizerKind::MemTag | SanitizerKind::Memory |
SanitizerKind::KernelMemory | SanitizerKind::Leak |
- SanitizerKind::Undefined | SanitizerKind::Integer | SanitizerKind::Bounds |
+ SanitizerKind::Undefined | SanitizerKind::Integer |
+ SanitizerKind::UnsignedShiftBase | SanitizerKind::Bounds |
SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
@@ -52,8 +53,9 @@
SanitizerKind::Thread | SanitizerKind::ObjCCast;
static const SanitizerMask RecoverableByDefault =
SanitizerKind::Undefined | SanitizerKind::Integer |
- SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
- SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
+ SanitizerKind::UnsignedShiftBase | SanitizerKind::ImplicitConversion |
+ SanitizerKind::Nullability | SanitizerKind::FloatDivideByZero |
+ SanitizerKind::ObjCCast;
static const SanitizerMask Unrecoverable =
SanitizerKind::Unreachable | SanitizerKind::Return;
static const SanitizerMask AlwaysRecoverable =
@@ -61,10 +63,10 @@
static const SanitizerMask NeedsLTO = SanitizerKind::CFI;
static const SanitizerMask TrappingSupported =
(SanitizerKind::Undefined & ~SanitizerKind::Vptr) |
- SanitizerKind::UnsignedIntegerOverflow | SanitizerKind::ImplicitConversion |
- SanitizerKind::Nullability | SanitizerKind::LocalBounds |
- SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
- SanitizerKind::ObjCCast;
+ SanitizerKind::UnsignedIntegerOverflow | SanitizerKind::UnsignedShiftBase |
+ SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+ SanitizerKind::LocalBounds | SanitizerKind::CFI |
+ SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
static const SanitizerMask CFIClasses =
SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
@@ -148,6 +150,7 @@
{"cfi_blacklist.txt", SanitizerKind::CFI},
{"ubsan_blacklist.txt",
SanitizerKind::Undefined | SanitizerKind::Integer |
+ SanitizerKind::UnsignedShiftBase |
SanitizerKind::Nullability |
SanitizerKind::FloatDivideByZero}};
Index: clang/lib/CodeGen/CGExprScalar.cpp
===================================================================
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3833,10 +3833,14 @@
if (Ops.LHS->getType() != RHS->getType())
RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
- bool SanitizeBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
- Ops.Ty->hasSignedIntegerRepresentation() &&
- !CGF.getLangOpts().isSignedOverflowDefined() &&
- !CGF.getLangOpts().CPlusPlus20;
+ bool SanitizeSignedBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
+ Ops.Ty->hasSignedIntegerRepresentation() &&
+ !CGF.getLangOpts().isSignedOverflowDefined() &&
+ !CGF.getLangOpts().CPlusPlus20;
+ bool SanitizeUnsignedBase =
+ CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) &&
+ Ops.Ty->hasUnsignedIntegerRepresentation();
+ bool SanitizeBase = SanitizeSignedBase || SanitizeUnsignedBase;
bool SanitizeExponent = CGF.SanOpts.has(SanitizerKind::ShiftExponent);
// OpenCL 6.3j: shift values are effectively % word size of LHS.
if (CGF.getLangOpts().OpenCL)
@@ -3869,11 +3873,12 @@
Ops.LHS, Builder.CreateSub(PromotedWidthMinusOne, RHS, "shl.zeros",
/*NUW*/ true, /*NSW*/ true),
"shl.check");
- if (CGF.getLangOpts().CPlusPlus) {
+ if (SanitizeUnsignedBase || CGF.getLangOpts().CPlusPlus) {
// In C99, we are not permitted to shift a 1 bit into the sign bit.
// Under C++11's rules, shifting a 1 bit into the sign bit is
// OK, but shifting a 1 bit out of it is not. (C89 and C++03 don't
// define signed left shifts, so we use the C99 and C++11 rules there).
+ // Unsigned shifts can always shift into the top bit.
llvm::Value *One = llvm::ConstantInt::get(BitsShiftedOff->getType(), 1);
BitsShiftedOff = Builder.CreateLShr(BitsShiftedOff, One);
}
@@ -3883,7 +3888,9 @@
llvm::PHINode *BaseCheck = Builder.CreatePHI(ValidBase->getType(), 2);
BaseCheck->addIncoming(Builder.getTrue(), Orig);
BaseCheck->addIncoming(ValidBase, CheckShiftBase);
- Checks.push_back(std::make_pair(BaseCheck, SanitizerKind::ShiftBase));
+ Checks.push_back(std::make_pair(
+ BaseCheck, SanitizeSignedBase ? SanitizerKind::ShiftBase
+ : SanitizerKind::UnsignedShiftBase));
}
assert(!Checks.empty());
Index: clang/include/clang/Basic/Sanitizers.def
===================================================================
--- clang/include/clang/Basic/Sanitizers.def
+++ clang/include/clang/Basic/Sanitizers.def
@@ -107,6 +107,7 @@
// IntegerSanitizer
SANITIZER("unsigned-integer-overflow", UnsignedIntegerOverflow)
+SANITIZER("unsigned-shift-base", UnsignedShiftBase)
// DataFlowSanitizer
SANITIZER("dataflow", DataFlow)
Index: clang/docs/UndefinedBehaviorSanitizer.rst
===================================================================
--- clang/docs/UndefinedBehaviorSanitizer.rst
+++ clang/docs/UndefinedBehaviorSanitizer.rst
@@ -153,6 +153,8 @@
unsigned overflow in C++. You can use ``-fsanitize=shift-base`` or
``-fsanitize=shift-exponent`` to check only left-hand side or
right-hand side of shift operation, respectively.
+ - ``-fsanitize=unsigned-shift-base``: check that an unsigned left-hand side of
+ a left shift operation doesn't overflow.
- ``-fsanitize=signed-integer-overflow``: Signed integer overflow, where the
result of a signed integer computation cannot be represented in its type.
This includes all the checks covered by ``-ftrapv``, as well as checks for
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
