Ubsan minimum assumed alignment

clang/lib/CodeGen/CGExprCXX.cpp

@@ -17,7 +17,9 @@
 #include "CodeGenFunction.h"
 #include "ConstantEmitter.h"
 #include "TargetInfo.h"
+#include "clang/AST/CharUnits.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Sanitizers.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/IR/Intrinsics.h"
 
@@ -1749,17 +1751,32 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
       allocator->isReservedGlobalPlacementOperator())
     result = Builder.CreateLaunderInvariantGroup(result);
 
+  // Check the default alignment of the type and why. Users may incorrectly
+  // return misaligned memory from a replaced operator new without knowing
+  // about default alignment.
+  TypeCheckKind checkKind = CodeGenFunction::TCK_ConstructorCall;
+  CharUnits checkAlignment = result.getAlignment();
+  const TargetInfo &TI = getContext().getTargetInfo();
+  unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth();
+  if (SanOpts.has(SanitizerKind::Alignment) &&
+      (DefaultTargetAlignment >
+       CGM.getContext().getTypeAlignInChars(allocType).getQuantity()) &&
+      !result.getAlignment().isOne() &&
+      result.getAlignment().getQuantity() <= DefaultTargetAlignment) {
+    checkKind = CodeGenFunction::TCK_ConstructorCallMinimumAlign;
+    checkAlignment = CharUnits::fromQuantity(DefaultTargetAlignment);
+  }
+
   // Emit sanitizer checks for pointer value now, so that in the case of an
   // array it was checked only once and not at each constructor call. We may
   // have already checked that the pointer is non-null.
   // FIXME: If we have an array cookie and a potentially-throwing allocator,
   // we'll null check the wrong pointer here.
   SanitizerSet SkippedChecks;
   SkippedChecks.set(SanitizerKind::Null, nullCheck);
-  EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
+  EmitTypeCheck(checkKind,
                 E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-                result, allocType, result.getAlignment(), SkippedChecks,
-                numElements);
+                result, allocType, checkAlignment, SkippedChecks, numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
                      allocSizeWithoutCookie);

clang/lib/CodeGen/CodeGenFunction.h

@@ -3296,7 +3296,10 @@ class CodeGenFunction : public CodeGenTypeCache {
     TCK_NonnullAssign,
     /// Checking the operand of a dynamic_cast or a typeid expression.  Must be
     /// null or an object within its lifetime.
-    TCK_DynamicOperation
+    TCK_DynamicOperation,
+    /// Checking the 'this' poiner for a constructor call, including that the
+    /// alignment is greater or equal to the targets minimum alignment
+    TCK_ConstructorCallMinimumAlign
   };
 
   /// Determine whether the pointer type check \p TCK permits null pointers.

clang/test/CodeGenCXX/ubsan-new-checks.cpp

