[clang] b18c59e - [clang][bytecode] Improve array index and offset add/sub bounds checks wrt. opaque pointers (#223353)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 14 05:32:13 PDT 2026
Author: Timm Baeder
Date: 2026-09-14T14:32:08+02:00
New Revision: b18c59ead34b97b087146cce7513a060d9ea1b85
URL: https://github.com/llvm/llvm-project/commit/b18c59ead34b97b087146cce7513a060d9ea1b85
DIFF: https://github.com/llvm/llvm-project/commit/b18c59ead34b97b087146cce7513a060d9ea1b85.diff
LOG: [clang][bytecode] Improve array index and offset add/sub bounds checks wrt. opaque pointers (#223353)
We were previously causing an assertion since the index had more than 64
bits.
Added:
Modified:
clang/lib/AST/ByteCode/Interp.cpp
clang/lib/AST/ByteCode/Interp.h
clang/lib/AST/ByteCode/InterpBuiltinObjectSize.cpp
clang/lib/AST/ByteCode/Pointer.cpp
clang/lib/AST/ByteCode/Pointer.h
clang/test/AST/ByteCode/cxx11.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index b0b8cfd7198ac..cc1c61c87da50 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -322,6 +322,16 @@ bool diagnoseShiftFailure(InterpState &S, CodePtr OpPC, ShiftFailure Failure,
return S.noteUndefinedBehavior();
}
+bool diagnoseArrayIndex(InterpState &S, CodePtr OpPC, const APSInt &Index,
+ std::optional<uint64_t> NumElems, bool IsArray) {
+ if (IsArray)
+ assert(NumElems);
+
+ S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
+ << Index << /*non-array=*/!IsArray << NumElems.value_or(0u);
+ return false;
+}
+
void cleanupAfterFunctionCall(InterpState &S, const Function *Func) {
assert(S.Current);
assert(Func);
@@ -3436,6 +3446,18 @@ bool arrayElemPtrOpaque(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
ElemType = ArrTy;
if (ArrTy->isArrayType()) {
+ unsigned IndexBits = std::max(Index.getBitWidth(), 32u) + 1;
+ APSInt NewIndex =
+ Index.extend(IndexBits) +
+ APSInt(APInt(IndexBits, Ptr.getIndex()), Index.isUnsigned());
+
+ if (NewIndex > Ptr.getNumElems() || NewIndex.isNegative())
+ diagnoseArrayIndex(S, OpPC, NewIndex, Ptr.getNumElems(),
+ OP.isArrayElement());
+
+ if (NewIndex.getActiveBits() > 64)
+ return false;
+
unsigned NewPathLength;
if (AllowReplace && OP.isArrayElement()) {
// This is what happens after an array-to-pointer-decay. We don't enter
@@ -3459,15 +3481,21 @@ bool arrayElemPtrOpaque(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
Ptr.getByteOffset());
} else {
- if (!validType(ElemType))
+ unsigned IndexBits = std::max(Index.getBitWidth(), 64u) + 1;
+ size_t CurrentIndex = Ptr.getByteOffset();
+ APSInt NewOffset =
+ Index.extend(IndexBits) +
+ APSInt(APInt(IndexBits, CurrentIndex), Index.isUnsigned());
+ if (NewOffset > 1 || NewOffset.isNegative())
+ diagnoseArrayIndex(S, OpPC, NewOffset, 0, false);
+
+ if (NewOffset.getActiveBits() > 64)
return false;
- unsigned ElemSize =
- S.getASTContext().getTypeSizeInChars(ElemType).getQuantity();
- size_t NewOffset = Ptr.getByteOffset() + (Index.getZExtValue() * ElemSize);
- bool PastEnd = Index != 0;
+ size_t NewByteOffset = CurrentIndex + Index.getZExtValue();
+ bool PastEnd = NewByteOffset != 0;
S.Stk.push<Pointer>(OP.withFieldType(ElemType.getTypePtr(), PastEnd),
- NewOffset);
+ NewByteOffset);
}
return true;
}
@@ -3498,21 +3526,32 @@ std::optional<Pointer> addSubOffsetOpaque(InterpState &S, CodePtr OpPC,
return std::nullopt;
}
- if (Offset > NumElems) {
- if (Op == ArithOp::Add)
- S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
- << Offset << /*non-array*/ !OP.isArrayElement() << NumElems;
- else
- S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
- << -Offset << /*non-array*/ !OP.isArrayElement() << NumElems;
- }
-
if (!validType(ElemTy) || !validType(ArrTy)) {
Invalid(S, OpPC);
return std::nullopt;
}
- if (Offset.getActiveBits() > 64)
+ APSInt NewIndex;
+ if (Op == ArithOp::Add) {
+ if (OP.isArrayElement()) {
+ NewIndex = Ptr.getIndex() + (Offset.extend(Offset.getBitWidth() + 2));
+ } else {
+ NewIndex =
+ (Ptr.getByteOffset()) + (Offset.extend(Offset.getBitWidth() + 2));
+ }
+ } else {
+ if (OP.isArrayElement()) {
+ NewIndex = Ptr.getIndex() - (Offset.extend(Offset.getBitWidth() + 2));
+ } else {
+ NewIndex =
+ (Ptr.getByteOffset()) - (Offset.extend(Offset.getBitWidth() + 2));
+ }
+ }
+
+ if (NewIndex > NumElems || NewIndex < 0)
+ diagnoseArrayIndex(S, OpPC, NewIndex, NumElems, OP.isArrayElement());
+
+ if (NewIndex.getActiveBits() > 64)
return std::nullopt;
// If the pointer is an array element, advance that index.
@@ -3527,17 +3566,7 @@ std::optional<Pointer> addSubOffsetOpaque(InterpState &S, CodePtr OpPC,
return OP.withPath(NewPath, NewPathLength, OP.FieldType.getPointer());
}
- unsigned ElemSize =
- S.getASTContext().getTypeSizeInChars(ElemTy).getQuantity();
- unsigned NewOffset;
- if (Op == ArithOp::Add)
- NewOffset = Ptr.getByteOffset() + (ElemSize * Offset.getZExtValue());
- else
- NewOffset = Ptr.getByteOffset() - (ElemSize * Offset.getZExtValue());
-
- // We already checked offset != before, so this is a non-array type being
- // offset by > 0.
- return Pointer(OP.withPastEnd(true), NewOffset);
+ return Pointer(OP.withPastEnd(true), NewIndex.getZExtValue());
}
bool virtBaseHelper(InterpState &S, const CXXRecordDecl *Decl,
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 7c71635561708..747311f42d757 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -83,6 +83,10 @@ bool diagnoseUninitialized(InterpState &S, CodePtr OpPC, bool Extern,
const Block *B, Lifetime LT = Lifetime::Started,
AccessKinds AK = AK_Read);
+bool diagnoseArrayIndex(InterpState &S, CodePtr OpPC, const APSInt &Index,
+ std::optional<uint64_t> NumElems = std::nullopt,
+ bool IsArray = true);
+
/// Checks a direct load of a primitive value from a global or local variable.
bool CheckGlobalLoad(InterpState &S, CodePtr OpPC, const Block *B);
bool CheckLocalLoad(InterpState &S, CodePtr OpPC, const Block *B);
@@ -2608,8 +2612,7 @@ std::optional<Pointer> OffsetHelper(InterpState &S, CodePtr OpPC,
N = Ptr.getByteOffset() - O;
if (N > 1)
- S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
- << N << /*non-array*/ true << 0;
+ diagnoseArrayIndex(S, OpPC, APSInt::getUnsigned(N), 0, /*IsArray=*/false);
return Pointer(Ptr.asFunctionPointer().Func, N);
} else if (Ptr.isStringPointer()) {
int64_t NewOffset;
@@ -2619,9 +2622,8 @@ std::optional<Pointer> OffsetHelper(InterpState &S, CodePtr OpPC,
NewOffset = Ptr.getRawOffset() - static_cast<int64_t>(Offset);
if (NewOffset < 0 ||
NewOffset > (Ptr.asStringPointer().getLiteral()->getLength() + 1)) {
- S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
- << NewOffset << /*non-array*/ false
- << (Ptr.asStringPointer().getLiteral()->getLength() + 1);
+ diagnoseArrayIndex(S, OpPC, APSInt::get(NewOffset),
+ (Ptr.asStringPointer().getLiteral()->getLength() + 1));
return std::nullopt;
}
return Pointer(Ptr.asStringPointer(), NewOffset);
@@ -2647,8 +2649,7 @@ std::optional<Pointer> OffsetHelper(InterpState &S, CodePtr OpPC,
/*IsUnsigned=*/false);
APSInt NewIndex =
(Op == ArithOp::Add) ? (APIndex + APOffset) : (APIndex - APOffset);
- S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
- << NewIndex << /*array*/ static_cast<int>(!Ptr.inArray()) << MaxIndex;
+ diagnoseArrayIndex(S, OpPC, NewIndex, MaxIndex, Ptr.inArray());
Invalid = true;
};
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinObjectSize.cpp b/clang/lib/AST/ByteCode/InterpBuiltinObjectSize.cpp
index c1a69e03c0e73..efb764cd72b19 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinObjectSize.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltinObjectSize.cpp
@@ -445,7 +445,8 @@ UnsignedOrNone evaluateBuiltinObjectSize(const ASTContext &ASTCtx,
return std::nullopt;
}
- *Offset += Ptr.getByteOffset();
+ *Offset += (Ptr.getByteOffset() *
+ ASTCtx.getTypeSizeInChars(OP.getFieldType()).getQuantity());
if (*Offset > *FullSize)
return 0u;
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 9a068071df570..c07b01da41f09 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -26,11 +26,13 @@ using namespace clang;
using namespace clang::interp;
// Helper to check if a Type can be passed to
-// ASTContext::getRecordLayout().
+// ASTContext::getTypeSize().
static bool validType(QualType T) {
if (const RecordDecl *RD = T->getAsRecordDecl())
return ASTContext::hasLayout(RD);
- return true;
+ return !T->isDependentType() && !T->isUndeducedAutoType() &&
+ !T->isSpecificBuiltinType(BuiltinType::UnknownAny) &&
+ !T->isIncompleteType();
}
Pointer::Pointer(Block *Pointee)
@@ -306,8 +308,12 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
}
}
size_t LayoutOffset = Opaque.computeLayoutOffset(ASTCtx).value_or(0);
- auto Offset = CharUnits::fromQuantity(LayoutOffset + getByteOffset());
- auto Result =
+ size_t ElemSize = 0;
+ if (validType(Opaque.getFieldType()))
+ ElemSize = ASTCtx.getTypeSizeInChars(Opaque.getFieldType()).getQuantity();
+ auto Offset =
+ CharUnits::fromQuantity(LayoutOffset + (this->Offset * ElemSize));
+ APValue Result =
APValue(Opaque.Base, Offset, Path,
/*IsOnePastEnd=*/Opaque.isOnePastEnd(), /*IsNullPtr=*/false);
Result.setConstexprUnknown(Opaque.isConstexprUnknown());
@@ -514,9 +520,7 @@ Pointer::computeOffsetForComparison(const ASTContext &ASTCtx) const {
case Storage::String:
return reinterpret_cast<uintptr_t>(Str.getLiteral()) + Offset;
case Storage::Opaque:
- if (auto O = Opaque.computeLayoutOffset(ASTCtx))
- return *O + Offset;
- return std::nullopt;
+ return computeLayoutOffset(ASTCtx);
}
auto getTypeSize = [&](QualType T) -> std::optional<size_t> {
@@ -597,8 +601,12 @@ Pointer::computeLayoutOffset(const ASTContext &ASTCtx) const {
case Storage::String:
return Offset * Str.getLiteral()->getCharByteWidth();
case Storage::Opaque:
- if (auto O = Opaque.computeLayoutOffset(ASTCtx))
- return *O + Offset;
+ if (auto O = Opaque.computeLayoutOffset(ASTCtx)) {
+ size_t TypeSize = 0;
+ if (QualType FT = Opaque.getFieldType(); validType(FT))
+ TypeSize = ASTCtx.getTypeSizeInChars(FT).getQuantity();
+ return *O + (Offset * TypeSize);
+ }
return std::nullopt;
}
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 5b43df9db49c4..0953c897b2564 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -985,6 +985,12 @@ class Pointer {
unsigned getNumElems() const {
if (isStringPointer())
return Str.getLiteral()->getLength() + 1;
+ if (isOpaquePointer()) {
+ const ArrayType *AT =
+ Opaque.getSurroundingArray()->getAsArrayTypeUnsafe();
+ if (const auto *CAT = dyn_cast_if_present<ConstantArrayType>(AT))
+ return CAT->getZExtSize();
+ }
if (!isBlockPointer())
return ~0u;
return view().getNumElems();
@@ -1008,6 +1014,11 @@ class Pointer {
int64_t getIndex() const {
if (isStringPointer())
return Offset;
+ if (isOpaquePointer()) {
+ if (Opaque.isArrayElement())
+ return Opaque.Path[Opaque.PathLength - 1].Index;
+ return 0;
+ }
if (!isBlockPointer())
return getIntegerRepresentation();
diff --git a/clang/test/AST/ByteCode/cxx11.cpp b/clang/test/AST/ByteCode/cxx11.cpp
index f2e0ce45154de..b5b01949d7098 100644
--- a/clang/test/AST/ByteCode/cxx11.cpp
+++ b/clang/test/AST/ByteCode/cxx11.cpp
@@ -509,3 +509,22 @@ namespace ConstexprForRangeVar {
// both-note-re {{read of implicit variable '__begin{{[0-9]+}}' of range-based 'for' loop is not allowed in a constant expression}}
}
}
+
+namespace OpaqueArrayIndex {
+
+ int n;
+ int a[1];
+ constexpr int *r = &(&n + 1)[(unsigned __int128)-1]; // both-error {{constant expression}} \
+ // both-note {{456 of non-array object}}
+ constexpr int *r2 = &a[(unsigned __int128)-1]; // both-error {{constant expression}} \
+ // both-note {{455 of array of 1 element}}
+ constexpr int *r3 = &a[2]; // both-error {{constant expression}} \
+ // both-note {{2 of array of 1 element}}
+ constexpr int *r4 = &a[-1]; // both-error {{constant expression}} \
+ // both-note {{-1 of array of 1 element}}
+ constexpr int *q = (&n + 1) - (unsigned __int128)-1; // both-error {{constant expression}} \
+ // both-note {{cannot refer to element -3402}}
+ constexpr int *f = &a[0] + 1 + (unsigned long)-1; // both-error {{constant expression}} \
+ // both-note {{cannot refer to element 1844}}
+
+}
More information about the cfe-commits
mailing list