[clang] 9a32f28 - [clang][bytecode] Fix a problem with array size limits (#109383)

[via cfe-commits]

Author: Timm Baeder
Date: 2024-09-20T11:26:58+02:00
New Revision: 9a32f28366b1099b0f5214e62473c0a2a2155434

URL: https://github.com/llvm/llvm-project/commit/9a32f28366b1099b0f5214e62473c0a2a2155434
DIFF: https://github.com/llvm/llvm-project/commit/9a32f28366b1099b0f5214e62473c0a2a2155434.diff

LOG: [clang][bytecode] Fix a problem with array size limits (#109383)

Descriptor::MaxArrayElemBytes is an unsigned value, which might overflow
the SizeT we have in CheckArraySize.

Added: 
    

Modified: 
    clang/lib/AST/ByteCode/Interp.h
    clang/test/AST/ByteCode/new-delete.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 52ccefee88642a..92bed32d56f4d5 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -241,11 +241,18 @@ bool CheckArraySize(InterpState &S, CodePtr OpPC, SizeT *NumElements,
   // FIXME: Both the SizeT::from() as well as the
   // NumElements.toAPSInt() in this function are rather expensive.
 
+  // Can't be too many elements if the bitwidth of NumElements is lower than
+  // that of Descriptor::MaxArrayElemBytes.
+  if ((NumElements->bitWidth() - NumElements->isSigned()) <
+      (sizeof(Descriptor::MaxArrayElemBytes) * 8))
+    return true;
+
   // FIXME: GH63562
   // APValue stores array extents as unsigned,
   // so anything that is greater that unsigned would overflow when
   // constructing the array, we catch this here.
   SizeT MaxElements = SizeT::from(Descriptor::MaxArrayElemBytes / ElemSize);
+  assert(MaxElements.isPositive());
   if (NumElements->toAPSInt().getActiveBits() >
           ConstantArrayType::getMaxSizeBits(S.getASTContext()) ||
       *NumElements > MaxElements) {

diff  --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 76858aa94bb37d..2ba1286b250dc6 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -718,6 +718,21 @@ namespace OperatorNewDelete {
   static_assert((std::allocator<float>().deallocate(std::allocator<float>().allocate(10)), 1) == 1);
 }
 
+namespace Limits {
+  template<typename T>
+  constexpr T dynarray(int elems, int i) {
+    T *p;
+    if constexpr (sizeof(T) == 1)
+      p = new T[elems]{"fox"};
+    else
+      p = new T[elems]{1, 2, 3};
+    T n = p[i];
+    delete [] p;
+    return n;
+  }
+  static_assert(dynarray<char>(5, 0) == 'f');
+}
+
 #else
 /// Make sure we reject this prior to C++20
 constexpr int a() { // both-error {{never produces a constant expression}}