On Fri, 7 Aug 2026 17:37:30 GMT, Matias Saavedra Silva <[email protected]> wrote:
>> [JDK-8376522](https://bugs.openjdk.org/browse/JDK-8376522) indicates that >> the thisUninit should be applied even when uninitializedThis is on the >> stack. Previous implementations failed to do this, so this patch properly >> enforces this spec change. >> >> Now that both the stack and locals must be considered when applying >> flagThisUninit, the origin of the flag needs to be preserved, so the >> StackMapReader now tracks if the flag originated from locals or the stack. >> Since the stack is cleared between frames, only an uninitializedThis from >> locals is preserved. >> >> In addition to VM changes, the ClassFile API had to be updated as it only >> considered stack frame locals. The test case offered by @liach in the JBS >> issue has been adapted to a JASM test to verify the change. Verified with >> tier 1-5 tests. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Matias Saavedra Silva has updated the pull request incrementally with one > additional commit since the last revision: > > Cleanup and test case improvement The overall change looks right but the naming and comments will lead to confusion. A couple of suggestions for clearer names and comments attached. src/hotspot/share/classfile/stackMapTable.cpp line 212: > 210: #define CHECK_NT CHECK_(VerificationType::bogus_type()) > 211: > 212: VerificationType StackMapReader::parse_verification_type(u1* flags, bool > in_locals, TRAPS) { `in_locals` -> `parsing_locals`? A slightly bigger refactoring that would be clearer is: enum class ParseLoc : int { Stack, Locals }; VerificationType StackMapReader::parse_verification_type(u1* flags, ParseLoc parse, TRAPS) And calls would use `ParseLocation::Stack` or `::Locals`. src/hotspot/share/classfile/stackMapTable.cpp line 234: > 232: > 233: // An uninitializedThis in the locals array can be preserved > between > 234: // frames while uninitializedThis in the stack cannot as the stack > is cleared. This isn't accurate. Most frames preserve the locals but not all - ie: a `chop_frame` is an empty stack that removes the last `k` locals. src/hotspot/share/classfile/stackMapTable.cpp line 345: > 343: } > 344: > 345: u1 flags = (u1)_uninit_in_locals; `_uninit_in_locals` -> `_uninit_in_prev_frame_locals` ? src/hotspot/share/classfile/stackMapTable.hpp line 139: > 137: // An uninitializedThis in the locals array can be inherited by > 138: // subsequent frames while uninitializedThis in the stack will be > 139: // discarded as the stack is cleared between frames. A frame can overwrite the uninitializeThis in the locals using a sequence like: aconst_null astore 0; Better not to have close but slightly incorrect info in the comments here. Maybe more accurate to say: We track whether an uninitializedThis was in the previous frame's locals independently of the flags parameter as most StackMapTable frames reuse the same locals as the previous frame. Chop and Full frames need to handle this specially ------------- PR Review: https://git.openjdk.org/jdk/pull/32242#pullrequestreview-4885770886 PR Review Comment: https://git.openjdk.org/jdk/pull/32242#discussion_r3738235659 PR Review Comment: https://git.openjdk.org/jdk/pull/32242#discussion_r3738134516 PR Review Comment: https://git.openjdk.org/jdk/pull/32242#discussion_r3738185190 PR Review Comment: https://git.openjdk.org/jdk/pull/32242#discussion_r3738108913
