[compiler-rt] 82d29b3 - Add an unsigned shift base sanitizer
JF Bastien via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 20:06:43 PDT 2020
Author: JF Bastien
Date: 2020-08-27T19:50:10-07:00
New Revision: 82d29b397bb24c6a9e5c41401278886f4614e544
URL: https://github.com/llvm/llvm-project/commit/82d29b397bb24c6a9e5c41401278886f4614e544
DIFF: https://github.com/llvm/llvm-project/commit/82d29b397bb24c6a9e5c41401278886f4614e544.diff
LOG: Add an unsigned shift base sanitizer
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 as part of -fsanitize=integer.
The flag is named: -fsanitize=unsigned-shift-base
This matches shift-base and shift-exponent flags.
<rdar://problem/46129047>
Differential Revision: https://reviews.llvm.org/D86000
Added:
clang/test/CodeGen/unsigned-shift-base.c
compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
Modified:
clang/docs/UndefinedBehaviorSanitizer.rst
clang/include/clang/Basic/Sanitizers.def
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Driver/ToolChain.cpp
clang/test/Driver/fsanitize.c
llvm/docs/ReleaseNotes.rst
Removed:
################################################################################
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index 76676dfce95b..3345536bf821 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -153,6 +153,8 @@ Available checks are:
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
index 2912bdd44b2d..9b8936cc520c 100644
--- a/clang/include/clang/Basic/Sanitizers.def
+++ b/clang/include/clang/Basic/Sanitizers.def
@@ -107,6 +107,7 @@ SANITIZER("vptr", Vptr)
// IntegerSanitizer
SANITIZER("unsigned-integer-overflow", UnsignedIntegerOverflow)
+SANITIZER("unsigned-shift-base", UnsignedShiftBase)
// DataFlowSanitizer
SANITIZER("dataflow", DataFlow)
@@ -171,7 +172,8 @@ SANITIZER_GROUP("implicit-conversion", ImplicitConversion,
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
index 511c6a66d555..d950ff1aec5a 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3720,10 +3720,14 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
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)
@@ -3756,11 +3760,12 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
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);
}
@@ -3770,7 +3775,9 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
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
index 7be83cade93a..f04b10ef30c9 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1016,14 +1016,14 @@ SanitizerMask ToolChain::getSupportedSanitizers() const {
// 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
index 000000000000..2260005512f0
--- /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
index cfefd3fb632c..bad519fcef24 100644
--- 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
index 000000000000..b49504e393ac
--- /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
index 5cd2386c9207..d30cca544019 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -157,6 +157,14 @@ Changes to the LLVM tools
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.
+
External Open Source Projects Using LLVM 12
===========================================
More information about the llvm-commits
mailing list