From f5f0bd8e3d972d040827c88adf1a9880ebcb821e Mon Sep 17 00:00:00 2001 From: James Y Knight Date: Mon, 7 Feb 2022 11:31:22 -0500 Subject: Revert "[Clang] Propagate guaranteed alignment for malloc and others" The above change assumed that malloc (and friends) would always allocate memory to getNewAlign(), even for allocations which have a smaller size. This is not actually required by spec (a 1-byte allocation may validly have 1-byte alignment). Some real-world malloc implementations do not provide this guarantee, and thus this optimization is breaking programs. Fixes #53540 This reverts commit c2297544c04764237cedc523083c7be2fb3833d4. Differential Revision: https://reviews.llvm.org/D118804 (cherry picked from commit 9545976ff160e19805a84a06a7e59d446f9994d9) --- clang/include/clang/Basic/TargetInfo.h | 4 +-- clang/lib/Sema/SemaDecl.cpp | 17 ---------- clang/test/CodeGen/alloc-fns-alignment.c | 56 ++++++++++++-------------------- 3 files changed, 22 insertions(+), 55 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index a49342a34f3e..e7db877f4e2b 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -644,8 +644,8 @@ public: } /// Return the largest alignment for which a suitably-sized allocation with - /// '::operator new(size_t)' or 'malloc' is guaranteed to produce a - /// correctly-aligned pointer. + /// '::operator new(size_t)' is guaranteed to produce a correctly-aligned + /// pointer. unsigned getNewAlign() const { return NewAlign ? NewAlign : std::max(LongDoubleAlign, LongLongAlign); } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index cbd9df4d6a7b..bcadf4139046 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15319,24 +15319,7 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) { if (!FD->hasAttr()) FD->addAttr(AllocAlignAttr::CreateImplicit(Context, ParamIdx(1, FD), FD->getLocation())); - LLVM_FALLTHROUGH; - case Builtin::BIcalloc: - case Builtin::BImalloc: - case Builtin::BImemalign: - case Builtin::BIrealloc: - case Builtin::BIstrdup: - case Builtin::BIstrndup: { - if (!FD->hasAttr()) { - unsigned NewAlign = Context.getTargetInfo().getNewAlign() / - Context.getTargetInfo().getCharWidth(); - IntegerLiteral *Alignment = IntegerLiteral::Create( - Context, Context.MakeIntValue(NewAlign, Context.UnsignedIntTy), - Context.UnsignedIntTy, FD->getLocation()); - FD->addAttr(AssumeAlignedAttr::CreateImplicit( - Context, Alignment, /*Offset=*/nullptr, FD->getLocation())); - } break; - } default: break; } diff --git a/clang/test/CodeGen/alloc-fns-alignment.c b/clang/test/CodeGen/alloc-fns-alignment.c index 8ab0610accf0..31dc9bb2d759 100644 --- a/clang/test/CodeGen/alloc-fns-alignment.c +++ b/clang/test/CodeGen/alloc-fns-alignment.c @@ -1,11 +1,9 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN16 -// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN16 -// RUN: %clang_cc1 -triple i386-apple-darwin -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN16 -// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN8 -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-malloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-MALLOC -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-calloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-CALLOC -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-realloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-REALLOC -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-aligned_alloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-ALIGNED_ALLOC +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm < %s | FileCheck %s + +// Note: this test originally asserted that malloc/calloc/realloc got alignment +// attributes on their return pointer. However, that was reverted in +// https://reviews.llvm.org/D118804 and it now asserts that they do _NOT_ get +// align attributes. typedef __SIZE_TYPE__ size_t; @@ -39,43 +37,29 @@ void *aligned_alloc_large_constant_test(size_t n) { } // CHECK-LABEL: @malloc_test -// ALIGN16: align 16 i8* @malloc - -// CHECK-LABEL: @calloc_test -// ALIGN16: align 16 i8* @calloc - -// CHECK-LABEL: @realloc_test -// ALIGN16: align 16 i8* @realloc - -// CHECK-LABEL: @aligned_alloc_variable_test -// ALIGN16: %[[ALLOCATED:.*]] = call align 16 i8* @aligned_alloc({{i32|i64}} noundef %[[ALIGN:.*]], {{i32|i64}} noundef %[[NBYTES:.*]]) -// ALIGN16-NEXT: call void @llvm.assume(i1 true) [ "align"(i8* %[[ALLOCATED]], {{i32|i64}} %[[ALIGN]]) ] +// CHECK: call i8* @malloc -// CHECK-LABEL: @aligned_alloc_constant_test -// ALIGN16: align 16 i8* @aligned_alloc - -// CHECK-LABEL: @aligned_alloc_large_constant_test -// ALIGN16: align 4096 i8* @aligned_alloc - -// CHECK-LABEL: @malloc_test -// ALIGN8: align 8 i8* @malloc +// CHECK: declare i8* @malloc // CHECK-LABEL: @calloc_test -// ALIGN8: align 8 i8* @calloc +// CHECK: call i8* @calloc + +// CHECK: declare i8* @calloc // CHECK-LABEL: @realloc_test -// ALIGN8: align 8 i8* @realloc +// CHECK: call i8* @realloc + +// CHECK: declare i8* @realloc // CHECK-LABEL: @aligned_alloc_variable_test -// ALIGN8: align 8 i8* @aligned_alloc +// CHECK: %[[ALLOCATED:.*]] = call i8* @aligned_alloc({{i32|i64}} noundef %[[ALIGN:.*]], {{i32|i64}} noundef %[[NBYTES:.*]]) +// CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(i8* %[[ALLOCATED]], {{i32|i64}} %[[ALIGN]]) ] + +// CHECK: declare i8* @aligned_alloc // CHECK-LABEL: @aligned_alloc_constant_test -// ALIGN8: align 8 i8* @aligned_alloc +// CHECK: call align 8 i8* @aligned_alloc // CHECK-LABEL: @aligned_alloc_large_constant_test -// ALIGN8: align 4096 i8* @aligned_alloc +// CHECK: call align 4096 i8* @aligned_alloc -// NOBUILTIN-MALLOC: declare i8* @malloc -// NOBUILTIN-CALLOC: declare i8* @calloc -// NOBUILTIN-REALLOC: declare i8* @realloc -// NOBUILTIN-ALIGNED_ALLOC: declare i8* @aligned_alloc -- cgit v1.2.3