https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/166755
>From 337b0d0d8d517a056eea440ae30cf9edab4f3a6a Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Fri, 17 Oct 2025 13:49:55 +0100 Subject: [PATCH 1/8] [UBSan] Report more detailed alignment report when overloaded global new returns incorrectly aligned memory --- clang/lib/CodeGen/CGExprCXX.cpp | 21 +++++++++- clang/lib/CodeGen/CodeGenFunction.h | 5 ++- compiler-rt/lib/ubsan/ubsan_checks.inc | 1 + compiler-rt/lib/ubsan/ubsan_handlers.cpp | 17 ++++++-- .../TestCases/TypeCheck/minimum-alignment.cpp | 39 +++++++++++++++++++ 5 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 14d8db32bafc6..37c2d47102b54 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -18,8 +18,12 @@ #include "ConstantEmitter.h" #include "TargetInfo.h" #include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/Sanitizers.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "llvm/IR/Intrinsics.h" +#include <cstdio> using namespace clang; using namespace CodeGen; @@ -1749,6 +1753,21 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { allocator->isReservedGlobalPlacementOperator()) result = Builder.CreateLaunderInvariantGroup(result); + // Check what type of constructor call the sanitizer is checking + // Different UB can occour with custom overloads of operator new + TypeCheckKind checkKind = CodeGenFunction::TCK_ConstructorCall; + const TargetInfo& TI = getContext().getTargetInfo(); + unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth(); + SourceManager &SM = getContext().getSourceManager(); + SourceLocation Loc = E->getOperatorNew()->getLocation(); + bool IsCustomOverload = !SM.isInSystemHeader(Loc); + if ( + SanOpts.has(SanitizerKind::Alignment) && + IsCustomOverload && + (DefaultTargetAlignment > CGM.getContext().getTypeAlignInChars(allocType).getQuantity()) + ) + checkKind = CodeGenFunction::TCK_ConstructorCallOverloadedNew; + // 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. @@ -1756,7 +1775,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { // 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); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 8c4c1c8c2dc95..2200715a79ea5 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/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_ConstructorCallOverloadedNew }; /// Determine whether the pointer type check \p TCK permits null pointers. diff --git a/compiler-rt/lib/ubsan/ubsan_checks.inc b/compiler-rt/lib/ubsan/ubsan_checks.inc index b1d09a9024e7e..df3ef0f595659 100644 --- a/compiler-rt/lib/ubsan/ubsan_checks.inc +++ b/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(AlignmentOnOverloadedNew, "alignment-new", "alignment") UBSAN_CHECK(InsufficientObjectSize, "insufficient-object-size", "object-size") UBSAN_CHECK(SignedIntegerOverflow, "signed-integer-overflow", "signed-integer-overflow") diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp index 63319f46734a4..a16d2c6879907 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -73,14 +73,17 @@ 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_ConstructorCallOverloadedNew }; 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"}; + "dynamic operation on", "constructor call with pointer from overloaded operator new on"}; } static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, @@ -94,7 +97,9 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, ? ErrorType::NullPointerUseWithNullability : ErrorType::NullPointerUse; else if (Pointer & (Alignment - 1)) - ET = ErrorType::MisalignedPointerUse; + ET = (Data->TypeCheckKind == TCK_ConstructorCallOverloadedNew) + ? ErrorType::AlignmentOnOverloadedNew + : ErrorType::MisalignedPointerUse; else ET = ErrorType::InsufficientObjectSize; @@ -117,6 +122,12 @@ 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::AlignmentOnOverloadedNew: + Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %2, " + "which requires target minimum assumed alignment of %3") + << 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") diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp new file mode 100644 index 0000000000000..8f9e7f652bafe --- /dev/null +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp @@ -0,0 +1,39 @@ +// RUN: %clangxx %gmlt -fsanitize=alignment %s -o %t +// RUN: %run %t 2>&1 | FileCheck %s + +// UNSUPPORTED: i386 +// UNSUPPORTED: armv7l + +// These sanitizers already overload the new operator so won't compile this test +// UNSUPPORTED: ubsan-msan +// UNSUPPORTED: ubsan-tsan + +#include <cassert> +#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 overloaded operator new on misaligned address 0x{{.*}} for type 'Foo', which requires target minimum assumed alignment of 16 + Foo* f = new Foo; + return 0; +} \ No newline at end of file >From 043ea10885c5743777c6b7f191d6ef74a952743b Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Fri, 17 Oct 2025 14:02:39 +0100 Subject: [PATCH 2/8] Tidy --- clang/lib/CodeGen/CGExprCXX.cpp | 20 +++++------ clang/lib/CodeGen/CodeGenFunction.h | 2 +- compiler-rt/lib/ubsan/ubsan_handlers.cpp | 32 +++++++++++------ .../TestCases/TypeCheck/minimum-alignment.cpp | 35 +++++++++---------- 4 files changed, 46 insertions(+), 43 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 37c2d47102b54..52d67be7f634a 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -23,7 +23,6 @@ #include "clang/Basic/SourceManager.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "llvm/IR/Intrinsics.h" -#include <cstdio> using namespace clang; using namespace CodeGen; @@ -1756,18 +1755,16 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { // Check what type of constructor call the sanitizer is checking // Different UB can occour with custom overloads of operator new TypeCheckKind checkKind = CodeGenFunction::TCK_ConstructorCall; - const TargetInfo& TI = getContext().getTargetInfo(); + const TargetInfo &TI = getContext().getTargetInfo(); unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth(); SourceManager &SM = getContext().getSourceManager(); SourceLocation Loc = E->getOperatorNew()->getLocation(); bool IsCustomOverload = !SM.isInSystemHeader(Loc); - if ( - SanOpts.has(SanitizerKind::Alignment) && - IsCustomOverload && - (DefaultTargetAlignment > CGM.getContext().getTypeAlignInChars(allocType).getQuantity()) - ) + if (SanOpts.has(SanitizerKind::Alignment) && IsCustomOverload && + (DefaultTargetAlignment > + CGM.getContext().getTypeAlignInChars(allocType).getQuantity())) checkKind = CodeGenFunction::TCK_ConstructorCallOverloadedNew; - + // 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. @@ -1775,10 +1772,9 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { // we'll null check the wrong pointer here. SanitizerSet SkippedChecks; SkippedChecks.set(SanitizerKind::Null, nullCheck); - EmitTypeCheck(checkKind, - E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(), - result, allocType, result.getAlignment(), SkippedChecks, - numElements); + EmitTypeCheck( + checkKind, E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(), + result, allocType, result.getAlignment(), SkippedChecks, numElements); EmitNewInitializer(*this, E, allocType, elementTy, result, numElements, allocSizeWithoutCookie); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 2200715a79ea5..06e0734e2f2ea 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -3297,7 +3297,7 @@ class CodeGenFunction : public CodeGenTypeCache { /// Checking the operand of a dynamic_cast or a typeid expression. Must be /// null or an object within its lifetime. TCK_DynamicOperation, - /// Checking the 'this' poiner for a constructor call, including that the + /// Checking the 'this' poiner for a constructor call, including that the /// alignment is greater or equal to the targets minimum alignment TCK_ConstructorCallOverloadedNew }; diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp index a16d2c6879907..5896b93700e6b 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -74,16 +74,25 @@ enum TypeCheckKind { /// Checking the operand of a dynamic_cast or a typeid expression. Must be /// null or an object within its lifetime. TCK_DynamicOperation, - /// Checking the 'this' poiner for a constructor call, including that the + /// Checking the 'this' poiner for a constructor call, including that the /// alignment is greater or equal to the targets minimum alignment TCK_ConstructorCallOverloadedNew }; 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", "constructor call with pointer from overloaded operator new 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 overloaded operator new on"}; } static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, @@ -98,8 +107,8 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, : ErrorType::NullPointerUse; else if (Pointer & (Alignment - 1)) ET = (Data->TypeCheckKind == TCK_ConstructorCallOverloadedNew) - ? ErrorType::AlignmentOnOverloadedNew - : ErrorType::MisalignedPointerUse; + ? ErrorType::AlignmentOnOverloadedNew + : ErrorType::MisalignedPointerUse; else ET = ErrorType::InsufficientObjectSize; @@ -123,10 +132,11 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, << TypeCheckKinds[Data->TypeCheckKind] << Data->Type; break; case ErrorType::AlignmentOnOverloadedNew: - Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %2, " - "which requires target minimum assumed alignment of %3") - << TypeCheckKinds[Data->TypeCheckKind] << (void *)Pointer - << Data->Type << Alignment; + Diag(Loc, DL_Error, ET, + "%0 misaligned address %1 for type %2, " + "which requires target minimum assumed alignment of %3") + << TypeCheckKinds[Data->TypeCheckKind] << (void *)Pointer << Data->Type + << Alignment; break; case ErrorType::MisalignedPointerUse: Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %3, " diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp index 8f9e7f652bafe..2204ff361672d 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp @@ -1,7 +1,7 @@ // RUN: %clangxx %gmlt -fsanitize=alignment %s -o %t // RUN: %run %t 2>&1 | FileCheck %s -// UNSUPPORTED: i386 +// UNSUPPORTED: i386 // UNSUPPORTED: armv7l // These sanitizers already overload the new operator so won't compile this test @@ -11,29 +11,26 @@ #include <cassert> #include <cstdlib> -void* operator new(std::size_t count) -{ - constexpr const size_t offset = 8; +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); + // 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); + // 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; + return (char *)ptr + offset; } -struct Foo -{ - void* _cookie1, *_cookie2; +struct Foo { + void *_cookie1, *_cookie2; }; static_assert(alignof(Foo) == 8); -int main() -{ - // CHECK: runtime error: constructor call with pointer from overloaded operator new on misaligned address 0x{{.*}} for type 'Foo', which requires target minimum assumed alignment of 16 - Foo* f = new Foo; - return 0; -} \ No newline at end of file +int main() { + // CHECK: runtime error: constructor call with pointer from overloaded operator new on misaligned address 0x{{.*}} for type 'Foo', which requires target minimum assumed alignment of 16 + Foo *f = new Foo; + return 0; +} >From d01970b433e62052e10a8361d4f5fc35ac7aba55 Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Thu, 30 Oct 2025 23:13:18 +0000 Subject: [PATCH 3/8] Remove strict check and generalise error message --- clang/lib/CodeGen/CGExprCXX.cpp | 2 +- compiler-rt/lib/ubsan/ubsan_handlers.cpp | 4 ++-- .../test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp | 2 +- compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 52d67be7f634a..c4f6afa320fe3 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1760,7 +1760,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { SourceManager &SM = getContext().getSourceManager(); SourceLocation Loc = E->getOperatorNew()->getLocation(); bool IsCustomOverload = !SM.isInSystemHeader(Loc); - if (SanOpts.has(SanitizerKind::Alignment) && IsCustomOverload && + if (SanOpts.has(SanitizerKind::Alignment) && (DefaultTargetAlignment > CGM.getContext().getTypeAlignInChars(allocType).getQuantity())) checkKind = CodeGenFunction::TCK_ConstructorCallOverloadedNew; diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp index 5896b93700e6b..2aaa899afa943 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -92,7 +92,7 @@ extern const char *const TypeCheckKinds[] = { "cast to virtual base of", "_Nonnull binding to", "dynamic operation on", - "constructor call with pointer from overloaded operator new on"}; + "constructor call with pointer from operator new on"}; } static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, @@ -134,7 +134,7 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, case ErrorType::AlignmentOnOverloadedNew: Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %2, " - "which requires target minimum assumed alignment of %3") + "which requires target minimum assumed %3 byte alignment") << TypeCheckKinds[Data->TypeCheckKind] << (void *)Pointer << Data->Type << Alignment; break; diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp index 2204ff361672d..4642126ab74c4 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp @@ -30,7 +30,7 @@ struct Foo { static_assert(alignof(Foo) == 8); int main() { - // CHECK: runtime error: constructor call with pointer from overloaded operator new on misaligned address 0x{{.*}} for type 'Foo', which requires target minimum assumed alignment of 16 + // 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; } diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp index e39a0ab4e6589..4b0b2b5923c6f 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp +++ b/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 target minimum assumed 4 byte alignment // CHECK-NEW-NEXT: [[PTR]]: note: pointer points here // CHECK-NEW-NEXT: {{^ 00 00 00 01 02 03 04 05}} // CHECK-NEW-NEXT: {{^ \^}} >From de077a2956c5c6997f29e33788f9799788da0cf8 Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Fri, 31 Oct 2025 16:34:29 +0000 Subject: [PATCH 4/8] Finished code removal --- clang/lib/CodeGen/CGExprCXX.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index c4f6afa320fe3..ba51f64c00f14 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1757,9 +1757,6 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { TypeCheckKind checkKind = CodeGenFunction::TCK_ConstructorCall; const TargetInfo &TI = getContext().getTargetInfo(); unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth(); - SourceManager &SM = getContext().getSourceManager(); - SourceLocation Loc = E->getOperatorNew()->getLocation(); - bool IsCustomOverload = !SM.isInSystemHeader(Loc); if (SanOpts.has(SanitizerKind::Alignment) && (DefaultTargetAlignment > CGM.getContext().getTypeAlignInChars(allocType).getQuantity())) >From e9d3243c074d290ea9dd0797b3f89471d2787d1d Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Mon, 3 Nov 2025 14:09:26 +0000 Subject: [PATCH 5/8] Rename error type and change comment to reflect changes in checks --- clang/lib/CodeGen/CGExprCXX.cpp | 7 ++++--- clang/lib/CodeGen/CodeGenFunction.h | 2 +- compiler-rt/lib/ubsan/ubsan_checks.inc | 2 +- compiler-rt/lib/ubsan/ubsan_handlers.cpp | 8 ++++---- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index ba51f64c00f14..f2dd22e9bed3b 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1752,15 +1752,16 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { allocator->isReservedGlobalPlacementOperator()) result = Builder.CreateLaunderInvariantGroup(result); - // Check what type of constructor call the sanitizer is checking - // Different UB can occour with custom overloads of operator new + // 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; const TargetInfo &TI = getContext().getTargetInfo(); unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth(); if (SanOpts.has(SanitizerKind::Alignment) && (DefaultTargetAlignment > CGM.getContext().getTypeAlignInChars(allocType).getQuantity())) - checkKind = CodeGenFunction::TCK_ConstructorCallOverloadedNew; + checkKind = CodeGenFunction::TCK_ConstructorCallMinimumAlign; // 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 diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 06e0734e2f2ea..047ca844c79de 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -3299,7 +3299,7 @@ class CodeGenFunction : public CodeGenTypeCache { TCK_DynamicOperation, /// Checking the 'this' poiner for a constructor call, including that the /// alignment is greater or equal to the targets minimum alignment - TCK_ConstructorCallOverloadedNew + TCK_ConstructorCallMinimumAlign }; /// Determine whether the pointer type check \p TCK permits null pointers. diff --git a/compiler-rt/lib/ubsan/ubsan_checks.inc b/compiler-rt/lib/ubsan/ubsan_checks.inc index df3ef0f595659..f8757d781afb8 100644 --- a/compiler-rt/lib/ubsan/ubsan_checks.inc +++ b/compiler-rt/lib/ubsan/ubsan_checks.inc @@ -28,7 +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(AlignmentOnOverloadedNew, "alignment-new", "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") diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp index 2aaa899afa943..fc6063af4562b 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -76,7 +76,7 @@ enum TypeCheckKind { TCK_DynamicOperation, /// Checking the 'this' poiner for a constructor call, including that the /// alignment is greater or equal to the targets minimum alignment - TCK_ConstructorCallOverloadedNew + TCK_ConstructorCallMinimumAlign }; extern const char *const TypeCheckKinds[] = { @@ -106,8 +106,8 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, ? ErrorType::NullPointerUseWithNullability : ErrorType::NullPointerUse; else if (Pointer & (Alignment - 1)) - ET = (Data->TypeCheckKind == TCK_ConstructorCallOverloadedNew) - ? ErrorType::AlignmentOnOverloadedNew + ET = (Data->TypeCheckKind == TCK_ConstructorCallMinimumAlign) + ? ErrorType::MinumumAssumedAlignment : ErrorType::MisalignedPointerUse; else ET = ErrorType::InsufficientObjectSize; @@ -131,7 +131,7 @@ 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::AlignmentOnOverloadedNew: + case ErrorType::MinumumAssumedAlignment: Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %2, " "which requires target minimum assumed %3 byte alignment") >From 4e32b7b7059aeffd2724e0954c045cfaa3cf26c7 Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Tue, 4 Nov 2025 16:16:51 +0000 Subject: [PATCH 6/8] Ensure we pass the right alignment to the error, and fixing failing tests --- clang/lib/CodeGen/CGExprCXX.cpp | 8 ++++++-- clang/test/CodeGenCXX/ubsan-new-checks.cpp | 4 ++-- .../test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp | 5 +++-- compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index f2dd22e9bed3b..9acf3a1817c82 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -17,6 +17,7 @@ #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/Basic/SourceLocation.h" @@ -1756,12 +1757,15 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { // 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())) + CGM.getContext().getTypeAlignInChars(allocType).getQuantity())){ 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 @@ -1772,7 +1776,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { SkippedChecks.set(SanitizerKind::Null, nullCheck); 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); diff --git a/clang/test/CodeGenCXX/ubsan-new-checks.cpp b/clang/test/CodeGenCXX/ubsan-new-checks.cpp index 60edd323648ab..c8a20ecfed3b4 100644 --- a/clang/test/CodeGenCXX/ubsan-new-checks.cpp +++ b/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]; diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp index 4642126ab74c4..96e1939ea4363 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp @@ -1,7 +1,7 @@ -// RUN: %clangxx %gmlt -fsanitize=alignment %s -o %t +// RUN: %clangxx %gmlt -std=c++17 -m64 -fsanitize=alignment %s -o %t // RUN: %run %t 2>&1 | FileCheck %s -// UNSUPPORTED: i386 +// UNSUPPORTED: i386, i686 // UNSUPPORTED: armv7l // These sanitizers already overload the new operator so won't compile this test @@ -10,6 +10,7 @@ #include <cassert> #include <cstdlib> +#include <cstddef> void *operator new(std::size_t count) { constexpr const size_t offset = 8; diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp index 4b0b2b5923c6f..74d5c9a35754c 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp +++ b/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 with pointer from operator new on misaligned address [[PTR:0x[0-9a-f]*]] for type 'S', which requires target minimum assumed 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: {{^ \^}} >From 10b77f37678a3d333b0e324b2118426eb5d2908f Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Thu, 6 Nov 2025 14:09:10 +0000 Subject: [PATCH 7/8] Add escape case if alignment is 1 --- clang/lib/CodeGen/CGExprCXX.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 9acf3a1817c82..73edf245353b2 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1762,7 +1762,8 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth(); if (SanOpts.has(SanitizerKind::Alignment) && (DefaultTargetAlignment > - CGM.getContext().getTypeAlignInChars(allocType).getQuantity())){ + CGM.getContext().getTypeAlignInChars(allocType).getQuantity()) && + !result.getAlignment().isOne()) { checkKind = CodeGenFunction::TCK_ConstructorCallMinimumAlign; checkAlignment = CharUnits::fromQuantity(DefaultTargetAlignment); } >From 91c2c6cf97468f72775eb403c1f7688af06d8c54 Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Thu, 6 Nov 2025 14:37:30 +0000 Subject: [PATCH 8/8] tidy --- clang/lib/CodeGen/CGExprCXX.cpp | 10 ++++------ .../ubsan/TestCases/TypeCheck/minimum-alignment.cpp | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 73edf245353b2..23c7c15cded03 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -20,8 +20,6 @@ #include "clang/AST/CharUnits.h" #include "clang/Basic/CodeGenOptions.h" #include "clang/Basic/Sanitizers.h" -#include "clang/Basic/SourceLocation.h" -#include "clang/Basic/SourceManager.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "llvm/IR/Intrinsics.h" @@ -1763,7 +1761,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { if (SanOpts.has(SanitizerKind::Alignment) && (DefaultTargetAlignment > CGM.getContext().getTypeAlignInChars(allocType).getQuantity()) && - !result.getAlignment().isOne()) { + !result.getAlignment().isOne() && result.getAlignment().getQuantity() < DefaultTargetAlignment) { checkKind = CodeGenFunction::TCK_ConstructorCallMinimumAlign; checkAlignment = CharUnits::fromQuantity(DefaultTargetAlignment); } @@ -1775,9 +1773,9 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { // we'll null check the wrong pointer here. SanitizerSet SkippedChecks; SkippedChecks.set(SanitizerKind::Null, nullCheck); - EmitTypeCheck( - checkKind, E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(), - result, allocType, checkAlignment, SkippedChecks, numElements); + EmitTypeCheck(checkKind, + E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(), + result, allocType, checkAlignment, SkippedChecks, numElements); EmitNewInitializer(*this, E, allocType, elementTy, result, numElements, allocSizeWithoutCookie); diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp index 96e1939ea4363..fb99227ee65f2 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp @@ -9,8 +9,8 @@ // UNSUPPORTED: ubsan-tsan #include <cassert> -#include <cstdlib> #include <cstddef> +#include <cstdlib> void *operator new(std::size_t count) { constexpr const size_t offset = 8; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
