[codex] Implement frame-aware line instrumentation#49
Merged
hengyunabc merged 1 commit intoJun 7, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades @AtLine instrumentation to be frame-aware by analyzing ASM frames at each line marker, spilling/restoring the operand stack when needed, and introducing configurable behavior for duplicate line numbers. It also updates local-variable bindings to avoid reading slots that are not live/typed in the current frame, and adds targeted regression tests for stack/local handling and concurrency-related scenarios.
Changes:
- Add frame-aware line matching (ASM analysis) and stack spill/restore support for line callbacks.
- Introduce
LineModeandLineDuplicatePolicyand wire them through@AtLine/LineLocationMatcher. - Make
@Binding.LocalVars/@Binding.LocalVarNamesframe-aware to skip unreadable locals; add comprehensive regression tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| bytekit-core/src/test/java/com/alibaba/bytekit/asm/interceptor/AtLineFrameAwareTest.java | New regression tests covering stack preservation, frame-aware locals, duplicate-line policies, and concurrency scenarios. |
| bytekit-core/src/main/java/com/alibaba/bytekit/utils/AsmOpUtils.java | Add frame-aware local variable filtering to avoid loading unreadable/uninitialized locals. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/MethodProcessor.java | Add initLineStackVariableNode helper for allocating locals used by line stack spilling. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/Location.java | Extend LineLocation with frame/stack metadata and a stack saver that spills/restores operand stack via temp locals. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/LineMode.java | New enum to select FRAME_AWARE vs LEGACY line matching behavior. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/LineDuplicatePolicy.java | New enum defining duplicate-line handling policies. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/LineLocationMatcher.java | Implement frame analysis, frame usability filtering, and duplicate-line policies; compute conservative analysis limits. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/filter/GroupLocationFilter.java | Refactor to extend AnyLocationFilter. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/filter/AnyLocationFilter.java | New “any-of” composite location filter implementation. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/location/filter/AllLocationFilter.java | New “all-of” composite location filter implementation. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/interceptor/annotation/AtLine.java | Add mode() and duplicatePolicy() options and pass them into LineLocationMatcher. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/binding/LocalVarsBinding.java | Use frame-aware valid-local filtering when the current location is a frame-aware LineLocation. |
| bytekit-core/src/main/java/com/alibaba/bytekit/asm/binding/LocalVarNamesBinding.java | Same as above for local variable names binding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+113
| boolean rejectAfterControlFlow = effectivePolicy == LineDuplicatePolicy.REJECT_AFTER_CONTROL_FLOW | ||
| && seenTargetLines.contains(lineNumberNode.line) | ||
| && Boolean.TRUE.equals(controlFlowAfterLine.get(lineNumberNode.line)); | ||
| if (!rejectAfterControlFlow | ||
| && allowByDuplicatePolicy(effectivePolicy, matchedLines, lineNumberNode.line, | ||
| lineBlockBoundary) | ||
| && locationFilter.allow(lineNumberNode, LocationType.LINE, false)) { | ||
| if (mode == LineMode.FRAME_AWARE) { | ||
| Frame<BasicValue> frame = frameOf(methodProcessor.getMethodNode(), lineFrames.frames, | ||
| lineNumberNode); | ||
| if (isFrameUsable(frame, lineFrames.precise)) { | ||
| locations.add(new LineLocation(lineNumberNode, lineNumberNode.line, frame, | ||
| locationIndex++)); | ||
| } | ||
| } else { | ||
| locations.add(new LineLocation(lineNumberNode, lineNumberNode.line)); | ||
| } | ||
| } | ||
| seenTargetLines.add(lineNumberNode.line); | ||
| controlFlowAfterLine.put(lineNumberNode.line, Boolean.FALSE); | ||
| } |
Comment on lines
+31
to
+33
| LineMode mode() default LineMode.FRAME_AWARE; | ||
|
|
||
| LineDuplicatePolicy duplicatePolicy() default LineDuplicatePolicy.DEFAULT; |
| } | ||
|
|
||
| start.countDown(); | ||
| done.await(); |
|
|
||
| private Object newInstance(String internalName, byte[] bytes) throws Exception { | ||
| Class<?> clazz = new TestClassLoader().define(internalName.replace('/', '.'), bytes); | ||
| return clazz.newInstance(); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
@AtLinematching with operand stack spill/restore for line callbacks.Root Cause
Line instrumentation could insert callbacks at source line markers without accounting for the current operand stack. That made some line locations unsafe when the original bytecode had live stack values at the line boundary, and it also made duplicate-line/control-flow cases ambiguous.
Validation
./mvnw -pl bytekit-core testgit diff --cached --checkbytekit-core:0.1.6:./mvnw -pl core -am -Dtest=EnhancerTest,WatchCommandTest,OgnlTest -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false -DskipITs test./mvnw -pl packaging -am -DskipTests packagewatch demo.MathGame primeFactors --line 51 '{params, localVarMap}' -x 2 -n 1hitAtLine:51and printed expected locals.