Skip to content

Detect delegating superclass constructors from bytecode instead of a hard-coded list #367

Closed

Description

@CatarinaGamboa

Context

#354 lets a user constructor that starts with super(...) take the state given by the spec of the constructor it calls. When that class has no spec of its own and the spec sits on an ancestor (e.g. MyException extends Exception calling super(msg), with the spec on Throwable(String)), the lookup has to walk through the intermediate binary class. That's only safe if the intermediate constructor passes its arguments unchanged to the superclass constructor.

We can't read the bodies of binary classes, so #354 hard-codes the classes known to do this: DELEGATING_JDK_CLASSES = {Exception, RuntimeException, Error} in AuxStateHandler. Any other intermediate class stops the walk and falls back to the default state. This is sound, but it doesn't scale beyond those classes.

Proposal: check delegation in the bytecode

A constructor that only delegates compiles (with javac) to a fixed instruction shape:

aload_0
<load param 1> ... <load param n>    // in order, with the right load instruction per type
invokespecial Super.<init>(<same descriptor>)
return

(ignoring labels, line numbers and frames). If the intermediate constructor matches that shape, keep walking up to the superclass's spec. If it doesn't, stop. For example, Exception(String) matches and gets walked through, while ClassNotFoundException(String) (which runs aload_1; aconst_null; invokespecial Exception.<init>(String, Throwable)) doesn't match and is rejected.

Sketch

  • Add org.ow2.asm:asm (BSD-3-Clause, single small jar).
  • Read the class bytes with ClassLoader.getSystemResourceAsStream("java/lang/Exception.class") for JDK classes. Third-party classes need a class loader over the classpath the verifier is given.
  • A MethodVisitor matches the shape above. long/double parameters take two slots.
  • Cache the result per constructor.
  • Replace the DELEGATING_JDK_CLASSES check in specifiedConstructor with this check, and delete the set.

Any constructor that doesn't match falls back to the default state, as it does today, so a false negative is safe.

Tests

  • MyException extends IOException with the spec only on Throwable inherits the state (walks through IOException and Exception).
  • A subclass of ClassNotFoundException calling super(String) does not inherit Throwable(String)'s state.
  • A non-JDK library hierarchy, if one is available on the test classpath.

Alternatives considered

  • Hand-kept list (current): sound, but it only covers the classes listed.
  • Specs for every intermediate class: no verifier code, but specs get duplicated down every hierarchy, and a forgotten class silently gets the default state.
  • @ExternalRefinementsFor(Klass.class) instead of a string: it gives compile-time checking of the class name but doesn't expose constructor bodies, so on its own it doesn't solve this.

Estimate: half a day to a day including tests.

Activity

  1. CatarinaGamboa commented on Oct 27, 2026

    @CatarinaGamboa
    CollaboratorAuthor

    A related case that the bytecode check would also cover: a JDK subclass instantiated directly (no user subclass). On main (0d489e1), new RuntimeException("failed") gets no state, so a correct initCause is reported:

    import liquidjava.specification.ExternalRefinementsFor;
    import liquidjava.specification.StateRefinement;
    import liquidjava.specification.StateSet;
    
    @ExternalRefinementsFor("java.lang.Throwable")
    @StateSet({"noThrowable", "withThrowable"})
    interface ThrowableRefinements {
        @StateRefinement(to = "noThrowable(this)")
        void Throwable(String message);
    
        @StateRefinement(to = "withThrowable(this)")
        void Throwable(String message, Throwable cause);
    
        @StateRefinement(from = "noThrowable(this)", to = "withThrowable(this)")
        Throwable initCause(Throwable cause);
    }
    public class Repro {
        static RuntimeException wrap(Throwable t) {
            RuntimeException e = new RuntimeException("failed");
            e.initCause(t);   // correct (RuntimeException(String) leaves no cause), but reported
            return e;
        }
    }
    State Refinement Error: found true but expected noThrowable(e⁸)
    4 |         e.initCause(t);   // correct (RuntimeException(String) leaves no cause), but reported
    

    new IllegalStateException("failed") gives the same. The pattern new XException(msg) followed by initCause(e) is common in real code, so every such use is a false error today.

    The bytecode check proposed here would also get the real bugs right. For example, ClassNotFoundException() compiles to aload_0; aconst_null; checkcast Throwable; invokespecial ReflectiveOperationException.<init>(Throwable), i.e. Throwable(Throwable) with a null cause, so the object is withThrowable. That is the real bug in elect86/scijava-common (fixed in a6fd9cf, "Fix IllegalStateException when class is not found"):

    final ClassNotFoundException cnfe = new ClassNotFoundException();
    cnfe.initCause(exc);   // IllegalStateException: Can't overwrite cause
    throw cnfe;

    To follow that, the check would map a constructor that passes null (or another argument) to a superclass constructor onto that constructor's spec, not only the pure "pass every argument unchanged" shape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    real-world-findingFound when experimenting liquidjava in real world repos or OSS

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions