[clang] 9545976 - Revert "[Clang] Propagate guaranteed alignment for malloc and others"

James Y Knight via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 8 11:35:09 PST 2022


Author: James Y Knight
Date: 2022-02-08T14:34:44-05:00
New Revision: 9545976ff160e19805a84a06a7e59d446f9994d9

URL: https://github.com/llvm/llvm-project/commit/9545976ff160e19805a84a06a7e59d446f9994d9
DIFF: https://github.com/llvm/llvm-project/commit/9545976ff160e19805a84a06a7e59d446f9994d9.diff

LOG: 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

Added: 
    

Modified: 
    clang/include/clang/Basic/TargetInfo.h
    clang/lib/Sema/SemaDecl.cpp
    clang/test/CodeGen/alloc-fns-alignment.c

Removed: 
    


################################################################################
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<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;
     }

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


        


More information about the cfe-commits mailing list