[clang][bytecode] Fix calling operator new with nothrow/align parameter

[tbaederr]

Discard all the parameters we don't care about.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Discard all the parameters we don't care about.

---

Full diff: https://github.com/llvm/llvm-project/pull/144271.diff

2 Files Affected:

• (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+19-1)
• (modified) clang/test/AST/ByteCode/new-delete.cpp (+16-6)

diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index 5fc5034569597..d01e3d042a8bf 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -1423,7 +1423,6 @@ static bool interp__builtin_operator_new(InterpState &S, CodePtr OpPC,
   // Walk up the call stack to find the appropriate caller and get the
   // element type from it.
   auto [NewCall, ElemType] = S.getStdAllocatorCaller("allocate");
-  APSInt Bytes = popToAPSInt(S.Stk, *S.getContext().classify(Call->getArg(0)));
 
   if (ElemType.isNull()) {
     S.FFDiag(Call, S.getLangOpts().CPlusPlus20
@@ -1439,6 +1438,25 @@ static bool interp__builtin_operator_new(InterpState &S, CodePtr OpPC,
     return false;
   }
 
+  // We only care about the first parameter (the size), so discard all the
+  // others.
+  {
+    unsigned NumArgs = Call->getNumArgs();
+    assert(NumArgs >= 1);
+
+    // The std::nothrow_t arg never gets put on the stack.
+    if (Call->getArg(NumArgs - 1)->getType()->isNothrowT())
+      --NumArgs;
+    auto Args = llvm::ArrayRef(Call->getArgs(), Call->getNumArgs());
+    // First arg is needed.
+    Args = Args.drop_front();
+
+    // Discard the rest.
+    for (const Expr *Arg : Args)
+      discard(S.Stk, *S.getContext().classify(Arg));
+  }
+
+  APSInt Bytes = popToAPSInt(S.Stk, *S.getContext().classify(Call->getArg(0)));
   CharUnits ElemSize = S.getASTContext().getTypeSizeInChars(ElemType);
   assert(!ElemSize.isZero());
   // Divide the number of bytes by sizeof(ElemType), so we get the number of
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 1ee41a98e13bb..9c293e5d15fc8 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -triple=i686-linux-gnu -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -verify=ref,both %s
-// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
-// RUN: %clang_cc1 -triple=i686-linux-gnu -std=c++20 -verify=ref,both %s
+// RUN: %clang_cc1            -verify=expected,both                        -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -std=c++20 -verify=expected,both                        -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -std=c++20 -verify=expected,both -triple=i686-linux-gnu -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1            -verify=ref,both                                                                     %s
+// RUN: %clang_cc1 -std=c++20 -verify=ref,both                                                                     %s
+// RUN: %clang_cc1 -std=c++20 -verify=ref,both      -triple=i686-linux-gnu                                         %s
 
 #if __cplusplus >= 202002L
 
@@ -1012,6 +1012,16 @@ constexpr int no_deallocate_nonalloc = (std::allocator<int>().deallocate((int*)&
                                                                                                              // both-note {{in call}} \
                                                                                                              // both-note {{declared here}}
 
+namespace OpNewNothrow {
+  constexpr int f() {
+      int *v = (int*)operator new(sizeof(int), std::align_val_t(2), std::nothrow); // both-note {{cannot allocate untyped memory in a constant expression; use 'std::allocator<T>::allocate' to allocate memory of type 'T'}}
+      operator delete(v, std::align_val_t(2), std::nothrow);
+      return 1;
+  }
+  static_assert(f()); // both-error {{not an integral constant expression}} \
+                      // both-note {{in call to}}
+}
+
 #else
 /// Make sure we reject this prior to C++20
 constexpr int a() { // both-error {{never produces a constant expression}}

[ojhunt]

Looks reasonable to me -- I spent a bunch of time trying to induce some case where this would do the wrong thing and I couldn't come up with anything as all the failure modes that would be possible outside of constexpr already fail in const eval due to them inherently triggering UB.