https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/147550
Instead, return the new Pointer. This was weird before because some callers had to get the value from the stack again to perform further operations. >From e0dc4095477f345470a37e3b826385822448078e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Tue, 8 Jul 2025 10:53:39 +0200 Subject: [PATCH] [clang][bytecode][NFC] Don't push anything in OffsetHelper Instead, return the new Pointer. This was weird before because some callers had to get the value from the stack again to perform further operations. --- clang/lib/AST/ByteCode/Interp.h | 105 ++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 45 deletions(-) diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 85a07abb5ed01..9d17f96c97c85 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2066,34 +2066,32 @@ inline bool CastMemberPtrPtr(InterpState &S, CodePtr OpPC) { //===----------------------------------------------------------------------===// template <class T, ArithOp Op> -bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset, - const Pointer &Ptr, bool IsPointerArith = false) { +std::optional<Pointer> OffsetHelper(InterpState &S, CodePtr OpPC, + const T &Offset, const Pointer &Ptr, + bool IsPointerArith = false) { // A zero offset does not change the pointer. - if (Offset.isZero()) { - S.Stk.push<Pointer>(Ptr); - return true; - } + if (Offset.isZero()) + return Ptr; if (IsPointerArith && !CheckNull(S, OpPC, Ptr, CSK_ArrayIndex)) { // The CheckNull will have emitted a note already, but we only // abort in C++, since this is fine in C. if (S.getLangOpts().CPlusPlus) - return false; + return std::nullopt; } // Arrays of unknown bounds cannot have pointers into them. if (!CheckArray(S, OpPC, Ptr)) - return false; + return std::nullopt; // This is much simpler for integral pointers, so handle them first. if (Ptr.isIntegralPointer()) { uint64_t V = Ptr.getIntegerRepresentation(); uint64_t O = static_cast<uint64_t>(Offset) * Ptr.elemSize(); if constexpr (Op == ArithOp::Add) - S.Stk.push<Pointer>(V + O, Ptr.asIntPointer().Desc); + return Pointer(V + O, Ptr.asIntPointer().Desc); else - S.Stk.push<Pointer>(V - O, Ptr.asIntPointer().Desc); - return true; + return Pointer(V - O, Ptr.asIntPointer().Desc); } else if (Ptr.isFunctionPointer()) { uint64_t O = static_cast<uint64_t>(Offset); uint64_t N; @@ -2105,8 +2103,7 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset, if (N > 1) S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index) << N << /*non-array*/ true << 0; - S.Stk.push<Pointer>(Ptr.asFunctionPointer().getFunction(), N); - return true; + return Pointer(Ptr.asFunctionPointer().getFunction(), N); } assert(Ptr.isBlockPointer()); @@ -2156,7 +2153,7 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset, } if (Invalid && S.getLangOpts().CPlusPlus) - return false; + return std::nullopt; // Offset is valid - compute it on unsigned. int64_t WideIndex = static_cast<int64_t>(Index); @@ -2172,15 +2169,11 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset, // we don't get here. if (Result == 0 && Ptr.isOnePastEnd()) { if (Ptr.getFieldDesc()->isArray()) - S.Stk.push<Pointer>(Ptr.atIndex(0)); - else - S.Stk.push<Pointer>(Ptr.asBlockPointer().Pointee, - Ptr.asBlockPointer().Base); - return true; + return Ptr.atIndex(0); + return Pointer(Ptr.asBlockPointer().Pointee, Ptr.asBlockPointer().Base); } - S.Stk.push<Pointer>(Ptr.atIndex(static_cast<uint64_t>(Result))); - return true; + return Ptr.atIndex(static_cast<uint64_t>(Result)); } template <PrimType Name, class T = typename PrimConv<Name>::T> @@ -2189,16 +2182,26 @@ bool AddOffset(InterpState &S, CodePtr OpPC) { Pointer Ptr = S.Stk.pop<Pointer>(); if (Ptr.isBlockPointer()) Ptr = Ptr.expand(); - return OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr, - /*IsPointerArith=*/true); + + if (std::optional<Pointer> Result = OffsetHelper<T, ArithOp::Add>( + S, OpPC, Offset, Ptr, /*IsPointerArith=*/true)) { + S.Stk.push<Pointer>(*Result); + return true; + } + return false; } template <PrimType Name, class T = typename PrimConv<Name>::T> bool SubOffset(InterpState &S, CodePtr OpPC) { const T &Offset = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.pop<Pointer>(); - return OffsetHelper<T, ArithOp::Sub>(S, OpPC, Offset, Ptr, - /*IsPointerArith=*/true); + + if (std::optional<Pointer> Result = OffsetHelper<T, ArithOp::Sub>( + S, OpPC, Offset, Ptr, /*IsPointerArith=*/true)) { + S.Stk.push<Pointer>(*Result); + return true; + } + return false; } template <ArithOp Op> @@ -2218,12 +2221,13 @@ static inline bool IncDecPtrHelper(InterpState &S, CodePtr OpPC, // Now the current Ptr again and a constant 1. OneT One = OneT::from(1); - if (!OffsetHelper<OneT, Op>(S, OpPC, One, P, /*IsPointerArith=*/true)) - return false; - - // Store the new value. - Ptr.deref<Pointer>() = S.Stk.pop<Pointer>(); - return true; + if (std::optional<Pointer> Result = + OffsetHelper<OneT, Op>(S, OpPC, One, P, /*IsPointerArith=*/true)) { + // Store the new value. + Ptr.deref<Pointer>() = *Result; + return true; + } + return false; } static inline bool IncPtr(InterpState &S, CodePtr OpPC) { @@ -2925,16 +2929,22 @@ inline bool ArrayElemPtr(InterpState &S, CodePtr OpPC) { if (Offset.isZero()) { if (Ptr.getFieldDesc()->isArray() && Ptr.getIndex() == 0) { - S.Stk.push<Pointer>(Ptr.atIndex(0)); - } else { - S.Stk.push<Pointer>(Ptr); + S.Stk.push<Pointer>(Ptr.atIndex(0).narrow()); + return true; } - } else { - if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) - return false; + S.Stk.push<Pointer>(Ptr); + return true; + } + + assert(!Offset.isZero()); + + if (std::optional<Pointer> Result = + OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) { + S.Stk.push<Pointer>(Result->narrow()); + return true; } - return NarrowPtr(S, OpPC); + return false; } template <PrimType Name, class T = typename PrimConv<Name>::T> @@ -2949,16 +2959,21 @@ inline bool ArrayElemPtrPop(InterpState &S, CodePtr OpPC) { if (Offset.isZero()) { if (Ptr.getFieldDesc()->isArray() && Ptr.getIndex() == 0) { - S.Stk.push<Pointer>(Ptr.atIndex(0)); - } else { - S.Stk.push<Pointer>(Ptr); + S.Stk.push<Pointer>(Ptr.atIndex(0).narrow()); + return true; } - } else { - if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) - return false; + S.Stk.push<Pointer>(Ptr); + return true; } - return NarrowPtr(S, OpPC); + assert(!Offset.isZero()); + + if (std::optional<Pointer> Result = + OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) { + S.Stk.push<Pointer>(Result->narrow()); + return true; + } + return false; } template <PrimType Name, class T = typename PrimConv<Name>::T> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits