Skip to content

Add regression test for handler(Exception+) advice woven into catch(Exception|LinkageError) multi-catch (#368) - #369

Open
robinjohri wants to merge 1 commit into
eclipse-aspectj:masterfrom
robinjohri:fix/gh368-handler-multicatch-regression-test
Open

robinjohri wants to merge 1 commit into
eclipse-aspectj:masterfrom
robinjohri:fix/gh368-handler-multicatch-regression-test

Conversation

@robinjohri

Copy link
Copy Markdown

Summary

Fixes #368.

Adds a regression test for a VerifyError that occurred when handler(Exception+) (or any handler(SomeType+)) advice was woven into a Java 7+ multi-catch clause mixing an Exception subtype with a non-Exception Throwable subtype, e.g.:

try {
    riskyCall();
} catch (Exception | LinkageError e) {
    // ...
}

In that case javac types e as the least-upper-bound java.lang.Throwable (not Exception), and the woven advice-dispatch code used to pass it on without a checkcast, so the woven class failed the JVM's bytecode verifier at class-load time with:

java.lang.VerifyError: Bad type on operand stack
Reason:
  Type 'java/lang/Throwable' (current frame, stack[2]) is not assignable to 'java/lang/Exception'

Why a test-only PR

I bisected this across released versions and confirmed it's already fixed:

Version Result
1.9.21, 1.9.21.1 Crashes
1.9.21.2 and every version since (up to current 1.9.25.1) Fixed

The fix landed silently between 1.9.21.1 and 1.9.21.2 (release notes for 1.9.22 say "No major bug fixes"), and I couldn't find any existing test covering this exact scenario, so nothing currently guards against a regression. See #368 for the full root-cause writeup and version bisection.

What this PR adds

  • tests/bugs1925/github_368/: a minimal Worker/LoggingAspect/Application reproduction, woven via load-time weaving (mirrors the existing bugs1921/github_279 LTW test pattern, since this only reproduces under a forked JVM + java agent).
  • A new ajc-test entry in ajc1925.xml and a corresponding test method in Bugs1925Tests.java.

Verified locally (JDK 17, mvn -pl tests test -Dtest=Bugs1925Tests): all 9 tests in the suite pass, including the new one, confirming current master is not affected.

Test plan

  • Ran Bugs1925Tests locally against current master — all pass, including the new test.
  • Independently verified (outside this repo, against the published aspectjweaver jars) that the same aspect/target reproduces the VerifyError on 1.9.21 and 1.9.21.1, and passes on 1.9.21.2+.

…xception|LinkageError) multi-catch (eclipse-aspectj#368)

Weaving handler(Exception+) advice into a Java 7+ multi-catch clause that
mixes an Exception subtype with a non-Exception Throwable subtype (e.g.
catch (Exception | LinkageError e)) used to produce invalid bytecode: the
caught variable's compile-time type is the least-upper-bound Throwable,
not Exception, and the woven advice-dispatch code passed it on without a
checkcast, causing a VerifyError at class-load time.

This was already fixed silently between 1.9.21.1 and 1.9.21.2 (no release
notes entry, no test coverage), so this adds a load-time-weaving
regression test to guard the fixed behaviour against future regressions.

Fixes eclipse-aspectj#368

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant