filcab added you to the CC list for the revision "UBSan: Fix alignment checks 
emitted in downcasts.".

Hi rsmith,

UBSan was checking for alignment of the derived class on the pointer to
the base class, before converting. With some class hierarchies, this could
generate false positives.

Added test-case.

http://llvm-reviews.chandlerc.com/D1303

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGenCXX/sanitizer-alignment-downcast.cpp

Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2764,18 +2764,18 @@
 
     LValue LV = EmitLValue(E->getSubExpr());
 
-    // C++11 [expr.static.cast]p2: Behavior is undefined if a downcast is
-    // performed and the object is not of the derived type.
-    if (SanitizePerformTypeCheck)
-      EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(),
-                    LV.getAddress(), E->getType());
-
     // Perform the base-to-derived conversion
     llvm::Value *Derived =
       GetAddressOfDerivedClass(LV.getAddress(), DerivedClassDecl,
                                E->path_begin(), E->path_end(),
                                /*NullCheckValue=*/false);
 
+    // C++11 [expr.static.cast]p2: Behavior is undefined if a downcast is
+    // performed and the object is not of the derived type.
+    if (SanitizePerformTypeCheck)
+      EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(),
+                    Derived, E->getType());
+
     return MakeAddrLValue(Derived, E->getType());
   }
   case CK_LValueBitCast: {
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -1234,15 +1234,18 @@
 
     llvm::Value *V = Visit(E);
 
+    llvm::Value *Derived =
+      CGF.GetAddressOfDerivedClass(V, DerivedClassDecl,
+                                   CE->path_begin(), CE->path_end(),
+                                   ShouldNullCheckClassCastValue(CE));
+
     // C++11 [expr.static.cast]p11: Behavior is undefined if a downcast is
     // performed and the object is not of the derived type.
     if (CGF.SanitizePerformTypeCheck)
       CGF.EmitTypeCheck(CodeGenFunction::TCK_DowncastPointer, CE->getExprLoc(),
-                        V, DestTy->getPointeeType());
+                        Derived, DestTy->getPointeeType());
 
-    return CGF.GetAddressOfDerivedClass(V, DerivedClassDecl,
-                                        CE->path_begin(), CE->path_end(),
-                                        ShouldNullCheckClassCastValue(CE));
+    return Derived;
   }
   case CK_UncheckedDerivedToBase:
   case CK_DerivedToBase: {
Index: test/CodeGenCXX/sanitizer-alignment-downcast.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/sanitizer-alignment-downcast.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -O2 \
+// RUN:   -fsanitize=alignment -o - %s | FileCheck %s
+
+class A // align=4
+{
+  int a1, a2, a3;
+};
+
+class B // align=8
+{
+  long b1, b2;
+};
+
+class C : public A, public B // align=16
+{
+  alignas(16) int c1;
+};
+
+// Make sure we check the alignment of the pointer after subtracting any
+// offset. The pointer before subtraction doesn't need to be aligned for
+// the destination type.
+
+// CHECK: define %class.C* @_Z15downcastPointerP1B(%class.B* %b)
+C *downcastPointer(B *b)
+{
+  return (C*)b;
+  // Alignment check from EmitTypeCheck(TCK_DowncastPointer, ...)
+  // CHECK: [[SUB:%sub[.a-z0-9]*]] = getelementptr i8* {{.*}}, i64 -1
+  // CHECK-NEXT: [[C:%[0-9]*]] = bitcast i8* [[SUB]] to %class.C*
+  // CHECK: [[FROM_PHI:%[0-9]*]] = phi %class.C* [ [[C]], %cast.notnull ], 
{{.*}}
+  // CHECK-NEXT: [[C_INT:%[0-9]*]] = ptrtoint %class.C* [[FROM_PHI]] to i64
+  // CHECK-NEXT: [[MASKED:%[0-9]*]] = and i64 [[C_INT]], 15
+}
+
+// CHECK: define %class.C* @_Z17downcastReferenceR1B(%class.B* %b)
+C& downcastReference(B &b)
+{
+  return (C&)b;
+  // Alignment check from EmitTypeCheck(TCK_DowncastReference, ...)
+  // CHECK:      [[SUB:%sub[.a-z0-9]*]] = getelementptr %class.B* %b, i64 -1
+  // CHECK-NEXT: [[C:%[0-9]*]] = bitcast %class.B* [[SUB]] to %class.C*
+  // CHECK-NEXT: [[C_INT:%[0-9]*]] = ptrtoint %class.B* [[SUB]] to i64
+  // CHECK-NEXT: [[MASKED:%[0-9]*]] = and i64 [[C_INT:%[0-9]*]] 15
+  // CHECK-NEXT: [[TEST:%[0-9]*]] = icmp eq i64 [[MASKED:%[0-9]*]] 0
+  // CHECK-NEXT: br i1 [[TEST:%[0-9]*]] label %cont2, label 
%handler.type_mismatch
+}
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2764,18 +2764,18 @@
 
     LValue LV = EmitLValue(E->getSubExpr());
 
-    // C++11 [expr.static.cast]p2: Behavior is undefined if a downcast is
-    // performed and the object is not of the derived type.
-    if (SanitizePerformTypeCheck)
-      EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(),
-                    LV.getAddress(), E->getType());
-
     // Perform the base-to-derived conversion
     llvm::Value *Derived =
       GetAddressOfDerivedClass(LV.getAddress(), DerivedClassDecl,
                                E->path_begin(), E->path_end(),
                                /*NullCheckValue=*/false);
 
