Skip to content

Commit f7f366c

Browse files
CatarinaGamboaclaude
CatarinaGamboa
and
claude
authored
Walk to an ancestor's constructor spec through JDK bytecode delegation (#433)
Closes #367. ## Problem A JDK exception created directly gets no state from the `Throwable` spec, so a correct `initCause` is reported: ```java RuntimeException e = new RuntimeException("failed"); e.initCause(t); // State Refinement Error: found true but expected noThrowable(e) ``` `new IllegalStateException("failed")` gives the same error. The real bug in this pattern, in scijava-common's `CommandInfo` (fixed in a6fd9cf), was reported, but only because the state was unknown. The actual cause is that `ClassNotFoundException()` already sets a (null) cause: ```java final ClassNotFoundException cnfe = new ClassNotFoundException(); cnfe.initCause(exc); // IllegalStateException: Can't overwrite 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: - through a hard-coded list (`Exception`, `RuntimeException`, `Error`); - for `super(...)` in user constructors; - when the ancestor constructor has the same parameter types. It also looked only at the first statement. So `super(m); initCause(c);` in a `RuntimeException` subclass inherited `Throwable(String)`'s state, and a second `initCause` was missed. ## Fix - **`ConstructorDelegation`** reads a JDK class with ASM. It accepts a constructor only when its whole body has this shape: ``` aload_0 (load of a parameter | constant [checkcast after null])* invokespecial <direct superclass>.<init> return ``` Any other instruction rejects the constructor: a field write, a method call, `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. - **`AuxStateHandler`** replaces `DELEGATING_JDK_CLASSES` with a walk through delegating constructors: - The arguments are mapped at each step. - Each ancestor's spec is looked up by exact parameter types, so `getFunction`'s fallback to another overload with the same number of parameters is not used. - The spec's parameters are replaced by the values passed: a variable's current instance, or a literal for boolean and int constants. A later assignment to the variable does not change the state. If a `to` state mentions a parameter whose value is not known (for example, a parameter that has no instance yet), the walk is refused. - A spec whose `to` mentions `old(this)` is skipped. That happens when a constructor spec leaves another state set unchanged. Calling such a constructor directly already fails on main with `Ghost 'old' could not be found`; that pre-existing bug is left for a separate issue. - State sets of the constructed class that the ancestor's `to` does not mention keep their default value. For example, `Stack()` runs `Vector()`, and Stack's own state set still starts empty. - **Where it applies:** to `new X(...)` when `X` has no spec of its own (except anonymous classes), and to a user constructor whose body is only `super(...)`, 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()` is `aload_0; aconst_null; checkcast Throwable; invokespecial ReflectiveOperationException(Throwable)`, so it reaches `Throwable(Throwable)` and the object is `withThrowable`. Not covered: - **Only JDK classes are read**, through the platform class loader. The verifier runs without the program's classpath, and its own libraries may be different versions from the ones the program uses. Third-party intermediate classes stop the walk. - **The result depends on the JDK the verifier runs on**, which may not be the one the program targets. If that JDK changes a constructor body, or its class files are newer than ASM 9.11 can read, the walk stops without a warning and the object gets the default state, as on main. - **Not walked:** `this(...)` chains inside a binary class, and constructors that do anything besides delegate. - **Parameter refinements:** a walked spec's parameter refinements are not checked at the `new` site. Only its state is applied, and nothing was checked there before. - **State-change hint:** on the delegated path the hint metadata that names the call which set the state is not recorded, so state hints cannot point at that call. - **Overridden methods:** methods a subclass overrides and an ancestor's constructor calls (e.g. `fillInStackTrace`) are not considered, as for any spec. So a subclass of a newly walked class such as `IOException`, whose override sets the cause, is now missed. Main reported it only because the state was unknown. A call to `initCause` inside the override is still reported. - **Type variables:** the ancestor lookup compares exact type names, so a spec parameter written as a type variable does not match the erased bytecode type. The walk then stops and the state falls back to the default. ## Tests - **`classes/throwable_jdk_subclass_correct`:** `new RuntimeException(msg)`, `new IllegalStateException(msg)` and `new Exception()` followed by `initCause`, and an `IOException` subclass calling `super(msg)`. Main reports 3 false errors. - **`classes/throwable_jdk_subclass_error`:** `new ClassNotFoundException()`, `new RuntimeException(msg, cause)` and a `ClassNotFoundException` subclass calling `super(String)`, each followed by `initCause`. It also has `MissingResourceException`, which is not delegating and so gets the default state. `TestDelegatedConstructorStates` checks that the state found on the first three is `withThrowable`; main reports the same lines with an unknown state. - **`classes/throwable_subclass_more_than_super_error`:** `super(m); initCause(c);`, an implicit `super()` followed by `initCause`, and an instance initializer that sets the cause. Main misses the first and third. - **`classes/delegated_constant_arguments_error`:** `LinkedHashSet()` reaches `HashSet(16, .75f, true)`, and `16` and `true` reach the spec as literals. - **`classes/delegated_subclass_sets_error`:** `Stack` keeps its own default state after `Vector()`. - **`classes/delegated_argument_reassigned_error`:** a variable passed to `new` and 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 mentions `old(this)` gets the default state, without an error. - **`classes/super_swapped_arguments_correct` / `_error`:** `Sub(boolean y, boolean x) { super(y, x); }` for `Base(boolean x, boolean y)`, whose state mentions both parameters. They are replaced at once; replacing one after the other turned both into `x` (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 - **scijava-common `CommandInfo.java` (d9dce68)** with the Throwable spec: - Main: `found !#fresh but expected noThrowable(cnfe)`. - With this PR: `found !#fresh && withThrowable(cnfe) but expected noThrowable(cnfe)` at line 304. - The fixed version (`throw new ClassNotFoundException(null, exc)`) passes on both. - **The 53 OSS exercises (74 files, buggy and fixed)**, on main (4a89df6) and with this PR: identical results. ## Dependency `org.ow2.asm:asm` 9.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](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 92963a7 commit f7f366c

