Retry: [ubsan] Detect UB loads from bitfields

It's possible to load out-of-range values from bitfields backed by a
boolean or an enum. Check for UB loads from bitfields.

This is the motivating example:

  struct S {
    BOOL b : 1; // Signed ObjC BOOL.
  };

  S s;
  s.b = 1; // This is actually stored as -1.
  if (s.b == 1) // Evaluates to false, -1 != 1.
    ...

Changes since the original commit:

- Single-bit bools are a special case (see CGF::EmitFromMemory), and we
  can't avoid dealing with them when loading from a bitfield. Don't try to
  insert a check in this case.

Differential Revision: https://reviews.llvm.org/D30423

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@297389 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGAtomic.cpp b/lib/CodeGen/CGAtomic.cpp
index 9287e46127bd5328aeac11bb16af1a7ebc5bb08c..28e20b53d656241ed3f2fe43b85f607fdbfa6ae8 100644 (file)
--- a/lib/CodeGen/CGAtomic.cpp
+++ b/lib/CodeGen/CGAtomic.cpp
@@ -1181,7 +1181,7 @@ RValue AtomicInfo::convertAtomicTempToRValue(Address addr,
   if (LVal.isBitField())
     return CGF.EmitLoadOfBitfieldLValue(
         LValue::MakeBitfield(addr, LVal.getBitFieldInfo(), LVal.getType(),
-                             LVal.getAlignmentSource()));
+                             LVal.getAlignmentSource()), loc);
   if (LVal.isVectorElt())
     return CGF.EmitLoadOfLValue(
         LValue::MakeVectorElt(addr, LVal.getVectorIdx(), LVal.getType(),

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 15cd7ccbaac744f3bdb6ad38834f70eb2eb6f1c2..ab4448bf3c56362219a5719a42f19aa757d5c529 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1327,6 +1327,13 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
   if (!NeedsBoolCheck && !NeedsEnumCheck)
     return false;
 
+  // Single-bit booleans don't need to be checked. Special-case this to avoid
+  // a bit width mismatch when handling bitfield values. This is handled by
+  // EmitFromMemory for the non-bitfield case.
+  if (IsBool &&
+      cast<llvm::IntegerType>(Value->getType())->getBitWidth() == 1)
+    return false;
+
   llvm::APInt Min, End;
   if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool))
     return true;
@@ -1549,10 +1556,11 @@ RValue CodeGenFunction::EmitLoadOfLValue(LValue LV, SourceLocation Loc) {
     return EmitLoadOfGlobalRegLValue(LV);
 
   assert(LV.isBitField() && "Unknown LValue type!");
-  return EmitLoadOfBitfieldLValue(LV);
+  return EmitLoadOfBitfieldLValue(LV, Loc);
 }
 
-RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV) {
+RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
+                                                 SourceLocation Loc) {
   const CGBitFieldInfo &Info = LV.getBitFieldInfo();
 
   // Get the output type.
@@ -1577,7 +1585,7 @@ RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV) {
                               "bf.clear");
   }
   Val = Builder.CreateIntCast(Val, ResLTy, Info.IsSigned, "bf.cast");
-
+  EmitScalarRangeCheck(Val, LV.getType(), Loc);
   return RValue::get(Val);
 }
 

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 6112f26f6d8ea7e36428d587fe4d517297deb9c9..d7b1993d5770108b8373d69382abd283950edae6 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -2943,7 +2943,7 @@ public:
   /// rvalue, returning the rvalue.
   RValue EmitLoadOfLValue(LValue V, SourceLocation Loc);
   RValue EmitLoadOfExtVectorElementLValue(LValue V);
-  RValue EmitLoadOfBitfieldLValue(LValue LV);
+  RValue EmitLoadOfBitfieldLValue(LValue LV, SourceLocation Loc);
   RValue EmitLoadOfGlobalRegLValue(LValue LV);
 
   /// EmitStoreThroughLValue - Store the specified rvalue into the specified

