[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
This commit is contained in:
Roman Lebedev 2020-02-20 16:39:26 +03:00
parent c7b7f76ae6
commit 9ea5d17cc9
No known key found for this signature in database
GPG Key ID: 083C3EBB4A1689E0
6 changed files with 59 additions and 8 deletions

View File

@ -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">;

View File

@ -3892,6 +3892,10 @@ public:
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(

View File

@ -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)

View File

@ -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);
}

View File

@ -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}}

View File

@ -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);