[clang][bytecode] Call CheckStore() before activating pointers #165235
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
We used to do this the other way around to work around an awkwardness with CheckStore, namely that we shouldn't check pointers for being activated when activating them. Add a parameter to CheckStore instead and call CheckStore() _before_ activating and initializing the pointers in the respective opcode implementations. Fixes llvm#164975
llvmbot added clang 
Member
| @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesWe used to do this the other way around to work around an awkwardness with CheckStore, namely that we shouldn't check pointers for being activated when activating them. Add a parameter to CheckStore instead and call CheckStore() before activating and initializing the pointers in the respective opcode implementations. Fixes #164975 Full diff: https://github.com/llvm/llvm-project/pull/165235.diff 3 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 910868b27f48e..0e2ba3c5e82d2 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -870,7 +870,8 @@ bool CheckFinalLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { return true; } -bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { +bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr, + bool WillBeActivated) { if (!Ptr.isBlockPointer() || Ptr.isZero()) return false; @@ -885,7 +886,7 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { return false; if (!CheckRange(S, OpPC, Ptr, AK_Assign)) return false; - if (!CheckActive(S, OpPC, Ptr, AK_Assign)) + if (!WillBeActivated && !CheckActive(S, OpPC, Ptr, AK_Assign)) return false; if (!CheckGlobal(S, OpPC, Ptr)) return false; diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 89f6fbefb1907..43c3ab76b15f9 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -75,7 +75,8 @@ bool CheckGlobalLoad(InterpState &S, CodePtr OpPC, const Block *B); bool CheckLocalLoad(InterpState &S, CodePtr OpPC, const Block *B); /// Checks if a value can be stored in a block. -bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr); +bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr, + bool WillBeActivated = false); /// Checks if a value can be initialized. bool CheckInit(InterpState &S, CodePtr OpPC, const Pointer &Ptr); @@ -1977,13 +1978,12 @@ bool StoreActivate(InterpState &S, CodePtr OpPC) { const T &Value = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.peek<Pointer>(); + if (!CheckStore(S, OpPC, Ptr, /*WilLBeActivated=*/true)) + return false; if (Ptr.canBeInitialized()) { Ptr.initialize(); Ptr.activate(); } - - if (!CheckStore(S, OpPC, Ptr)) - return false; Ptr.deref<T>() = Value; return true; } @@ -1993,12 +1993,12 @@ bool StoreActivatePop(InterpState &S, CodePtr OpPC) { const T &Value = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.pop<Pointer>(); + if (!CheckStore(S, OpPC, Ptr, /*WilLBeActivated=*/true)) + return false; if (Ptr.canBeInitialized()) { Ptr.initialize(); Ptr.activate(); } - if (!CheckStore(S, OpPC, Ptr)) - return false; Ptr.deref<T>() = Value; return true; } @@ -2007,7 +2007,8 @@ template <PrimType Name, class T = typename PrimConv<Name>::T> bool StoreBitField(InterpState &S, CodePtr OpPC) { const T &Value = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.peek<Pointer>(); - if (!CheckStore(S, OpPC, Ptr)) + + if (!CheckStore(S, OpPC, Ptr, /*WilLBeActivated=*/true)) return false; if (Ptr.canBeInitialized()) Ptr.initialize(); @@ -2037,12 +2038,13 @@ template <PrimType Name, class T = typename PrimConv<Name>::T> bool StoreBitFieldActivate(InterpState &S, CodePtr OpPC) { const T &Value = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.peek<Pointer>(); + + if (!CheckStore(S, OpPC, Ptr, /*WilLBeActivated=*/true)) + return false; if (Ptr.canBeInitialized()) { Ptr.initialize(); Ptr.activate(); } - if (!CheckStore(S, OpPC, Ptr)) - return false; if (const auto *FD = Ptr.getField()) Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue()); else @@ -2055,12 +2057,12 @@ bool StoreBitFieldActivatePop(InterpState &S, CodePtr OpPC) { const T &Value = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.pop<Pointer>(); + if (!CheckStore(S, OpPC, Ptr, /*WillBeActivated=*/true)) + return false; if (Ptr.canBeInitialized()) { Ptr.initialize(); Ptr.activate(); } - if (!CheckStore(S, OpPC, Ptr)) - return false; if (const auto *FD = Ptr.getField()) Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue()); else diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp index 6bccbda23f468..414070417a02a 100644 --- a/clang/test/AST/ByteCode/unions.cpp +++ b/clang/test/AST/ByteCode/unions.cpp @@ -977,4 +977,13 @@ namespace UnionMemberOnePastEnd { } static_assert(!b()); } + +namespace ActicvateInvalidPtr { + constexpr void bar() { // both-error {{never produces a constant expression}} + union { + int a[1]; + } foo; + foo.a[1] = 0; // both-note {{assignment to dereferenced one-past-the-end pointer}} + } +} #endif |
Collaborator
| LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/24576 Here is the relevant piece of the build log for the reference |
Collaborator
| LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/15023 Here is the relevant piece of the build log for the reference |
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
…165235) We used to do this the other way around to work around an awkwardness with CheckStore, namely that we shouldn't check pointers for being activated when activating them. Add a parameter to CheckStore instead and call CheckStore() _before_ activating and initializing the pointers in the respective opcode implementations. Fixes llvm#164975
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
…165235) We used to do this the other way around to work around an awkwardness with CheckStore, namely that we shouldn't check pointers for being activated when activating them. Add a parameter to CheckStore instead and call CheckStore() _before_ activating and initializing the pointers in the respective opcode implementations. Fixes llvm#164975
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
…165235) We used to do this the other way around to work around an awkwardness with CheckStore, namely that we shouldn't check pointers for being activated when activating them. Add a parameter to CheckStore instead and call CheckStore() _before_ activating and initializing the pointers in the respective opcode implementations. Fixes llvm#164975
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
We used to do this the other way around to work around an awkwardness with CheckStore, namely that we shouldn't check pointers for being activated when activating them.
Add a parameter to CheckStore instead and call CheckStore() before activating and initializing the pointers in the respective opcode implementations.
Fixes #164975