lebedev.ri updated this revision to Diff 245625.
lebedev.ri added a comment.

Rebased ontop of patch demoting call-site-based align attr checking from error 
into a warning, NFC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73020/new/

https://reviews.llvm.org/D73020

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/diagnose_if.cpp
  clang/test/SemaCXX/operator-new-size-diagnose_if.cpp
  clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp

Index: clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
===================================================================
--- clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
+++ clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
@@ -32,7 +32,7 @@
 
 void *ptr_variable(int align) { return new (std::align_val_t(align)) A; }
 void *ptr_align16() { return new (std::align_val_t(16)) A; }
-void *ptr_align15() { return new (std::align_val_t(15)) A; }
+void *ptr_align15() { return new (std::align_val_t(15)) A; } // expected-warning {{requested alignment is not a power of 2}}
 
 struct alignas(128) S {
   S() {}
@@ -49,11 +49,9 @@
   return new (std::align_val_t(256)) S;
 }
 void *alloc_overaligned_struct_with_extra_255_alignment(int align) {
-  return new (std::align_val_t(255)) S;
+  return new (std::align_val_t(255)) S; // expected-warning {{requested alignment is not a power of 2}}
 }
 
 std::align_val_t align_variable(int align) { return std::align_val_t(align); }
 std::align_val_t align_align16() { return std::align_val_t(16); }
 std::align_val_t align_align15() { return std::align_val_t(15); }
-
-// expected-no-diagnostics
Index: clang/test/SemaCXX/operator-new-size-diagnose_if.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/operator-new-size-diagnose_if.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++14
+
+using size_t = decltype(sizeof(int));
+
+#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__)))
+
+namespace operator_new {
+struct T0 {
+  int j = 0;
+  static void *operator new(size_t i) _diagnose_if(i == sizeof(int), "yay", "warning"); // expected-note{{from 'diagnose_if'}}
+};
+
+struct T1 {
+  int j = 0;
+  static void *operator new[](size_t i) _diagnose_if(i == 8 * sizeof(int), "yay", "warning"); // expected-note 2{{from 'diagnose_if'}}
+};
+
+void run(int x) {
+  new T0;       // expected-warning{{yay}}
+  new T1[8];    // expected-warning{{yay}}
+  new T1[4][2]; // expected-warning{{yay}}
+  new T1[x];    // no warning.
+}
+} // namespace operator_new
Index: clang/test/SemaCXX/diagnose_if.cpp
===================================================================
--- clang/test/SemaCXX/diagnose_if.cpp
+++ clang/test/SemaCXX/diagnose_if.cpp
@@ -634,7 +634,7 @@
 namespace operator_new {
 struct Foo {
   int j;
-  static void *operator new(size_t i) _diagnose_if(i, "oh no", "warning");
+  static void *operator new(size_t i) _diagnose_if(i, "oh no", "warning"); // expected-note{{from 'diagnose_if'}}
 };
 
 struct Bar {
@@ -643,10 +643,7 @@
 };
 
 void run() {
-  // FIXME: This should emit a diagnostic.
-  new Foo();
-  // This is here because we sometimes pass a dummy argument `operator new`. We
-  // should ignore this, rather than complaining about it.
+  new Foo(); // expected-warning{{oh no}}
   new Bar();
 }
 }
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2116,18 +2116,80 @@
     // arguments. Skip the first parameter because we don't have a corresponding
     // argument. Skip the second parameter too if we're passing in the
     // alignment; we've already filled it in.
+    unsigned NumImplicitArgs = PassAlignment ? 2 : 1;
     if (GatherArgumentsForCall(PlacementLParen, OperatorNew, Proto,
-                               PassAlignment ? 2 : 1, PlacementArgs,
-                               AllPlaceArgs, CallType))
+                               NumImplicitArgs, PlacementArgs, AllPlaceArgs,
+                               CallType))
       return ExprError();
 
     if (!AllPlaceArgs.empty())
       PlacementArgs = AllPlaceArgs;
 
-    // FIXME: This is wrong: PlacementArgs misses out the first (size) argument.
-    DiagnoseSentinelCalls(OperatorNew, PlacementLParen, PlacementArgs);
+    // We would like to perform some checking on the given `operator new` call,
+    // but the PlacementArgs does not contain the implicit arguments,
+    // namely allocation size and maybe allocation alignment,
+    // so we need to conjure them.
 
-    // FIXME: Missing call to CheckFunctionCall or equivalent
+    QualType SizeTy = Context.getSizeType();
+    unsigned SizeTyWidth = Context.getTypeSize(SizeTy);
+
+    llvm::APInt SingleEltSize(
+        SizeTyWidth, Context.getTypeSizeInChars(AllocType).getQuantity());
+
+    // How many bytes do we want to allocate here?
+    llvm::Optional<llvm::APInt> AllocationSize;
+    if (!ArraySize.hasValue() && !AllocType->isDependentType()) {
+      // For non-array operator new, we only want to allocate one element.
+      AllocationSize = SingleEltSize;
+    } else if (KnownArraySize.hasValue() && !AllocType->isDependentType()) {
+      // For array operator new, only deal with static array size case.
+      bool Overflow;
+      AllocationSize = llvm::APInt(SizeTyWidth, *KnownArraySize)
+                           .umul_ov(SingleEltSize, Overflow);
+      (void)Overflow;
+      assert(
+          !Overflow &&
+          "Expected that all the overflows would have been handled already.");
+    }
+
+    IntegerLiteral AllocationSizeLiteral(
+        Context,
+        AllocationSize.getValueOr(llvm::APInt::getNullValue(SizeTyWidth)),
+        SizeTy, SourceLocation());
+    // Otherwise, if we failed to constant-fold the allocation size, we'll
+    // just give up and pass-in something opaque, that isn't a null pointer.
+    OpaqueValueExpr OpaqueAllocationSize(SourceLocation(), SizeTy, VK_RValue,
+                                         OK_Ordinary, /*SourceExpr=*/nullptr);
+
+    // Let's synthesize the alignment argument in case we will need it.
+    // Since we *really* want to allocate these on stack, this is slightly ugly
+    // because there might not be a `std::align_val_t` type.
+    EnumDecl *StdAlignValT = getStdAlignValT();
+    QualType AlignValT =
+        StdAlignValT ? Context.getTypeDeclType(StdAlignValT) : SizeTy;
+    IntegerLiteral AlignmentLiteral(
+        Context,
+        llvm::APInt(Context.getTypeSize(SizeTy),
+                    Alignment / Context.getCharWidth()),
+        SizeTy, SourceLocation());
+    ImplicitCastExpr DesiredAlignment(ImplicitCastExpr::OnStack, AlignValT,
+                                      CK_IntegralCast, &AlignmentLiteral,
+                                      VK_RValue);
+
+    // Adjust placement args by prepending conjured size and alignment exprs.
+    llvm::SmallVector<Expr *, 8> CallArgs;
+    CallArgs.reserve(NumImplicitArgs + PlacementArgs.size());
+    CallArgs.emplace_back(AllocationSize.hasValue()
+                              ? static_cast<Expr *>(&AllocationSizeLiteral)
+                              : &OpaqueAllocationSize);
+    if (PassAlignment)
+      CallArgs.emplace_back(&DesiredAlignment);
+    CallArgs.insert(CallArgs.end(), PlacementArgs.begin(), PlacementArgs.end());
+
+    DiagnoseSentinelCalls(OperatorNew, PlacementLParen, CallArgs);
+
+    checkCall(OperatorNew, Proto, /*ThisArg=*/nullptr, CallArgs,
+              /*IsMemberFunction=*/false, StartLoc, Range, CallType);
 
     // Warn if the type is over-aligned and is being allocated by (unaligned)
     // global operator new.
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3896,8 +3896,9 @@
     auto *AA = FDecl->getAttr<AllocAlignAttr>();
     const Expr *Arg = Args[AA->getParamIndex().getASTIndex()];
     if (!Arg->isValueDependent()) {
-      llvm::APSInt I(64);
-      if (Arg->isIntegerConstantExpr(I, Context)) {
+      Expr::EvalResult Align;
+      if (Arg->EvaluateAsInt(Align, Context)) {
+        const llvm::APSInt &I = Align.Val.getInt();
         if (!I.isPowerOf2())
           Diag(Arg->getExprLoc(), diag::warn_alignment_not_power_of_two)
               << Arg->getSourceRange();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to