Repository navigation
Walk to an ancestor's constructor spec through JDK bytecode delegation - #433
Conversation
|
[Codex review] I checked the bytecode matcher’s accepted instruction sequence, exact overload lookup after delegation, argument mapping, and refusal of constructors with other work. The focused constructor tests and full verifier suite pass locally at this head ( |
|
[Claude] Thanks for checking. The two limits (the walk depends on the runtime JDK's bytecode, and ancestor parameter refinements are not checked) stay listed in the PR description and will go into the release notes when this ships. |
Add ConstructorDelegation, which reads a JDK class with ASM and reports whether a constructor's whole body is a call to a superclass constructor (aload_0, loads of its parameters or constants, invokespecial, return), and what it passes for each superclass parameter. Results are cached per constructor. Adds org.ow2.asm:asm 9.11 (BSD-3-Clause). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the hard-coded DELEGATING_JDK_CLASSES list: a constructor without
a spec is followed through JDK constructors that only delegate, mapping
each spec parameter back to the argument or constant passed for it, and
looking up each ancestor's spec by exact parameter types. The walk is
used both for super(...) in user constructors and for a direct
new XException(...): new RuntimeException("m") now gets
Throwable(String)'s state, and new ClassNotFoundException() gets
Throwable(Throwable)'s withThrowable state. Anything else falls back to
the default state as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Inherit a super constructor's state only when the constructor body is
just that super(...) call and the class has no instance initializers or
field initializers: anything after super(...) may change the state
(e.g. super(m); initCause(c);), which was missed, also before this
branch, for subclasses of RuntimeException.
- Pass constant arguments to the spec as literals of the refinement
language instead of variables named after their value.
- Keep the default value of the constructed class's own state sets that
the ancestor's spec does not mention (e.g. Stack's, after Vector()).
- Pass a method parameter given to new by its name when it has no
instance yet, as for other calls.
- Leave anonymous classes (new C() { ... }) to the default state.
- ConstructorDelegation: say that results depend on the JDK the verifier
runs on; record cache key; check for a missing superclass once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test suite only checks where errors are reported, and main reports the same lines of throwable_jdk_subclass_error with an unknown state. Assert that the state found is withThrowable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A variable passed to new is referred to only through its current instance. Its raw name made the state follow a later assignment (new PrinterStateReasons(n); n = 5;), and a lambda parameter reached Z3 with the wrong sort; with no instance the value is unknown and the walk stops. - Skip a spec whose to-state mentions old(this) (it leaves another state set unchanged): a new object has no old state, and the delegated call failed with "Ghost 'old' could not be found". - Literals: booleans and ints only; extract the shared CtLiteral case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With Sub(boolean y, boolean x) { super(y, x); } for Base(boolean x, boolean y),
replacing x by y and then y by x turned both into x, so a state that mentions
both parameters was wrong.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
636dc4d to
4c8da01
Compare
Closes #367.
Problem
A JDK exception created directly gets no state from the
Throwablespec, so a correctinitCauseis reported:new IllegalStateException("failed")gives the same error. The real bug in this pattern, in scijava-common'sCommandInfo(fixed in a6fd9cf), was reported, but only because the state was unknown. The actual cause is thatClassNotFoundException()already sets a (null) cause:Cause
A constructor with no spec of its own gets the default state. #354 lets a lookup walk up to an ancestor's spec, but only:
Exception,RuntimeException,Error);super(...)in user constructors;It also looked only at the first statement. So
super(m); initCause(c);in aRuntimeExceptionsubclass inheritedThrowable(String)'s state, and a secondinitCausewas missed.Fix
ConstructorDelegationreads a JDK class with ASM. It accepts a constructor only when its whole body has this shape:this(...), or a branch. For each superclass parameter it records what is passed: one of the constructor's own parameters (any order or subset), or a constant. Int constants passed for a boolean, int, short or byte parameter keep their value. Other constants (null, strings, long/float/double, chars) have no known value, in bytecode and in source alike. Results are cached per constructor.AuxStateHandlerreplacesDELEGATING_JDK_CLASSESwith a walk through delegating constructors:getFunction's fallback to another overload with the same number of parameters is not used.tostate mentions a parameter whose value is not known (for example, a parameter that has no instance yet), the walk is refused.tomentionsold(this)is skipped. That happens when a constructor spec leaves another state set unchanged. Calling such a constructor directly already fails on main withGhost 'old' could not be found; that pre-existing bug is left for a separate issue.todoes not mention keep their default value. For example,Stack()runsVector(), and Stack's own state set still starts empty.new X(...)whenXhas no spec of its own (except anonymous classes), and to a user constructor whose body is onlysuper(...), with no instance or field initializers. That is the source-level version of the bytecode rule. Anything else gets the default state, as before.ClassNotFoundException()isaload_0; aconst_null; checkcast Throwable; invokespecial ReflectiveOperationException(Throwable), so it reachesThrowable(Throwable)and the object iswithThrowable.Not covered:
this(...)chains inside a binary class, and constructors that do anything besides delegate.newsite. Only its state is applied, and nothing was checked there before.fillInStackTrace) are not considered, as for any spec. So a subclass of a newly walked class such asIOException, whose override sets the cause, is now missed. Main reported it only because the state was unknown. A call toinitCauseinside the override is still reported.Tests
classes/throwable_jdk_subclass_correct:new RuntimeException(msg),new IllegalStateException(msg)andnew Exception()followed byinitCause, and anIOExceptionsubclass callingsuper(msg). Main reports 3 false errors.classes/throwable_jdk_subclass_error:new ClassNotFoundException(),new RuntimeException(msg, cause)and aClassNotFoundExceptionsubclass callingsuper(String), each followed byinitCause. It also hasMissingResourceException, which is not delegating and so gets the default state.TestDelegatedConstructorStateschecks that the state found on the first three iswithThrowable; main reports the same lines with an unknown state.classes/throwable_subclass_more_than_super_error:super(m); initCause(c);, an implicitsuper()followed byinitCause, and an instance initializer that sets the cause. Main misses the first and third.classes/delegated_constant_arguments_error:LinkedHashSet()reachesHashSet(16, .75f, true), and16andtruereach the spec as literals.classes/delegated_subclass_sets_error:Stackkeeps its own default state afterVector().classes/delegated_argument_reassigned_error: a variable passed tonewand assigned before or after the call, and a lambda parameter.classes/delegated_spec_keeping_other_sets_correct: a delegated call to a constructor spec that mentionsold(this)gets the default state, without an error.classes/super_swapped_arguments_correct/_error:Sub(boolean y, boolean x) { super(y, x); }forBase(boolean x, boolean y), whose state mentions both parameters. They are replaced at once; replacing one after the other turned both intox(also wrong on main).ConstructorDelegationTest: unit tests for the bytecode matcher.mvn test: 532 tests, 0 failures (rebased on main at 92963a7).Real-file results
CommandInfo.java(d9dce68) with the Throwable spec:found !#fresh but expected noThrowable(cnfe).found !#fresh && withThrowable(cnfe) but expected noThrowable(cnfe)at line 304.throw new ClassNotFoundException(null, exc)) passes on both.Dependency
org.ow2.asm:asm9.11, licence BSD-3-Clause, a single jar with no transitive dependencies. Spoon 11.5.0 does not bring ASM in.🤖 Generated with Claude Code