30 files changed

Lines changed: 1037 additions & 52 deletions

File tree

‎liquidjava-example/src/main/java/testSuite/classes/delegated_argument_reassigned_error/HashMapRefinements.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package testSuite.classes.delegated_argument_reassigned_error;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.Ghost;
5+
import liquidjava.specification.StateRefinement;
6+
7+
@ExternalRefinementsFor("java.util.HashMap")
8+
@Ghost("int capacity")
9+
public interface HashMapRefinements<K, V> {
10+
@StateRefinement(to = "capacity(this) == initialCapacity")
11+
void HashMap(int initialCapacity);
12+
13+
@StateRefinement(from = "capacity(this) == 5")
14+
V get(Object key);
15+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_argument_reassigned_error/Test.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package testSuite.classes.delegated_argument_reassigned_error;
2+
3+
import java.util.function.IntFunction;
4+
import javax.print.attribute.standard.PrinterStateReasons;
5+
6+
// PrinterStateReasons(int) runs HashMap(int): the capacity is the argument's value at the call, not a later one
7+
public class Test {
8+
static void assignedBefore(int n) {
9+
n = 5;
10+
PrinterStateReasons r = new PrinterStateReasons(n);
11+
r.get("k");
12+
}
13+
14+
static void assignedAfter(int n) {
15+
PrinterStateReasons r = new PrinterStateReasons(n);
16+
n = 5;
17+
r.get("k"); // Expect: State Refinement Error
18+
}
19+
20+
static void assignedBeforeAndAfter(int n) {
21+
n = 6;
22+
PrinterStateReasons r = new PrinterStateReasons(n);
23+
n = 5;
24+
r.get("k"); // Expect: State Refinement Error
25+
}
26+
27+
static IntFunction<PrinterStateReasons> lambdaParameter() {
28+
return k -> {
29+
PrinterStateReasons r = new PrinterStateReasons(k);
30+
r.get("k"); // Expect: State Refinement Error
31+
return r;
32+
};
33+
}
34+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_constant_arguments_error/HashSetRefinements.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package testSuite.classes.delegated_constant_arguments_error;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.Ghost;
5+
import liquidjava.specification.StateRefinement;
6+
import liquidjava.specification.StateSet;
7+
8+
@ExternalRefinementsFor("java.util.HashSet")
9+
@Ghost("int capacity")
10+
@StateSet({ "plain", "linked" })
11+
public interface HashSetRefinements<E> {
12+
@StateRefinement(to = "capacity(this) == initialCapacity && (dummy --> linked(this)) && (!dummy --> plain(this))")
13+
void HashSet(int initialCapacity, float loadFactor, boolean dummy);
14+
15+
@StateRefinement(from = "capacity(this) == 16 && linked(this)")
16+
boolean add(E e);
17+
18+
@StateRefinement(from = "plain(this)")
19+
void clear();
20+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_constant_arguments_error/Test.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package testSuite.classes.delegated_constant_arguments_error;
2+
3+
import java.util.LinkedHashSet;
4+
5+
// LinkedHashSet() runs HashSet(16, .75f, true) and LinkedHashSet(int n) runs HashSet(n, .75f, true): the constants
6+
// passed reach the spec as literals
7+
public class Test {
8+
static void defaultCapacity() {
9+
LinkedHashSet<String> s = new LinkedHashSet<>();
10+
s.add("x");
11+
}
12+
13+
static void linked() {
14+
LinkedHashSet<String> s = new LinkedHashSet<>();
15+
s.clear(); // Expect: State Refinement Error
16+
}
17+
18+
static void otherCapacity(int n) {
19+
LinkedHashSet<String> s = new LinkedHashSet<>(n);
20+
s.add("x"); // Expect: State Refinement Error
21+
}
22+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_spec_keeping_other_sets_correct/HashMapRefinements.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package testSuite.classes.delegated_spec_keeping_other_sets_correct;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.StateRefinement;
5+
import liquidjava.specification.StateSet;
6+
7+
@ExternalRefinementsFor("java.util.HashMap")
8+
@StateSet({ "a1", "a2" })
9+
@StateSet({ "b1", "b2" })
10+
public interface HashMapRefinements<K, V> {
11+
@StateRefinement(to = "a2(this)")
12+
void HashMap(int initialCapacity);
13+
14+
@StateRefinement(from = "b1(this)")
15+
V get(Object key);
16+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_spec_keeping_other_sets_correct/Test.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package testSuite.classes.delegated_spec_keeping_other_sets_correct;
2+
3+
import javax.print.attribute.standard.PrinterStateReasons;
4+
5+
// HashMap(int)'s spec leaves the b set unchanged, which refers to the old object: a new object has none, so the
6+
// delegated call gets the default state (b1) instead of that spec
7+
public class Test {
8+
static void get() {
9+
PrinterStateReasons r = new PrinterStateReasons(5);
10+
r.get("k");
11+
}
12+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_subclass_sets_error/StackRefinements.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package testSuite.classes.delegated_subclass_sets_error;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.StateRefinement;
5+
import liquidjava.specification.StateSet;
6+
7+
@ExternalRefinementsFor("java.util.Stack")
8+
@StateSet({ "empty", "nonEmpty" })
9+
public interface StackRefinements<E> {
10+
@StateRefinement(from = "empty(this)", to = "nonEmpty(this)")
11+
E push(E item);
12+
13+
@StateRefinement(from = "nonEmpty(this)")
14+
E peek();
15+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_subclass_sets_error/Test.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package testSuite.classes.delegated_subclass_sets_error;
2+
3+
import java.util.Stack;
4+
5+
// Stack() runs Vector(), whose spec sets only Vector's ghost: Stack's own state set starts at its default (empty)
6+
public class Test {
7+
static void pushThenPeek() {
8+
Stack<String> s = new Stack<>();
9+
s.push("a");
10+
s.peek();
11+
}
12+
13+
static void peekEmpty() {
14+
Stack<String> s = new Stack<>();
15+
s.peek(); // Expect: State Refinement Error
16+
}
17+
}

‎liquidjava-example/src/main/java/testSuite/classes/delegated_subclass_sets_error/VectorRefinements.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package testSuite.classes.delegated_subclass_sets_error;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.Ghost;
5+
import liquidjava.specification.StateRefinement;
6+
7+
@ExternalRefinementsFor("java.util.Vector")
8+
@Ghost("int elements")
9+
public interface VectorRefinements<E> {
10+
@StateRefinement(to = "elements(this) == 0")
11+
void Vector();
12+
}

‎liquidjava-example/src/main/java/testSuite/classes/super_swapped_arguments_correct/Base.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package testSuite.classes.super_swapped_arguments_correct;
2+
3+
import liquidjava.specification.StateRefinement;
4+
import liquidjava.specification.StateSet;
5+
6+
@StateSet({"open", "closed"})
7+
public class Base {
8+
@StateRefinement(to = "(x && !y && open(this)) || (!(x && !y) && closed(this))")
9+
public Base(boolean x, boolean y) {}
10+
11+
@StateRefinement(from = "open(this)", to = "closed(this)")
12+
public void close() {}
13+
}

0 commit comments

Comments
 (0)