+    // C++11 [expr.static.cast]p2: Behavior is undefined if a downcast is
+    // performed and the object is not of the derived type.
+    if (SanitizePerformTypeCheck)
+      EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(),
+                    Derived, E->getType());
+
     return MakeAddrLValue(Derived, E->getType());
   }
   case CK_LValueBitCast: {
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -1234,15 +1234,18 @@
 
     llvm::Value *V = Visit(E);
 
+    llvm::Value *Derived =
+      CGF.GetAddressOfDerivedClass(V, DerivedClassDecl,
+                                   CE->path_begin(), CE->path_end(),
+                                   ShouldNullCheckClassCastValue(CE));
+
     // C++11 [expr.static.cast]p11: Behavior is undefined if a downcast is
     // performed and the object is not of the derived type.
     if (CGF.SanitizePerformTypeCheck)
       CGF.EmitTypeCheck(CodeGenFunction::TCK_DowncastPointer, CE->getExprLoc(),
-                        V, DestTy->getPointeeType());
+                        Derived, DestTy->getPointeeType());
 
-    return CGF.GetAddressOfDerivedClass(V, DerivedClassDecl,
-                                        CE->path_begin(), CE->path_end(),
-                                        ShouldNullCheckClassCastValue(CE));
+    return Derived;
   }
   case CK_UncheckedDerivedToBase:
   case CK_DerivedToBase: {
Index: test/CodeGenCXX/sanitizer-alignment-downcast.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/sanitizer-alignment-downcast.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -O2 \
+// RUN:   -fsanitize=alignment -o - %s | FileCheck %s
+
+class A // align=4
+{
+  int a1, a2, a3;
+};
+
+class B // align=8
+{
+  long b1, b2;
+};
+
+class C : public A, public B // align=16
+{
+  alignas(16) int c1;
+};
+
+// Make sure we check the alignment of the pointer after subtracting any
+// offset. The pointer before subtraction doesn't need to be aligned for
+// the destination type.
+
+// CHECK: define %class.C* @_Z15downcastPointerP1B(%class.B* %b)
+C *downcastPointer(B *b)
+{
+  return (C*)b;
+  // Alignment check from EmitTypeCheck(TCK_DowncastPointer, ...)
+  // CHECK: [[SUB:%sub[.a-z0-9]*]] = getelementptr i8* {{.*}}, i64 -1
+  // CHECK-NEXT: [[C:%[0-9]*]] = bitcast i8* [[SUB]] to %class.C*
+  // CHECK: [[FROM_PHI:%[0-9]*]] = phi %class.C* [ [[C]], %cast.notnull ], {{.*}}
+  // CHECK-NEXT: [[C_INT:%[0-9]*]] = ptrtoint %class.C* [[FROM_PHI]] to i64
+  // CHECK-NEXT: [[MASKED:%[0-9]*]] = and i64 [[C_INT]], 15
+}
+
+// CHECK: define %class.C* @_Z17downcastReferenceR1B(%class.B* %b)
+C& downcastReference(B &b)
+{
+  return (C&)b;
+  // Alignment check from EmitTypeCheck(TCK_DowncastReference, ...)
+  // CHECK:      [[SUB:%sub[.a-z0-9]*]] = getelementptr %class.B* %b, i64 -1
+  // CHECK-NEXT: [[C:%[0-9]*]] = bitcast %class.B* [[SUB]] to %class.C*
+  // CHECK-NEXT: [[C_INT:%[0-9]*]] = ptrtoint %class.B* [[SUB]] to i64
+  // CHECK-NEXT: [[MASKED:%[0-9]*]] = and i64 [[C_INT:%[0-9]*]] 15
+  // CHECK-NEXT: [[TEST:%[0-9]*]] = icmp eq i64 [[MASKED:%[0-9]*]] 0
+  // CHECK-NEXT: br i1 [[TEST:%[0-9]*]] label %cont2, label %handler.type_mismatch
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to