vsk updated this revision to Diff 90975.
vsk added a comment.
- Fix an incorrect comment on the field "e2" in struct S. We emit a check for
it because 0b11 = -1 = 3.
- Skip the range check on the field "e2" in struct S2, because the range of the
bitfield is the same as the range clang infers for the enum.
https://reviews.llvm.org/D30729
Files:
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCXX/ubsan-bitfields.cpp
Index: test/CodeGenCXX/ubsan-bitfields.cpp
===================================================================
--- test/CodeGenCXX/ubsan-bitfields.cpp
+++ test/CodeGenCXX/ubsan-bitfields.cpp
@@ -7,7 +7,9 @@
};
struct S {
- E e1 : 10;
+ E e1 : 10; //< Wide enough for the unsigned range check.
+ E e2 : 2; //< Emit the check, since -1 = 3 = 0b11.
+ E e3 : 1; //< Skip the check.
};
// CHECK-LABEL: define i32 @_Z4loadP1S
@@ -19,3 +21,58 @@
// CHECK: call void @__ubsan_handle_load_invalid_value
return s->e1;
}
+
+// CHECK-LABEL: define i32 @_Z5load2P1S
+E load2(S *s) {
+ // CHECK: [[LOAD:%.*]] = load i16, i16* {{.*}}
+ // CHECK: [[LSHR:%.*]] = lshr i16 [[LOAD]], 10
+ // CHECK: [[CLEAR:%.*]] = and i16 [[LSHR]], 3
+ // CHECK: [[CAST:%.*]] = zext i16 [[CLEAR]] to i32
+ // CHECK: icmp ule i32 [[CAST]], 3, !nosanitize
+ // CHECK: call void @__ubsan_handle_load_invalid_value
+ return s->e2;
+}
+
+// CHECK-LABEL: define i32 @_Z5load3P1S
+E load3(S *s) {
+ // CHECK-NOT: !nosanitize
+ return s->e3;
+}
+
+enum E2 {
+ x = -3,
+ y = 3
+};
+
+struct S2 {
+ E2 e1 : 4; //< Wide enough for signed range checks.
+ E2 e2 : 3; //< Skip the check, since the range of the bitfield is the same
+ // as the range of the enum: [-4, 3].
+ E2 e3 : 2; //< Still skip the check.
+};
+
+// CHECK-LABEL: define i32 @_Z5load4P2S2
+E2 load4(S2 *s) {
+ // CHECK: [[LOAD:%.*]] = load i16, i16* {{.*}}
+ // CHECK: [[SHL:%.*]] = shl i16 [[LOAD]], 12
+ // CHECK: [[ASHR:%.*]] = ashr i16 [[SHL]], 12
+ // CHECK: [[CAST:%.*]] = sext i16 [[ASHR]] to i32
+ // CHECK: [[UPPER_BOUND:%.*]] = icmp sle i32 [[CAST]], 3, !nosanitize
+ // CHECK: [[LOWER_BOUND:%.*]] = icmp sge i32 [[CAST]], -4, !nosanitize
+ // CHECK: [[BOUND:%.*]] = and i1 [[UPPER_BOUND]], [[LOWER_BOUND]], !nosanitize
+ // CHECK: br i1 [[BOUND]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_load_invalid_value
+ return s->e1;
+}
+
+// CHECK-LABEL: define i32 @_Z5load5P2S2
+E2 load5(S2 *s) {
+ // CHECK-NOT: !nosanitize
+ return s->e2;
+}
+
+// CHECK-LABEL: define i32 @_Z5load6P2S2
+E2 load6(S2 *s) {
+ // CHECK-NOT: !nosanitize
+ return s->e3;
+}
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2892,11 +2892,12 @@
llvm::Value *EmitFromMemory(llvm::Value *Value, QualType Ty);
/// Check if the scalar \p Value is within the valid range for the given
- /// type \p Ty.
+ /// type \p Ty. If \p BitWidth is provided, it must be the bit width of the
+ /// storage for the scalar.
///
- /// Returns true if a check is needed (even if the range is unknown).
- bool EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
- SourceLocation Loc);
+ /// Returns true if range metadata for the scalar should be dropped.
+ bool EmitScalarRangeCheck(llvm::Value *Value, QualType Ty, SourceLocation Loc,
+ Optional<unsigned> BitWidth = None);
/// EmitLoadOfScalar - Load a scalar value from an address, taking
/// care to appropriately convert from the memory representation to
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1302,7 +1302,8 @@
}
bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
- SourceLocation Loc) {
+ SourceLocation Loc,
+ Optional<unsigned> BitWidth) {
bool HasBoolCheck = SanOpts.has(SanitizerKind::Bool);
bool HasEnumCheck = SanOpts.has(SanitizerKind::Enum);
if (!HasBoolCheck && !HasEnumCheck)
@@ -1319,9 +1320,32 @@
if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool))
return true;
+ --End;
+ if (BitWidth) {
+ if (!Min) {
+ // If End > MaxValueForWidth, then Value < End. Skip the range check.
+ auto MaxValueForWidth =
+ llvm::APInt::getMaxValue(*BitWidth).zextOrSelf(End.getBitWidth());
+ if (End.ugt(MaxValueForWidth))
+ return false;
+ } else {
+ assert(Min.eq(-(End + 1)) &&
+ "The full range check assumes Min = -(End + 1)");
+ assert(Ty->hasSignedIntegerRepresentation() &&
+ "The full range check works on signed integers only");
+
+ // If Min <= MinValueForWidth, then Value >= Min. Since Min = -(End + 1),
+ // End >= MaxValueForWidth, and Value <= End. Skip the range check.
+ auto MinValueForWidth =
+ llvm::APInt::getSignedMinValue(*BitWidth).sextOrSelf(
+ End.getBitWidth());
+ if (Min.sle(MinValueForWidth))
+ return false;
+ }
+ }
+
SanitizerScope SanScope(this);
llvm::Value *Check;
- --End;
if (!Min) {
Check = Builder.CreateICmpULE(
Value, llvm::ConstantInt::get(getLLVMContext(), End));
@@ -1566,7 +1590,7 @@
"bf.clear");
}
Val = Builder.CreateIntCast(Val, ResLTy, Info.IsSigned, "bf.cast");
- EmitScalarRangeCheck(Val, LV.getType(), Loc);
+ EmitScalarRangeCheck(Val, LV.getType(), Loc, Info.Size);
return RValue::get(Val);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