@@ -75,7 +75,7 @@ S3 *func_07() {
   // CHECK-LABEL: define {{.*}} @_Z7func_07v
   // CHECK:       and i64 %{{.*}}, 31, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
-  // CHECK:       and i64 %{{.*}}, 3, !nosanitize
+  // CHECK:       and i64 %{{.*}}, 15, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
   // CHECK:       ret ptr
   return new S3;
@@ -85,7 +85,7 @@ S3 *func_08() {
   // CHECK-LABEL: define {{.*}} @_Z7func_08v
   // CHECK:       and i64 %{{.*}}, 31, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
-  // CHECK:       and i64 %{{.*}}, 3, !nosanitize
+  // CHECK:       and i64 %{{.*}}, 15, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
   // CHECK:       ret ptr
   return new S3[10];

compiler-rt/lib/ubsan/ubsan_checks.inc

@@ -28,6 +28,7 @@ UBSAN_CHECK(NullptrAfterNonZeroOffset, "nullptr-after-nonzero-offset",
 UBSAN_CHECK(PointerOverflow, "pointer-overflow", "pointer-overflow")
 UBSAN_CHECK(MisalignedPointerUse, "misaligned-pointer-use", "alignment")
 UBSAN_CHECK(AlignmentAssumption, "alignment-assumption", "alignment")
+UBSAN_CHECK(MinumumAssumedAlignment, "minimum-assumed-alignment", "alignment")
 UBSAN_CHECK(InsufficientObjectSize, "insufficient-object-size", "object-size")
 UBSAN_CHECK(SignedIntegerOverflow, "signed-integer-overflow",
             "signed-integer-overflow")

compiler-rt/lib/ubsan/ubsan_handlers.cpp

@@ -73,14 +73,26 @@ enum TypeCheckKind {
   TCK_NonnullAssign,
   /// Checking the operand of a dynamic_cast or a typeid expression.  Must be
   /// null or an object within its lifetime.
-  TCK_DynamicOperation
+  TCK_DynamicOperation,
+  /// Checking the 'this' poiner for a constructor call, including that the
+  /// alignment is greater or equal to the targets minimum alignment
+  TCK_ConstructorCallMinimumAlign
 };
 
 extern const char *const TypeCheckKinds[] = {
-    "load of", "store to", "reference binding to", "member access within",
-    "member call on", "constructor call on", "downcast of", "downcast of",
-    "upcast of", "cast to virtual base of", "_Nonnull binding to",
-    "dynamic operation on"};
+    "load of",
+    "store to",
+    "reference binding to",
+    "member access within",
+    "member call on",
+    "constructor call on",
+    "downcast of",
+    "downcast of",
+    "upcast of",
+    "cast to virtual base of",
+    "_Nonnull binding to",
+    "dynamic operation on",
+    "constructor call with pointer from operator new on"};
 }
 
 static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
@@ -94,7 +106,9 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
              ? ErrorType::NullPointerUseWithNullability
              : ErrorType::NullPointerUse;
   else if (Pointer & (Alignment - 1))
-    ET = ErrorType::MisalignedPointerUse;
+    ET = (Data->TypeCheckKind == TCK_ConstructorCallMinimumAlign)
+             ? ErrorType::MinumumAssumedAlignment
+             : ErrorType::MisalignedPointerUse;
   else
     ET = ErrorType::InsufficientObjectSize;
 
@@ -117,6 +131,13 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
     Diag(Loc, DL_Error, ET, "%0 null pointer of type %1")
         << TypeCheckKinds[Data->TypeCheckKind] << Data->Type;
     break;
+  case ErrorType::MinumumAssumedAlignment:
+    Diag(Loc, DL_Error, ET,
+         "%0 misaligned address %1 for type %2, "
+         "which requires target minimum assumed %3 byte alignment")
+        << TypeCheckKinds[Data->TypeCheckKind] << (void *)Pointer << Data->Type
+        << Alignment;
+    break;
   case ErrorType::MisalignedPointerUse:
     Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %3, "
                         "which requires %2 byte alignment")

compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp

@@ -0,0 +1,37 @@
+// RUN: %clangxx %gmlt -std=c++17 -m64 -fsanitize=alignment %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+
+// UNSUPPORTED: i386, i686
+// UNSUPPORTED: armv7l
+
+// These sanitizers already overload the new operator so won't compile this test
+// UNSUPPORTED: ubsan-msan
+// UNSUPPORTED: ubsan-tsan
+
+#include <cassert>
+#include <cstddef>
+#include <cstdlib>
+
+void *operator new(std::size_t count) {
+  constexpr const size_t offset = 8;
+
+  // allocate a bit more so we can safely offset it
+  void *ptr = std::malloc(count + offset);
+
+  // verify malloc returned 16 bytes aligned mem
+  static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ == 16);
+  assert(((std::ptrdiff_t)ptr & (__STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1)) == 0);
+
+  return (char *)ptr + offset;
+}
+
+struct Foo {
+  void *_cookie1, *_cookie2;
+};
+
+static_assert(alignof(Foo) == 8);
+int main() {
+  // CHECK: runtime error: constructor call with pointer from operator new on misaligned address 0x{{.*}} for type 'Foo', which requires target minimum assumed 16 byte alignment
+  Foo *f = new Foo;
+  return 0;
+}

compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp

@@ -101,7 +101,7 @@ int main(int, char **argv) {
     return s->f() && 0;
 
   case 'n':
-    // CHECK-NEW: misaligned.cpp:[[@LINE+4]]{{(:21)?}}: runtime error: constructor call on misaligned address [[PTR:0x[0-9a-f]*]] for type 'S', which requires 4 byte alignment
+    // CHECK-NEW: misaligned.cpp:[[@LINE+4]]{{(:21)?}}: runtime error: constructor call{{( with pointer from operator new)?}} on misaligned address [[PTR:0x[0-9a-f]*]] for type 'S', which requires {{(4|(target minimum assumed (8|(16))))}} byte alignment
     // CHECK-NEW-NEXT: [[PTR]]: note: pointer points here
     // CHECK-NEW-NEXT: {{^ 00 00 00 01 02 03 04  05}}
     // CHECK-NEW-NEXT: {{^             \^}}