From 9ea5d17cc9544838c73e593de4ef224d54fa1cff Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Thu, 20 Feb 2020 16:39:26 +0300 Subject: [PATCH] [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 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 --- .../clang/Basic/DiagnosticSemaKinds.td | 3 ++ clang/lib/CodeGen/CGCall.cpp | 4 ++ clang/lib/Sema/SemaChecking.cpp | 6 +-- .../non-power-of-2-alignment-assumptions.c | 46 +++++++++++++++++++ clang/test/Sema/alloc-align-attr.c | 2 +- clang/test/SemaCXX/alloc-align-attr.cpp | 6 +-- 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGen/non-power-of-2-alignment-assumptions.c 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>; 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 @@ public: const auto *AlignmentCI = dyn_cast(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 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 c; + dependent_param_struct c; c.Foo(1); dependent_param_struct d; // expected-note {{in instantiation of template class 'dependent_param_struct' requested here}} d.Foo(1.0);