Skip to content

feat(agent): ClassFile API backend ordinary args + AnyType[] + review-nit cleanup - #948

Draft
jbachorik wants to merge 5 commits into
developfrom
muse/classfileapi-return-duration
Draft

feat(agent): ClassFile API backend ordinary args + AnyType[] + review-nit cleanup#948
jbachorik wants to merge 5 commits into
developfrom
muse/classfileapi-return-duration

Conversation

@jbachorik

@jbachorik jbachorik commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

What does this change do?

Follow-up to #843 (the ClassFile API backend parity work, squash-merged into develop as 9f39a250). Closes the parity gaps #843 left open and sweeps the open code-quality review findings:

  • ENTRY/RETURN ordinary method arguments — handlers declaring ordinary enclosing-method args (typed or AnyType[]) were silently skipped by the ClassFile API backend. Added fixed-local-slot loading (methodArgSlot) threaded via new ENTRY/RETURN emitProbeCall overloads.
  • AnyType[] aggregate packaging (ENTRY/RETURN/CALL) — added emitAnyTypeArray (anewarray + per-element load/box/arrayStore, mirroring ASM AnyTypeArgProvider), detected via the pre-replacement descriptor; plus an AnyType[] short-circuit in canEmitCallProbe (without it sameStackType rejected the Object[] aggregate and the probe was silently skipped).
  • ClassFilter.isSubTypeOf — folded the JLS-dead if (cache == null) guard into the ci == null conservative no-match path (ClassCache.getInstance() uses the holder idiom, never null under normal class-init semantics).
  • ClassFileApiFeatureSmokeTest — referenced each previously-unused captured probe param via BTraceUtils.str() / array.length, clearing the "useless parameter" findings and strengthening the smoke assertions.
  • canEmitCallProbe — removed an unread staticCall local.
  • Plan doc at docs/superpowers/plans/2026-07-06-classfileapi-remaining-gaps.md documenting the gaps, the ASM parity references, and per-task status.

Related issue

N/A — follow-up to #843.

Scope and compatibility

  • I identified the affected module(s) and kept unrelated changes out of this PR.
  • This preserves the supported Java/runtime compatibility tiers, or the change is documented below.
  • This does not change the masked-JAR layout, class-loader boundary, or wire protocol.
  • If it does, I updated the relevant architecture documentation and verification plan.

Compatibility or migration notes: None. ClassFile API backend changes only affect Java 26+ (class-file major ≥ 70) instrumentation; the ASM backend path is untouched. No API/CLI/protocol/packaging change.

Testing

  • Unit tests
  • Integration tests (if applicable)
  • spotlessCheck
  • Documentation/link or sample verification (if applicable)

Commands and results:

# Unit + verifier (JDK 26)
JAVA_HOME=$HOME/.sdkman/candidates/java/26-tem GRADLE_USER_HOME=$(pwd)/.gradle-user \
  ./gradlew :btrace-agent:classFileApiBackendTest
# -> 121 tests, 0 failures, 0 skipped

# Full agent suite
GRADLE_USER_HOME=$(pwd)/.gradle-user ./gradlew :btrace-agent:test
# -> BUILD SUCCESSFUL

# Formatting
JAVA_HOME=$HOME/.sdkman/candidates/java/26-tem GRADLE_USER_HOME=$(pwd)/.gradle-user \
  ./gradlew :btrace-agent:spotlessApply :integration-tests:spotlessApply
# -> BUILD SUCCESSFUL

# Integration matrix (-Pintegration test), dist btrace.jar v3.0.0-SNAPSHOT
# JDK 8/11/17(zulu)/21/26 -> BUILD SUCCESSFUL
# BTraceFunctionalTests.testOnMethod() PASSED on 8/11/17/21 (ClassCache-init race path)
# ClassFileApiTests smoke (Task 8 str() changes) PASSED on JDK 26
# JDK 17.0.19-sem: 6 PreparedModeAuthenticationFunctionalTest failures with
#   AttachNotSupportedException — JDK-build attach-infra issue, not this branch
#   (zulu-17.0.18 passes the same auth tests).

Documentation and release impact

  • User-facing documentation is updated, or no documentation change is needed.
  • Release notes/changelog are updated, or no release-note entry is needed.
  • Samples, distribution contents, or published coordinates are updated if affected.
  • This change is safe to merge independently of a release, or the dependency is explained below.

Release notes / follow-up work: No user-facing behavior change beyond fixing silently-skipped probes on Java 26+. Known out-of-scope edge case: sub-int (byte/short/char/boolean) AnyType[] elements box via the existing boxPrimitiveReturn (consistent with the @Return boxing and the JVMS verifier's int-collapse); not exercised by tests (ASM AnytypeArgs likewise covers only String/long/String[]/int[]). A deferred Phase-1 shared-validation refactor (the canEmit* duplication) is intentionally NOT included — pure refactor, no functional gain, integration just proved the current code works.

Final checklist

  • I reviewed the complete diff and removed unrelated changes.
  • New or changed behavior has appropriate tests, or the reason for not adding them is explained above.
  • User-facing behavior, APIs, samples, and documentation are consistent with this change.
  • I did not include generated build output, local configuration, credentials, or other accidental files.

This change is Reviewable

…I backend

ENTRY/RETURN handlers declaring ordinary enclosing-method arguments (typed
or AnyType[]) were silently skipped by the ClassFile API backend, and CALL
AnyType[] aggregate packaging was missing.

- Thread methodArgTypes (from methodDesc) into emitProbeCall via new
  ENTRY/RETURN convenience overloads.
- Typed ordinary method args loaded from fixed local slots (methodArgSlot).
- AnyType[] aggregate packaging: anewarray + per-element load/box/arrayStore,
  mirroring ASM AnyTypeArgProvider; detected via the pre-replace descriptor.
- AnyType[] short-circuit in canEmitCallProbe (sameStackType otherwise
  rejected the Object[] aggregate and the probe was silently skipped).
- Drop an unread staticCall local in canEmitCallProbe.

7 new unit tests each assert the JVM verifier accepts the instrumented bytes
(defineClass via an isolated ClassLoader), plus invokedynamic descriptor and
anewarray presence for the aggregate cases.
ClassCache.getInstance() uses the holder idiom (static final in a nested
class), so the cache is never null under normal JLS class-init semantics.
Fold the JLS-dead 'if (cache == null)' guard into the 'ci == null'
conservative no-match path.
…oke test

The handler params (value, array, lock, key, set, exception) drove
instrumentation but were unused in the body, tripping code-quality
'useless parameter' findings. Reference each via BTraceUtils.str() (Object
params) / array.length so the smoke test also asserts the captured value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant