[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, erichkeane, aaron.ballman, jdoerfert.
lebedev.ri added a project: clang.
lebedev.ri added a parent revision: D73019: [Sema] Don't disallow placing `__attribute__((alloc_align(param_idx)))` on `std::align_val_t`-typed parameters.

There was even a TODO for this.
The main motivation is to make use of call-site based
`__attribute__((alloc_align(param_idx)))` validation (D72996 <https://reviews.llvm.org/D72996>).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73020

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExprCXX.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
@@ -30,10 +30,8 @@
 
 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-error {{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/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2078,10 +2078,15 @@
     if (!AllPlaceArgs.empty())
       PlacementArgs = AllPlaceArgs;
 
-    // FIXME: This is wrong: PlacementArgs misses out the first (size) argument.
-    DiagnoseSentinelCalls(OperatorNew, PlacementLParen, PlacementArgs);
+    llvm::SmallVector<Expr *, 8> CallArgs;
+    CallArgs.reserve(1 + PlacementArgs.size());
+    CallArgs.emplace_back(nullptr); // FIXME: size argument.
+    CallArgs.insert(CallArgs.end(), PlacementArgs.begin(), PlacementArgs.end());
 
-    // FIXME: Missing call to CheckFunctionCall or equivalent
+    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
@@ -4476,7 +4476,7 @@
   if (FDecl && FDecl->hasAttr<AllocAlignAttr>()) {
     auto *AA = FDecl->getAttr<AllocAlignAttr>();
     const Expr *Arg = Args[AA->getParamIndex().getASTIndex()];
-    if (!Arg->isTypeDependent() && !Arg->isValueDependent()) {
+    if (Arg && !Arg->isTypeDependent() && !Arg->isValueDependent()) {
       llvm::APSInt I(64);
       if (Arg->isIntegerConstantExpr(I, Context)) {
         if (!I.isPowerOf2()) {
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -14460,8 +14460,10 @@
                                                     const Expr *E,
                                                     llvm::APSInt *Value,
                                                     SourceLocation *Loc) {
-  if (!E->getType()->isIntegralOrUnscopedEnumerationType()) {
-    if (Loc) *Loc = E->getExprLoc();
+  if (!E->getType()->isIntegralOrUnscopedEnumerationType() &&
+      !E->getType()->isAlignValT()) {
+    if (Loc)
+      *Loc = E->getExprLoc();
     return false;
   }
 
@@ -14593,7 +14595,8 @@
   ArgVector ArgValues(Args.size());
   for (ArrayRef<const Expr*>::iterator I = Args.begin(), E = Args.end();
        I != E; ++I) {
-    if ((*I)->isValueDependent() ||
+    // Arg can be null in malformed code.
+    if (!*I || (*I)->isValueDependent() ||
         !Evaluate(ArgValues[I - Args.begin()], Info, *I) ||
         Info.EvalStatus.HasSideEffects)
       // If evaluation fails, throw away the argument entirely.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73020.239038.patch
Type: text/x-patch
Size: 3639 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200120/94b76576/attachment.bin>