r293343 - [ubsan] Sanity-check shift amounts before truncation (fixes PR27271)
Filipe Cabecinhas via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 30 04:15:52 PST 2017
Another example + possible fix (two WidthMinusOne, one per (possibly
different) bitwidth): (not fully tested)
int f() {
return 0 << 0L;
}
diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index 40d949dece..5c9055d49a 100644
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -2751,8 +2751,9 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
isa<llvm::IntegerType>(Ops.LHS->getType())) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
SmallVector<std::pair<Value *, SanitizerMask>, 2> Checks;
- llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, Ops.RHS);
- llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS, WidthMinusOne);
+ llvm::Value *WidthMinusOneRHS = GetWidthMinusOneValue(Ops.LHS, Ops.RHS);
+ llvm::Value *WidthMinusOneLHS = GetWidthMinusOneValue(Ops.LHS, Ops.LHS);
+ llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS,
WidthMinusOneRHS);
if (SanitizeExponent) {
Checks.push_back(
@@ -2768,11 +2769,11 @@ Value *ScalarExprEmitter::EmitShl(const
BinOpInfo &Ops) {
llvm::BasicBlock *CheckShiftBase = CGF.createBasicBlock("check");
Builder.CreateCondBr(ValidExponent, CheckShiftBase, Cont);
CGF.EmitBlock(CheckShiftBase);
- llvm::Value *BitsShiftedOff =
- Builder.CreateLShr(Ops.LHS,
- Builder.CreateSub(WidthMinusOne, RHS, "shl.zeros",
- /*NUW*/true, /*NSW*/true),
- "shl.check");
+ llvm::Value *BitsShiftedOff = Builder.CreateLShr(
+ Ops.LHS,
+ Builder.CreateSub(WidthMinusOneLHS, RHS, "shl.zeros",
+ /*NUW*/ true, /*NSW*/ true),
+ "shl.check");
if (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
F
On Mon, Jan 30, 2017 at 11:51 AM, Alex L via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Hi Vedant,
>
> This commit has caused a compiler crash in our stage 2 green dragon
> ASAN+Ubsan bot
> (http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_build/). I have
> reverted it in r293475. The following example reproduces the crash with
> -fsanitize=undefined :
>
> typedef unsigned long long uint64_t;
> typedef signed long long int64_t;
>
> uint64_t foo(int64_t x, unsigned i) {
> return x << i;
> }
>
> Alex
>
>
> On 27 January 2017 at 23:02, Vedant Kumar via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: vedantk
>> Date: Fri Jan 27 17:02:44 2017
>> New Revision: 293343
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=293343&view=rev
>> Log:
>> [ubsan] Sanity-check shift amounts before truncation (fixes PR27271)
>>
>> Ubsan does not report UB shifts in some cases where the shift exponent
>> needs to be truncated to match the type of the shift base. We perform a
>> range check on the truncated shift amount, leading to false negatives.
>>
>> Fix the issue (PR27271) by performing the range check on the original
>> shift amount.
>>
>> Differential Revision: https://reviews.llvm.org/D29234
>>
>> Added:
>> cfe/trunk/test/CodeGen/ubsan-shift.c
>> Modified:
>> cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=293343&r1=293342&r2=293343&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Jan 27 17:02:44 2017
>> @@ -2751,8 +2751,8 @@ Value *ScalarExprEmitter::EmitShl(const
>> isa<llvm::IntegerType>(Ops.LHS->getType())) {
>> CodeGenFunction::SanitizerScope SanScope(&CGF);
>> SmallVector<std::pair<Value *, SanitizerMask>, 2> Checks;
>> - llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS);
>> - llvm::Value *ValidExponent = Builder.CreateICmpULE(RHS,
>> WidthMinusOne);
>> + llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, Ops.RHS);
>> + llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS,
>> WidthMinusOne);
>>
>> if (SanitizeExponent) {
>> Checks.push_back(
>>
>> Added: cfe/trunk/test/CodeGen/ubsan-shift.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-shift.c?rev=293343&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/ubsan-shift.c (added)
>> +++ cfe/trunk/test/CodeGen/ubsan-shift.c Fri Jan 27 17:02:44 2017
>> @@ -0,0 +1,29 @@
>> +// RUN: %clang_cc1 -triple=x86_64-apple-darwin -fsanitize=shift-exponent
>> -emit-llvm %s -o - | FileCheck %s
>> +
>> +// CHECK-LABEL: define i32 @f1
>> +int f1(int c, int shamt) {
>> +// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
>> +// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
>> + return 1 << (c << shamt);
>> +}
>> +
>> +// CHECK-LABEL: define i32 @f2
>> +int f2(long c, int shamt) {
>> +// CHECK: icmp ule i32 %{{.*}}, 63, !nosanitize
>> +// CHECK: icmp ule i64 %{{.*}}, 31, !nosanitize
>> + return 1 << (c << shamt);
>> +}
>> +
>> +// CHECK-LABEL: define i32 @f3
>> +unsigned f3(unsigned c, int shamt) {
>> +// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
>> +// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
>> + return 1U << (c << shamt);
>> +}
>> +
>> +// CHECK-LABEL: define i32 @f4
>> +unsigned f4(unsigned long c, int shamt) {
>> +// CHECK: icmp ule i32 %{{.*}}, 63, !nosanitize
>> +// CHECK: icmp ule i64 %{{.*}}, 31, !nosanitize
>> + return 1U << (c << shamt);
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list