diff --git a/test/CodeGenCXX/ubsan-bitfields.cpp b/test/CodeGenCXX/ubsan-bitfields.cpp
new file mode 100644 (file)
index 0000000..5595db0
--- /dev/null
+++ b/test/CodeGenCXX/ubsan-bitfields.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=bool,enum | FileCheck %s
+
+enum E {
+  a = 1,
+  b = 2,
+  c = 3
+};
+
+struct S {
+  E e1 : 10;
+};
+
+// CHECK-LABEL: define i32 @_Z4loadP1S
+E load(S *s) {
+  // CHECK: [[LOAD:%.*]] = load i16, i16* {{.*}}
+  // CHECK: [[CLEAR:%.*]] = and i16 [[LOAD]], 1023
+  // CHECK: [[CAST:%.*]] = zext i16 [[CLEAR]] to i32
+  // CHECK: icmp ule i32 [[CAST]], 3, !nosanitize
+  // CHECK: call void @__ubsan_handle_load_invalid_value
+  return s->e1;
+}
+
+struct Bool {
+  bool b1 : 1;
+  bool b2 : 7;
+  bool b3 : 16;
+};
+
+// CHECK-LABEL: define zeroext i1 @_Z13load_cpp_boolP4Bool
+bool load_cpp_bool(Bool *b) {
+  // CHECK-NOT: call void @__ubsan_handle_load_invalid_value
+  // CHECK-NOT: !nosanitize
+  return b->b1 || b->b2 || b->b3;
+}

diff --git a/test/CodeGenObjC/ubsan-bool.m b/test/CodeGenObjC/ubsan-bool.m
index 6d6c08358d079eddec27e5814d6c33c9fcba17a5..b30562c4d423354ea7c50e7dd72dc0da0d20c969 100644 (file)
--- a/test/CodeGenObjC/ubsan-bool.m
+++ b/test/CodeGenObjC/ubsan-bool.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - | FileCheck %s -check-prefixes=SHARED,OBJC
-// RUN: %clang_cc1 -x objective-c++ -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - | FileCheck %s -check-prefixes=SHARED,OBJC
+// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - -w | FileCheck %s -check-prefixes=SHARED,OBJC
+// RUN: %clang_cc1 -x objective-c++ -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - -w | FileCheck %s -check-prefixes=SHARED,OBJC
 // RUN: %clang_cc1 -x c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=bool %s -o - | FileCheck %s -check-prefixes=SHARED,C
 
 typedef signed char BOOL;
@@ -10,4 +10,57 @@ BOOL f1() {
   // C-NOT: call void @__ubsan_handle_load_invalid_value
   BOOL a = 2;
   return a + 1;
+  // SHARED: ret i8
 }
+
+struct S1 {
+  BOOL b1 : 1;
+};
+
+// SHARED-LABEL: f2
+BOOL f2(struct S1 *s) {
+  // OBJC: [[LOAD:%.*]] = load i8, i8* {{.*}}
+  // OBJC: [[SHL:%.*]] = shl i8 [[LOAD]], 7
+  // OBJC: [[ASHR:%.*]] = ashr i8 [[SHL]], 7
+  // OBJC: icmp ule i8 [[ASHR]], 1, !nosanitize
+  // OBJC: call void @__ubsan_handle_load_invalid_value
+
+  // C-NOT: call void @__ubsan_handle_load_invalid_value
+  return s->b1;
+  // SHARED: ret i8
+}
+
+#ifdef __OBJC__
+@interface I1 {
+@public
+  BOOL b1 : 1;
+}
+@property (nonatomic) BOOL b1;
+@end
+@implementation I1
+@synthesize b1;
+@end
+
+// Check the synthesized getter.
+// OBJC-LABEL: define internal signext i8 @"\01-[I1 b1]"
+// OBJC: [[IVAR:%.*]] = load i64, i64* @"OBJC_IVAR_$_I1.b1"
+// OBJC: [[ADDR:%.*]] = getelementptr inbounds i8, i8* {{.*}}, i64 [[IVAR]]
+// OBJC: [[LOAD:%.*]] = load i8, i8* {{.*}}
+// OBJC: [[SHL:%.*]] = shl i8 [[LOAD]], 7
+// OBJC: [[ASHR:%.*]] = ashr i8 [[SHL]], 7
+// OBJC: icmp ule i8 [[ASHR]], 1, !nosanitize
+// OBJC: call void @__ubsan_handle_load_invalid_value
+
+// Also check direct accesses to the ivar.
+// OBJC-LABEL: f3
+BOOL f3(I1 *i) {
+  // OBJC: [[LOAD:%.*]] = load i8, i8* {{.*}}
+  // OBJC: [[SHL:%.*]] = shl i8 [[LOAD]], 7
+  // OBJC: [[ASHR:%.*]] = ashr i8 [[SHL]], 7
+  // OBJC: icmp ule i8 [[ASHR]], 1, !nosanitize
+  // OBJC: call void @__ubsan_handle_load_invalid_value
+
+  return i->b1;
+  // OBJC: ret i8
+}
+#endif /* __OBJC__ */