Author: Marlus Cadanus da Costa Date: 2026-07-02T15:57:49-07:00 New Revision: 7b11c7ce86aafbf4782ceb31543e27dc69617a7a
URL: https://github.com/llvm/llvm-project/commit/7b11c7ce86aafbf4782ceb31543e27dc69617a7a DIFF: https://github.com/llvm/llvm-project/commit/7b11c7ce86aafbf4782ceb31543e27dc69617a7a.diff LOG: [Clang] Fix offsetof sign-extending unsigned array indices >= 128 (#204139) When evaluating __builtin_offsetof with an unsigned integer array index (e.g. uint8_t, uint16_t) whose value has the high bit set, Clang was calling getSExtValue() on the APSInt index, which sign-extends the value and produces a large bogus offset. Fix this to use the correct kind of extension to extend smaller values, and to check for overflow in conversions of larger values. Fixes #199319 AI Tool Use: GitHub Copilot (Claude Sonnet 4.6) was used to assist in identifying the root cause of the bug in ExprConstant.cpp and drafting the fix. The fix was reviewed, tested, and validated manually. Added: clang/test/Sema/offsetof-unsigned-index.c Modified: clang/docs/ReleaseNotes.md clang/include/clang/Basic/DiagnosticASTKinds.td clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Interp.h clang/lib/AST/ByteCode/InterpBuiltin.cpp clang/lib/AST/ByteCode/Opcodes.td clang/lib/AST/ExprConstant.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 6788763d0687e..3e6c10666ca16 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -759,6 +759,9 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the calling `__builtin_bit_cast`. (#GH200112) - Clang now SFINAE friendly when the ``__reference_meows_from_temporary`` builtins should SFINAE friendly when the 1st type is not a reference type. (#GH206524) +- Fixed `__builtin_offsetof` incorrectly sign-extending unsigned array indices + with the high bit set (e.g. `uint8_t` values >= 128), which produced wrong + offset values in constant expressions. (#GH199319) #### Bug Fixes to Attribute Support diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td index bde418695f647..f86f0157b2b1f 100644 --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -24,6 +24,8 @@ def note_constexpr_invalid_downcast : Note< "cannot cast object of dynamic type %0 to type %1">; def note_constexpr_overflow : Note< "value %0 is outside the range of representable values of type %1">; +def note_constexpr_offsetof_overflow : Note< + "overflow in offsetof">; def note_constexpr_negative_shift : Note<"negative shift count %0">; def note_constexpr_large_shift : Note< "shift count %0 >= width of type %1 (%2 bit%s2)">; diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 95f9a2f507335..2458860f44eb5 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -3764,6 +3764,13 @@ bool Compiler<Emitter>::VisitOffsetOfExpr(const OffsetOfExpr *E) { continue; } + if (IndexT == PT_IntAP || IndexT == PT_IntAPS) { + if (!this->visit(ArrayIndexExpr)) + return false; + if (!this->emitCastNoOverflow(IndexT, E)) + return false; + continue; + } if (!this->visit(ArrayIndexExpr)) return false; // Cast to Sint64. diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 97d9e7cc14e32..84564d0a33cff 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2884,6 +2884,19 @@ bool CastAPS(InterpState &S, CodePtr OpPC, uint32_t BitWidth) { return true; } +// Cast an AP integer to Sint64, failing constant evaluation if the value is +// negative or too large to fit (i.e. truncation would change the value). +template <PrimType Name, class T = typename PrimConv<Name>::T> +bool CastNoOverflow(InterpState &S, CodePtr OpPC) { + T Source = S.Stk.pop<T>(); + APSInt Val = Source.toAPSInt(); + if (Val.isNegative() || Val.getActiveBits() > 63) + return Invalid(S, OpPC); + S.Stk.push<Integral<64, true>>( + Integral<64, true>::from((int64_t)Val.getZExtValue())); + return true; +} + template <PrimType Name, class T = typename PrimConv<Name>::T> bool CastIntegralFloating(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem, uint32_t FPOI) { diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp index bd35819aa4260..73672d1d99d2f 100644 --- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp +++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp @@ -6760,12 +6760,28 @@ bool InterpretOffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E, // When generating bytecode, we put all the index expressions as Sint64 on // the stack. int64_t Index = ArrayIndices[ArrayIndex]; + if (Index < 0) + return Invalid(S, OpPC); const ArrayType *AT = S.getASTContext().getAsArrayType(CurrentType); if (!AT) return false; CurrentType = AT->getElementType(); CharUnits ElementSize = S.getASTContext().getTypeSizeInChars(CurrentType); - Result += Index * ElementSize; + int64_t ElemSize = ElementSize.getQuantity(); + if (Index != 0 && ElemSize > llvm::maxIntN(64) / Index) { + S.FFDiag(S.Current->getLocation(OpPC), + diag::note_constexpr_offsetof_overflow) + << S.Current->getRange(OpPC); + return false; + } + int64_t Offset = Index * ElemSize; + if (Result.getQuantity() > llvm::maxIntN(64) - Offset) { + S.FFDiag(S.Current->getLocation(OpPC), + diag::note_constexpr_offsetof_overflow) + << S.Current->getRange(OpPC); + return false; + } + Result += CharUnits::fromQuantity(Offset); ++ArrayIndex; break; } diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index b6190554178f3..7cda124b40e24 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -748,6 +748,16 @@ def CastAPS : Opcode { let HasGroup = 1; } +def APOnlyTypeClass : TypeClass { + let Types = [IntAP, IntAPS]; +} + +// Cast from an AP integer type to Sint64, failing if the value doesn't fit. +def CastNoOverflow : Opcode { + let Types = [APOnlyTypeClass]; + let HasGroup = 1; +} + // Cast an integer to a floating type def CastIntegralFloating : Opcode { let Types = [AluTypeClass]; diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 5538dc49ba0a5..fb82e0addfdac 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -19456,7 +19456,19 @@ bool IntExprEvaluator::VisitOffsetOfExpr(const OffsetOfExpr *OOE) { return Error(OOE); CurrentType = AT->getElementType(); CharUnits ElementSize = Info.Ctx.getTypeSizeInChars(CurrentType); - Result += IdxResult.getSExtValue() * ElementSize; + // Reject negative indices, indices too large to fit in int64_t, + // and overflow in the offset computation. + if (IdxResult.isNegative() || IdxResult.getActiveBits() > 63) + return Error(OOE); + int64_t IdxVal = IdxResult.getExtValue(); + int64_t ElemSize = ElementSize.getQuantity(); + if (IdxVal != 0 && + ElemSize > std::numeric_limits<int64_t>::max() / IdxVal) + return Error(OOE, diag::note_constexpr_offsetof_overflow); + int64_t Offset = IdxVal * ElemSize; + if (Result.getQuantity() > std::numeric_limits<int64_t>::max() - Offset) + return Error(OOE, diag::note_constexpr_offsetof_overflow); + Result += CharUnits::fromQuantity(Offset); break; } diff --git a/clang/test/Sema/offsetof-unsigned-index.c b/clang/test/Sema/offsetof-unsigned-index.c new file mode 100644 index 0000000000000..56396eb2a12e6 --- /dev/null +++ b/clang/test/Sema/offsetof-unsigned-index.c @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-linux-gnu +// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-linux-gnu -fexperimental-new-constant-interpreter + +// Test that offsetof correctly zero-extends unsigned array indices >= 128. +// Previously, Clang would sign-extend uint8_t indices >= 128, producing +// a large bogus offset value instead of the correct one. +// Also tests that negative indices and oversized __uint128_t indices are rejected. +// https://github.com/llvm/llvm-project/issues/199319 + +typedef unsigned char uint8_t; +typedef unsigned short uint16_t; +typedef unsigned long long uint64_t; + +struct MyStruct { + void *ptrs[256]; +}; + +// Unsigned indices that were previously sign-extended must be zero-extended. +_Static_assert(__builtin_offsetof(struct MyStruct, ptrs[(uint8_t)127]) == 127 * sizeof(void *), + "offsetof with uint8_t index 127 should be correct"); + +_Static_assert(__builtin_offsetof(struct MyStruct, ptrs[(uint8_t)128]) == 128 * sizeof(void *), + "offsetof with uint8_t index 128 should be correctly zero-extended, not sign-extended"); + +_Static_assert(__builtin_offsetof(struct MyStruct, ptrs[(uint8_t)255]) == 255 * sizeof(void *), + "offsetof with uint8_t index 255 should be correctly zero-extended, not sign-extended"); + +// uint16_t index: values >= 32768 were also affected by sign-extension. +struct BigStruct { char data[65536]; }; +_Static_assert(__builtin_offsetof(struct BigStruct, data[(uint16_t)32768]) == 32768, + "offsetof with uint16_t index 32768 should be correctly zero-extended"); + +// Negative indices must be rejected. +struct NegIdxStruct { int a; int x[1]; }; +_Static_assert(__builtin_offsetof(struct NegIdxStruct, x[-1]) == 0, ""); // expected-error {{not an integral constant expression}} + +// __uint128_t indices >= 0x8000000000000000 must be rejected. +_Static_assert(__builtin_offsetof(struct NegIdxStruct, x[(__uint128_t)0x8000000000000000]) == 0, ""); // expected-error {{not an integral constant expression}} + +// Small __uint128_t values that fit in int64_t must work correctly. +_Static_assert(__builtin_offsetof(struct NegIdxStruct, x[(__uint128_t)0]) == + __builtin_offsetof(struct NegIdxStruct, x), + "offsetof with __uint128_t index 0 should work"); +_Static_assert(__builtin_offsetof(struct NegIdxStruct, x[(__uint128_t)1]) == + __builtin_offsetof(struct NegIdxStruct, x) + sizeof(int), + "offsetof with __uint128_t index 1 should work"); + +// __uint128_t indices > UINT64_MAX must be rejected (e.g. adding another zero: +// old code would truncate 2^64 to 0 via PT_Uint64 cast, silently producing a +// wrong result instead of an error). +_Static_assert(__builtin_offsetof(struct NegIdxStruct, x[((__uint128_t)1 << 64)]) == 0, ""); // expected-error {{not an integral constant expression}} + +// A uint64_t index that causes index*sizeof(element) to overflow int64_t must +// be rejected. 4611686018427387904 * sizeof(short)==2 == 2^63 > INT64_MAX. +struct ShortArray { short data[2]; }; +_Static_assert(__builtin_offsetof(struct ShortArray, data[(uint64_t)4611686018427387904ULL]) == 0, ""); // expected-error {{not an integral constant expression}} expected-note {{overflow in offsetof}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
