[Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning

[lebedev.ri]

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?

[lebedev.ri]

Scale ambitions down - only demote the call-site-based AllocAlignAttr error.

[lebedev.ri]

Bump @rsmith / @erichkeane
thanks

[lebedev.ri]

Bump @rsmith / @erichkeane
thanks

[erichkeane]

1 nit, otherwise LGTM.

clang/include/clang/Basic/DiagnosticSemaKinds.td

3037

Can you use the above one (err_alignment_not_power_of_two .Text) so that they are forced to have the same message?

[lebedev.ri]

In D73996#1883360, @erichkeane wrote:

1 nit, otherwise LGTM.

Thank you for the review!

clang/include/clang/Basic/DiagnosticSemaKinds.td

3037

Oh, i always forget about that feature, thanks for pointing it out!

[lebedev.ri]

Rebased, NFC.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_alloca_align_alignof : Warning<
   InGroup<DiagGroup<"alloca-with-align-alignof">>;
 
 def err_alignment_too_small : Error<
   "requested alignment must be %0 or greater">;
 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">;
 
 def warn_alignment_builtin_useless : Warning<
   "%select{aligning a value|the result of checking whether a value is aligned}0"
   " to 1 byte is %select{a no-op|always true}0">, InGroup<TautologicalCompare>;
 def err_attribute_aligned_too_great : Error<
   "requested alignment must be %0 bytes or smaller">;

clang/lib/CodeGen/CGCall.cpp

public:
   /// If we can, materialize the alignment as an attribute on return value.
   LLVM_NODISCARD llvm::AttributeList
   TryEmitAsCallSiteAttribute(const llvm::AttributeList &Attrs) {
     if (!AA || OffsetCI || CGF.SanOpts.has(SanitizerKind::Alignment))
       return Attrs;
     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(
             AlignmentCI->getLimitedValue(llvm::Value::MaximumAlignment)));
     AA = nullptr; // We're done. Disallow doing anything else.
     return NewAttrs;
   }
 

clang/lib/Sema/SemaChecking.cpp

void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
   }
 
   if (FDecl && FDecl->hasAttr<AllocAlignAttr>()) {
     auto *AA = FDecl->getAttr<AllocAlignAttr>();
     const Expr *Arg = Args[AA->getParamIndex().getASTIndex()];
     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)
               << Arg->getSourceRange() << Sema::MaximumAlignment;
       }
     }
   }
 

clang/test/CodeGen/non-power-of-2-alignment-assumptions.c

+// 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);
+}

clang/test/Sema/alloc-align-attr.c

 
 void *passthrought(int a) {
   return test_ptr_alloc_align(a);
 }
 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}}
 }

clang/test/SemaCXX/alloc-align-attr.cpp

 
 void dependent_impl(int align) {
   dependent_ret<int> a; // expected-note {{in instantiation of template class 'dependent_ret<int>' requested here}}
   a.Foo(1);
   a.Foo2(1);
   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);
   dependent_param_func<int>(1);
   dependent_param_func<float>(1); // expected-note {{in instantiation of function template specialization 'dependent_param_func<float>' requested here}}
 }