Skip to content

Commit f5f0bd8

Browse files
jyknighttstellar
authored andcommitted
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 llvm#53540 This reverts commit c229754. Differential Revision: https://reviews.llvm.org/D118804 (cherry picked from commit 9545976)
1 parent 7246d58 commit f5f0bd8

File tree

3 files changed

+22
-55
lines changed

3 files changed

+22
-55
lines changed

clang/include/clang/Basic/TargetInfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -644,8 +644,8 @@ class TargetInfo : public virtual TransferrableTargetInfo,
644644
}
645645

646646
/// Return the largest alignment for which a suitably-sized allocation with
647-
/// '::operator new(size_t)' or 'malloc' is guaranteed to produce a
648-
/// correctly-aligned pointer.
647+
/// '::operator new(size_t)' is guaranteed to produce a correctly-aligned
648+
/// pointer.
649649
unsigned getNewAlign() const {
650650
return NewAlign ? NewAlign : std::max(LongDoubleAlign, LongLongAlign);
651651
}

clang/lib/Sema/SemaDecl.cpp

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15319,24 +15319,7 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
1531915319
if (!FD->hasAttr<AllocAlignAttr>())
1532015320
FD->addAttr(AllocAlignAttr::CreateImplicit(Context, ParamIdx(1, FD),
1532115321
FD->getLocation()));
15322-
LLVM_FALLTHROUGH;
15323-
case Builtin::BIcalloc:
15324-
case Builtin::BImalloc:
15325-
case Builtin::BImemalign:
15326-
case Builtin::BIrealloc:
15327-
case Builtin::BIstrdup:
15328-
case Builtin::BIstrndup: {
15329-
if (!FD->hasAttr<AssumeAlignedAttr>()) {
15330-
unsigned NewAlign = Context.getTargetInfo().getNewAlign() /
15331-
Context.getTargetInfo().getCharWidth();
15332-
IntegerLiteral *Alignment = IntegerLiteral::Create(
15333-
Context, Context.MakeIntValue(NewAlign, Context.UnsignedIntTy),
15334-
Context.UnsignedIntTy, FD->getLocation());
15335-
FD->addAttr(AssumeAlignedAttr::CreateImplicit(
15336-
Context, Alignment, /*Offset=*/nullptr, FD->getLocation()));
15337-
}
1533815322
break;
15339-
}
1534015323
default:
1534115324
break;
1534215325
}
Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN16
2-
// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN16
3-
// RUN: %clang_cc1 -triple i386-apple-darwin -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN16
4-
// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm < %s | FileCheck %s --check-prefix=ALIGN8
5-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-malloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-MALLOC
6-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-calloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-CALLOC
7-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-realloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-REALLOC
8-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fno-builtin-aligned_alloc -emit-llvm < %s | FileCheck %s --check-prefix=NOBUILTIN-ALIGNED_ALLOC
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm < %s | FileCheck %s
2+
3+
// Note: this test originally asserted that malloc/calloc/realloc got alignment
4+
// attributes on their return pointer. However, that was reverted in
5+
// https://reviews.llvm.org/D118804 and it now asserts that they do _NOT_ get
6+
// align attributes.
97

108
typedef __SIZE_TYPE__ size_t;
119

@@ -39,43 +37,29 @@ void *aligned_alloc_large_constant_test(size_t n) {
3937
}
4038

4139
// CHECK-LABEL: @malloc_test
42-
// ALIGN16: align 16 i8* @malloc
43-
44-
// CHECK-LABEL: @calloc_test
45-
// ALIGN16: align 16 i8* @calloc
46-
47-
// CHECK-LABEL: @realloc_test
48-
// ALIGN16: align 16 i8* @realloc
49-
50-
// CHECK-LABEL: @aligned_alloc_variable_test
51-
// ALIGN16: %[[ALLOCATED:.*]] = call align 16 i8* @aligned_alloc({{i32|i64}} noundef %[[ALIGN:.*]], {{i32|i64}} noundef %[[NBYTES:.*]])
52-
// ALIGN16-NEXT: call void @llvm.assume(i1 true) [ "align"(i8* %[[ALLOCATED]], {{i32|i64}} %[[ALIGN]]) ]
40+
// CHECK: call i8* @malloc
5341

54-
// CHECK-LABEL: @aligned_alloc_constant_test
55-
// ALIGN16: align 16 i8* @aligned_alloc
56-
57-
// CHECK-LABEL: @aligned_alloc_large_constant_test
58-
// ALIGN16: align 4096 i8* @aligned_alloc
59-
60-
// CHECK-LABEL: @malloc_test
61-
// ALIGN8: align 8 i8* @malloc
42+
// CHECK: declare i8* @malloc
6243

6344
// CHECK-LABEL: @calloc_test
64-
// ALIGN8: align 8 i8* @calloc
45+
// CHECK: call i8* @calloc
46+
47+
// CHECK: declare i8* @calloc
6548

6649
// CHECK-LABEL: @realloc_test
67-
// ALIGN8: align 8 i8* @realloc
50+
// CHECK: call i8* @realloc
51+
52+
// CHECK: declare i8* @realloc
6853

6954
// CHECK-LABEL: @aligned_alloc_variable_test
70-
// ALIGN8: align 8 i8* @aligned_alloc
55+
// CHECK: %[[ALLOCATED:.*]] = call i8* @aligned_alloc({{i32|i64}} noundef %[[ALIGN:.*]], {{i32|i64}} noundef %[[NBYTES:.*]])
56+
// CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(i8* %[[ALLOCATED]], {{i32|i64}} %[[ALIGN]]) ]
57+
58+
// CHECK: declare i8* @aligned_alloc
7159

7260
// CHECK-LABEL: @aligned_alloc_constant_test
73-
// ALIGN8: align 8 i8* @aligned_alloc
61+
// CHECK: call align 8 i8* @aligned_alloc
7462

7563
// CHECK-LABEL: @aligned_alloc_large_constant_test
76-
// ALIGN8: align 4096 i8* @aligned_alloc
64+
// CHECK: call align 4096 i8* @aligned_alloc
7765

78-
// NOBUILTIN-MALLOC: declare i8* @malloc
79-
// NOBUILTIN-CALLOC: declare i8* @calloc
80-
// NOBUILTIN-REALLOC: declare i8* @realloc
81-
// NOBUILTIN-ALIGNED_ALLOC: declare i8* @aligned_alloc

0 commit comments

Comments
 (0)