diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst --- a/clang/docs/UndefinedBehaviorSanitizer.rst +++ b/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 diff --git a/clang/include/clang/Basic/Sanitizers.def b/clang/include/clang/Basic/Sanitizers.def --- a/clang/include/clang/Basic/Sanitizers.def +++ b/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) @@ -171,7 +172,8 @@ SANITIZER_GROUP("integer", Integer, ImplicitConversion | IntegerDivideByZero | Shift | - SignedIntegerOverflow | UnsignedIntegerOverflow) + SignedIntegerOverflow | UnsignedIntegerOverflow | + UnsignedShiftBase) SANITIZER("local-bounds", LocalBounds) SANITIZER_GROUP("bounds", Bounds, ArrayBounds | LocalBounds) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/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()); diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp --- a/clang/lib/Driver/ToolChain.cpp +++ b/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() || diff --git a/clang/test/CodeGen/unsigned-shift-base.c b/clang/test/CodeGen/unsigned-shift-base.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c --- a/clang/test/Driver/fsanitize.c +++ b/clang/test/Driver/fsanitize.c @@ -32,7 +32,7 @@ // CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib" // RUN: %clang -target %itanium_abi_triple -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope" -// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-unsigned-integer-truncation|implicit-signed-integer-truncation|implicit-integer-sign-change),?){8}"}} +// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-unsigned-integer-truncation|implicit-signed-integer-truncation|implicit-integer-sign-change|unsigned-shift-base),?){9}"}} // RUN: %clang -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER // RUN: %clang -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER diff --git a/compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp b/compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp new file mode 100644 --- /dev/null +++ b/compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp @@ -0,0 +1,54 @@ +// 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' + + return 1; +} diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -140,6 +140,18 @@ Changes to LLDB --------------------------------- +Changes to Sanitizers +--------------------- + +The integer sanitizer `-fsanitize=integer` now has a new sanitizer: +`-fsanitize=unsigned-shift-base`. 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. + +One can silence the check with masking, for example: + + `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs` + External Open Source Projects Using LLVM 12 ===========================================