From 9545976ff160e19805a84a06a7e59d446f9994d9 Mon Sep 17 00:00:00 2001 From: James Y Knight Date: Mon, 7 Feb 2022 11:31:22 -0500 Subject: [PATCH] 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 --- clang/include/clang/Basic/TargetInfo.h | 4 +- clang/lib/Sema/SemaDecl.cpp | 16 ------ clang/test/CodeGen/alloc-fns-alignment.c | 70 ++++++++---------------- 3 files changed, 25 insertions(+), 65 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 324c123e5be72..22918f7e12e84 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -646,8 +646,8 @@ class TargetInfo : public virtual TransferrableTargetInfo, } /// 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 fce33991c4aa4..71c5fa5509760 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15320,23 +15320,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::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 29d6e9e4fb380..bbd213a2bf048 100644 --- a/clang/test/CodeGen/alloc-fns-alignment.c +++ b/clang/test/CodeGen/alloc-fns-alignment.c @@ -1,12 +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 -fno-builtin-memalign -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-MEMALIGN +// 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; @@ -49,57 +46,36 @@ void *memalign_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: call i8* @malloc -// 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-LABEL: @memalign_variable_test -// ALIGN16: %[[ALLOCATED:.*]] = call align 16 i8* @memalign({{i32|i64}} noundef %[[ALIGN:.*]], {{i32|i64}} noundef %[[NBYTES:.*]]) -// ALIGN16-NEXT: call void @llvm.assume(i1 true) [ "align"(i8* %[[ALLOCATED]], {{i32|i64}} %[[ALIGN]]) ] - -// 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: @memalign_large_constant_test -// ALIGN16: align 4096 i8* @memalign - -// 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: @memalign_variable_test -// ALIGN8: align 8 i8* @memalign +// CHECK: %[[ALLOCATED:.*]] = call i8* @memalign({{i32|i64}} noundef %[[ALIGN:.*]], {{i32|i64}} noundef %[[NBYTES:.*]]) +// CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(i8* %[[ALLOCATED]], {{i32|i64}} %[[ALIGN]]) ] // 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 // CHECK-LABEL: @memalign_large_constant_test -// ALIGN8: align 4096 i8* @memalign +// CHECK: align 4096 i8* @memalign -// NOBUILTIN-MALLOC: declare i8* @malloc -// NOBUILTIN-CALLOC: declare i8* @calloc -// NOBUILTIN-REALLOC: declare i8* @realloc -// NOBUILTIN-ALIGNED_ALLOC: declare i8* @aligned_alloc -// NOBUILTIN-MEMALIGN: declare i8* @memalign