Skip to content

Commit

Permalink
Revert "[Clang] Propagate guaranteed alignment for malloc and others"
Browse files Browse the repository at this point in the history
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 c229754.

Differential Revision: https://reviews.llvm.org/D118804
  • Loading branch information
jyknight committed Feb 8, 2022
1 parent 34d557f commit 9545976
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 65 deletions.
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/TargetInfo.h
Expand Up @@ -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);
}
Expand Down
16 changes: 0 additions & 16 deletions clang/lib/Sema/SemaDecl.cpp
Expand Up @@ -15320,23 +15320,7 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
if (!FD->hasAttr<AllocAlignAttr>())
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<AssumeAlignedAttr>()) {
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;
}
Expand Down
70 changes: 23 additions & 47 deletions 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;

Expand Down Expand Up @@ -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

0 comments on commit 9545976

Please sign in to comment.