https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/152532
>From 47142640181a1c882429a2ba7b83d87d18cfd7d1 Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Fri, 17 Oct 2025 13:49:55 +0100 Subject: [PATCH 1/3] [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 c52526c89f171..a7f5f6c59e8fd 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; @@ -1736,6 +1740,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. @@ -1743,7 +1762,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 f0565c1de04c4..7b0810c047fec 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -3292,7 +3292,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 eb9ea2c46f3cb33125b5172d5d8f2a600f750c07 Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Fri, 17 Oct 2025 14:02:39 +0100 Subject: [PATCH 2/3] 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 a7f5f6c59e8fd..f9f5995d03f78 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; @@ -1743,18 +1742,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. @@ -1762,10 +1759,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 7b0810c047fec..55cb929e0af4e 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -3293,7 +3293,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 4cac6e277618361f3e6f4a3e93295de285ac88fa Mon Sep 17 00:00:00 2001 From: gbMattN <[email protected]> Date: Thu, 30 Oct 2025 23:13:18 +0000 Subject: [PATCH 3/3] 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 f9f5995d03f78..f2ef4a187ea37 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1747,7 +1747,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: {{^ \^}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
