Author: henrimarconni Date: 2026-10-17T19:07:51Z New Revision: aa8686516c37fb64873d3730f118a1d07a831671
URL: https://github.com/llvm/llvm-project/commit/aa8686516c37fb64873d3730f118a1d07a831671 DIFF: https://github.com/llvm/llvm-project/commit/aa8686516c37fb64873d3730f118a1d07a831671.diff LOG: [clang][CodeGen] Fix assertion failure for new T[SIZE_MAX] with array cookie (#232570) Fixes #232250. `EmitCXXNewExpr` asserts that `allocSize == allocSizeWithoutCookie` if and only if no array cookie is needed. The "only if" direction does not always hold. `EmitCXXNewAllocSize` replaces the allocation size with an all-ones value when the size computation overflows, so that `operator new` fails. If the count times the element size is already all-ones and adding the cookie then overflows, both values are the same uniqued constant even though a cookie is present. For example, on i386: struct S { ~S(); }; void f() { new S[0xFFFFFFFFu]; } Here `sizeof(S) == 1`, the cookie is 4 bytes, and `0xFFFFFFFF + 4` overflows, so the assertion fired. This can't happen on 64-bit targets for constant counts, because Sema already rejects them with "array is too large". The generated code was already correct (`operator new[]` is called with -1), so only the assertion was too strict. This patch keeps the direction that always holds (no cookie implies equal sizes), drops the converse, and documents why. Added: clang/test/CodeGenCXX/new-array-cookie-overflow.cpp Modified: clang/lib/CodeGen/CGExprCXX.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 8279611accba33..a1b3e28277b359 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1789,8 +1789,12 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { cleanupDominator = Builder.CreateUnreachable(); } - assert((allocSize == allocSizeWithoutCookie) == - CalculateCookiePadding(*this, E).isZero()); + // Without a cookie the two sizes must match. With a cookie they usually + // diff er, but on overflow the allocation size is all-ones, which can + // coincide with the size without the cookie. + assert(allocSize == llvm::Constant::getAllOnesValue(SizeTy) || + ((allocSize == allocSizeWithoutCookie) == + CalculateCookiePadding(*this, E).isZero())); if (allocSize != allocSizeWithoutCookie) { assert(E->isArray()); allocation = CGM.getCXXABI().InitializeArrayCookie( diff --git a/clang/test/CodeGenCXX/new-array-cookie-overflow.cpp b/clang/test/CodeGenCXX/new-array-cookie-overflow.cpp new file mode 100644 index 00000000000000..690e2d046ceee8 --- /dev/null +++ b/clang/test/CodeGenCXX/new-array-cookie-overflow.cpp @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s + +// Regression test for https://github.com/llvm/llvm-project/issues/232250. +// With a 32-bit size_t, 'new S[0xFFFFFFFF]' has no multiplication overflow +// (sizeof(S) == 1), but adding the array cookie overflows. The allocation +// size becomes all-ones, which can coincide with the size without the cookie. +// This used to hit an assertion in EmitCXXNewExpr. + +struct S { + ~S(); +}; + +// CHECK-LABEL: define {{.*}} @_Z1fv( +// CHECK: call {{.*}} @_Znaj(i32 {{.*}}-1) +void f() { new S[0xFFFFFFFFu]; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
