[clang] 9ea5d17 - [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 20 05:40:17 PST 2020
Author: Roman Lebedev
Date: 2020-02-20T16:39:26+03:00
New Revision: 9ea5d17cc9544838c73e593de4ef224d54fa1cff
URL: https://github.com/llvm/llvm-project/commit/9ea5d17cc9544838c73e593de4ef224d54fa1cff
DIFF: https://github.com/llvm/llvm-project/commit/9ea5d17cc9544838c73e593de4ef224d54fa1cff.diff
LOG: [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning
Summary:
As @rsmith notes in https://reviews.llvm.org/D73020#inline-672219
while that is certainly UB land, it may not be actually reachable at runtime, e.g.:
```
template<int N> void *make() {
if ((N & (N-1)) == 0)
return operator new(N, std::align_val_t(N));
else
return operator new(N);
}
void *p = make<7>();
```
and we shouldn't really error-out there.
That being said, i'm not really following the logic here.
Which ones of these cases should remain being an error?
Reviewers: rsmith, erichkeane
Reviewed By: erichkeane
Subscribers: cfe-commits, rsmith
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73996
Added:
clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/alloc-align-attr.c
clang/test/SemaCXX/alloc-align-attr.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1d1c8df6161d..60f2c777676d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3033,6 +3033,9 @@ def err_alignment_too_big : Error<
"requested alignment must be %0 or smaller">;
def err_alignment_not_power_of_two : Error<
"requested alignment is not a power of 2">;
+def warn_alignment_not_power_of_two : Warning<
+ err_alignment_not_power_of_two.Text>,
+ InGroup<DiagGroup<"non-power-of-two-alignment">>;
def err_alignment_dependent_typedef_name : Error<
"requested alignment is dependent but declaration is not dependent">;
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 6cc01540febe..98365dbc223f 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -3892,6 +3892,10 @@ template <typename AlignedAttrTy> class AbstractAssumeAlignedAttrEmitter {
const auto *AlignmentCI = dyn_cast<llvm::ConstantInt>(Alignment);
if (!AlignmentCI)
return Attrs;
+ // We may legitimately have non-power-of-2 alignment here.
+ // If so, this is UB land, emit it via `@llvm.assume` instead.
+ if (!AlignmentCI->getValue().isPowerOf2())
+ return Attrs;
llvm::AttributeList NewAttrs = maybeRaiseRetAlignmentAttribute(
CGF.getLLVMContext(), Attrs,
llvm::Align(
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index eca82c559e06..a986ef2bb685 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3898,11 +3898,9 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
if (!Arg->isValueDependent()) {
llvm::APSInt I(64);
if (Arg->isIntegerConstantExpr(I, Context)) {
- if (!I.isPowerOf2()) {
- Diag(Arg->getExprLoc(), diag::err_alignment_not_power_of_two)
+ if (!I.isPowerOf2())
+ Diag(Arg->getExprLoc(), diag::warn_alignment_not_power_of_two)
<< Arg->getSourceRange();
- return;
- }
if (I > Sema::MaximumAlignment)
Diag(Arg->getExprLoc(), diag::warn_assume_aligned_too_great)
diff --git a/clang/test/CodeGen/non-power-of-2-alignment-assumptions.c b/clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
new file mode 100644
index 000000000000..9467f6228dfc
--- /dev/null
+++ b/clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
@@ -0,0 +1,46 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+void *__attribute__((alloc_align(1))) alloc(int align);
+
+// CHECK-LABEL: @t0(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[ALIGN_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT: store i32 [[ALIGN:%.*]], i32* [[ALIGN_ADDR]], align 4
+// CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* [[ALIGN_ADDR]], align 4
+// CHECK-NEXT: [[CALL:%.*]] = call i8* @alloc(i32 [[TMP0]])
+// CHECK-NEXT: [[ALIGNMENTCAST:%.*]] = zext i32 [[TMP0]] to i64
+// CHECK-NEXT: [[MASK:%.*]] = sub i64 [[ALIGNMENTCAST]], 1
+// CHECK-NEXT: [[PTRINT:%.*]] = ptrtoint i8* [[CALL]] to i64
+// CHECK-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], [[MASK]]
+// CHECK-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
+// CHECK-NEXT: call void @llvm.assume(i1 [[MASKCOND]])
+// CHECK-NEXT: ret void
+//
+void t0(int align) {
+ alloc(align);
+}
+// CHECK-LABEL: @t1(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[ALIGN_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT: store i32 [[ALIGN:%.*]], i32* [[ALIGN_ADDR]], align 4
+// CHECK-NEXT: [[CALL:%.*]] = call i8* @alloc(i32 7)
+// CHECK-NEXT: [[PTRINT:%.*]] = ptrtoint i8* [[CALL]] to i64
+// CHECK-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], 6
+// CHECK-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
+// CHECK-NEXT: call void @llvm.assume(i1 [[MASKCOND]])
+// CHECK-NEXT: ret void
+//
+void t1(int align) {
+ alloc(7);
+}
+// CHECK-LABEL: @t2(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[ALIGN_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT: store i32 [[ALIGN:%.*]], i32* [[ALIGN_ADDR]], align 4
+// CHECK-NEXT: [[CALL:%.*]] = call align 8 i8* @alloc(i32 8)
+// CHECK-NEXT: ret void
+//
+void t2(int align) {
+ alloc(8);
+}
diff --git a/clang/test/Sema/alloc-align-attr.c b/clang/test/Sema/alloc-align-attr.c
index aa0fbd2cee3d..942ec117ee20 100644
--- a/clang/test/Sema/alloc-align-attr.c
+++ b/clang/test/Sema/alloc-align-attr.c
@@ -24,7 +24,7 @@ void *align16() {
return test_ptr_alloc_align(16);
}
void *align15() {
- return test_ptr_alloc_align(15); // expected-error {{requested alignment is not a power of 2}}
+ return test_ptr_alloc_align(15); // expected-warning {{requested alignment is not a power of 2}}
}
void *align536870912() {
return test_ptr_alloc_align(1073741824); // expected-warning {{requested alignment must be 536870912 bytes or smaller; maximum alignment assumed}}
diff --git a/clang/test/SemaCXX/alloc-align-attr.cpp b/clang/test/SemaCXX/alloc-align-attr.cpp
index f910cbcf2c90..bb59ab332dee 100644
--- a/clang/test/SemaCXX/alloc-align-attr.cpp
+++ b/clang/test/SemaCXX/alloc-align-attr.cpp
@@ -30,14 +30,14 @@ void dependent_impl(int align) {
dependent_ret<int *> b;
b.Foo(1);
b.Foo2(1);
- b.Foo(3); // expected-error {{requested alignment is not a power of 2}}
- b.Foo2(3); // expected-error {{requested alignment is not a power of 2}}
+ b.Foo(3); // expected-warning {{requested alignment is not a power of 2}}
+ b.Foo2(3); // expected-warning {{requested alignment is not a power of 2}}
b.Foo(1073741824); // expected-warning {{requested alignment must be 536870912 bytes or smaller; maximum alignment assumed}}
b.Foo2(1073741824); // expected-warning {{requested alignment must be 536870912 bytes or smaller; maximum alignment assumed}}
b.Foo(align);
b.Foo2(align);
- dependent_param_struct<int> c;
+ dependent_param_struct<int> c;
c.Foo(1);
dependent_param_struct<float> d; // expected-note {{in instantiation of template class 'dependent_param_struct<float>' requested here}}
d.Foo(1.0);
More information about the cfe-commits
mailing list