[PATCH] UBSan: Fix alignment checks emitted in downcasts.

[Filipe Cabecinhas]

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
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1303.1.patch
Type: text/x-patch
Size: 4143 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130807/5de122b5/attachment.bin>