Skip to content

Conversation

@tbaederr
Copy link
Contributor

We need to use the base offset in both cases.
Also, add additional assertions to make sure we don't miss this case again.

Fixes #155132

We need to use the base offset in both cases.
Also, add additional assertions to make sure we don't miss this case
again.

Fixes llvm#155132
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Aug 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

We need to use the base offset in both cases.
Also, add additional assertions to make sure we don't miss this case again.

Fixes #155132


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

3 Files Affected:

  • (modified) clang/lib/AST/ByteCode/EvaluationResult.cpp (+2-2)
  • (modified) clang/lib/AST/ByteCode/Pointer.h (+11-6)
  • (modified) clang/test/AST/ByteCode/invalid.cpp (+8)
diff --git a/clang/lib/AST/ByteCode/EvaluationResult.cpp b/clang/lib/AST/ByteCode/EvaluationResult.cpp
index b11531f4296a2..5110e243d167c 100644
--- a/clang/lib/AST/ByteCode/EvaluationResult.cpp
+++ b/clang/lib/AST/ByteCode/EvaluationResult.cpp
@@ -178,8 +178,8 @@ bool EvaluationResult::checkFullyInitialized(InterpState &S,
 static void collectBlocks(const Pointer &Ptr,
                           llvm::SetVector<const Block *> &Blocks) {
   auto isUsefulPtr = [](const Pointer &P) -> bool {
-    return P.isLive() && !P.isZero() && !P.isDummy() && P.isDereferencable() &&
-           !P.isUnknownSizeArray() && !P.isOnePastEnd();
+    return P.isLive() && P.isBlockPointer() && !P.isZero() && !P.isDummy() &&
+           P.isDereferencable() && !P.isUnknownSizeArray() && !P.isOnePastEnd();
   };
 
   if (!isUsefulPtr(Ptr))
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 0ce54ab0a17df..825a92b4424cb 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -696,15 +696,20 @@ class Pointer {
     assert(asBlockPointer().Pointee);
     assert(isDereferencable());
     assert(getFieldDesc()->isPrimitiveArray());
+    assert(I < getFieldDesc()->getNumElems());
 
     unsigned ElemByteOffset = I * getFieldDesc()->getElemSize();
-    if (isArrayRoot())
-      return *reinterpret_cast<T *>(asBlockPointer().Pointee->rawData() +
-                                    asBlockPointer().Base + sizeof(InitMapPtr) +
-                                    ElemByteOffset);
+    if (isArrayRoot()) {
+      unsigned ReadOffset = BS.Base + sizeof(InitMapPtr) + ElemByteOffset;
+      assert(ReadOffset + sizeof(T) <=
+             BS.Pointee->getDescriptor()->getAllocSize());
+      return *reinterpret_cast<T *>(BS.Pointee->rawData() + ReadOffset);
+    }
 
-    return *reinterpret_cast<T *>(asBlockPointer().Pointee->rawData() + Offset +
-                                  ElemByteOffset);
+    unsigned ReadOffset = BS.Base + ElemByteOffset;
+    assert(ReadOffset + sizeof(T) <=
+           BS.Pointee->getDescriptor()->getAllocSize());
+    return *reinterpret_cast<T *>(BS.Pointee->rawData() + ReadOffset);
   }
 
   /// Whether this block can be read from at all. This is only true for
diff --git a/clang/test/AST/ByteCode/invalid.cpp b/clang/test/AST/ByteCode/invalid.cpp
index 2a6c2d13e8467..affb40eada870 100644
--- a/clang/test/AST/ByteCode/invalid.cpp
+++ b/clang/test/AST/ByteCode/invalid.cpp
@@ -58,3 +58,11 @@ namespace Casts {
   /// Just make sure this doesn't crash.
   float PR9558 = reinterpret_cast<const float&>("asd");
 }
+
+
+/// This used to crash in collectBlock().
+struct S {
+};
+S s;
+S *sp[2] = {&s, &s};
+S *&spp = sp[1];

@tbaederr tbaederr merged commit 0f4db1a into llvm:main Aug 25, 2025
12 of 13 checks passed
tbaederr added a commit that referenced this pull request Aug 25, 2025
tbaederr added a commit to tbaederr/llvm-project that referenced this pull request Aug 25, 2025
This reverts commit 9642aad.

Since elem() only works on primitive arrays anyway, we don't have to do
the isArrayRoot() check at all.
tbaederr added a commit that referenced this pull request Aug 26, 2025
…155276)

This reverts commit 9642aad.

Since elem() only works on primitive arrays anyway, we don't have to do
the isArrayRoot() check at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[clang][bytecode] Assertion `isIntegralPointer()' failed with reference to pointer array element

2 participants