Private subclasses of public classes should not be reported as linkage errors

[suztomo]

Followup of #1599 (comment). It was a false positive.

com.sun.tools.internal.ws.wscompile.WsgenOptions (source) references com.sun.xml.internal.ws.api.BindingID$SOAPHTTPImpl (target). The latter is a private class. Linkage Checker detected it as a linkage error. However, the reference turned out to be not harmful. This is because the source class is not instantiating the private class or declaring a variable with the type.

One of the referencing method is the code below:

// in WsgenOptions.java
  BindingID getBindingID(String protocol) {
    if (protocol.equals("soap1.1")) {
      return BindingID.SOAP11_HTTP;
    } else if (protocol.equals("Xsoap1.2")) {
      return BindingID.SOAP12_HTTP;
    } else {
      String lexical = (String)this.nonstdProtocols.get(protocol);
      return lexical != null ? BindingID.parse(lexical) : null;
    }
  }

Here, BindingID.SOAP11_HTTP has type com.sun.xml.internal.ws.api.BindingID$SOAPHTTPImpl (private class), which is a subclass of BindingID class. As WsgenOptions is not touching the private class, calling the getBindingID method does not cause a LinkageError at runtime.

Simplified Example

Suppose we have the following class A, B, and C:

package foo;

public class A {
  public static final B f = new B();

  private static class B extends A {
  }
}

package foo;

public class C {
  public static void main(String[] arguments) {
    A a = A.f;
    System.out.println(a);
  }

  void f(A.B b) { // This should fail. How?
    System.out.println(b);
  }
}

When compiled, C's class file contains a reference to (private) A$B class. Linkage Checker would report this reference as a linkage error, simply because A$B is not accessible from C. However, this is a false positive as the code compiles and run without a problem.

  Compiled from "C.java"
Constant pool:
...
   #3 = Fieldref           #23.#24        // foo/A.f:Lfoo/A$B;
...
  #23 = Class              #32            // foo/A
  #24 = NameAndType        #33:#37        // f:Lfoo/A$B;
...
  #32 = Utf8               foo/A
  #33 = Utf8               f
  #34 = Class              #41            // foo/A$B
  #37 = Utf8               Lfoo/A$B;
...
  #41 = Utf8               foo/A$B

Interestingly the field #34 is not referenced by other constant pool entries or byte code. How about the case of com.sun.tools.internal.ws.wscompile.WsgenOptions?

com.sun.tools.internal.ws.wscompile.WsgenOptions has the class reference (javap output) used in InnerClasses attribute:

#392 = Class              #408          // com/sun/xml/internal/ws/api/BindingID$SOAPHTTPImpl
...
InnerClasses:
     private static final #393= #392 of #300; //SOAPHTTPImpl=class com/sun/xml/internal/ws/api/BindingID$SOAPHTTPImpl of class com/sun/xml/internal/ws/api/BindingID

https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6

If the constant pool of a class or interface C contains at least one CONSTANT_Class_info entry (§4.4.1) which represents a class or interface that is not a member of a package, then there must be exactly one InnerClasses attribute in the attributes table of the ClassFile structure for C.

How about "If a class reference is only used by the InnerClasses annotation, we skip it"?

Java Language Specification: 12.3.3. Resolution of Symbolic References

https://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.3.3

For constructor, we can checknew. But it cannot detect wrong access of A.B

How about "If a class reference is only used by NameAndType, we ignore it"?

[elharo]

Nice analysis. This seems like a high priority to fix. It is not exclusive to JDK classes, and private or non-public subclasses of public types are a common pattern in OO design in general and Java in particular.

[suztomo]

@elharo I'm thinking to fix this by reading byte code of the (source) class file. It's a true linkage error only when the class reference is used in

• Calling constructor of the class,
• Accessing the field of the class, or
• Accessing the method of the class

What do you think?

[suztomo]

Actually without subclassing, the case still stands.

package foo;

public class A {
  public static final B f = new B();

  private static class B {
  }
}

package foo;

public class C {
  public static void main(String[] arguments) {
    System.out.println(A.f);
  }
}

[elharo]

Is it? In this case you're relying on on B being a subclass of java.lang.Object and thus having a toString() method that's invoked polymorphically. You're calling PrintStream.toString(Object o).

[elharo]

This is a good example though. What we need to be testing here is whether the type of variable passed to the println method is accessible, not whether the actual runtime type of A.f is accessible.

[elharo]

This example shows we can't rely on it being a linkage error when accessing the method of the inaccessible class. This case does call the toString() method of the class, which is allowed.

[suztomo]

Right, being the subclass of java.lang.Object seems important. (Class C's bytecode does not call Object.toString(); https://gist.github.com/suztomo/500cbca470aefc2ba74e9d38a30e5cb0)

Explicitly calling toString fails compilation:

[elharo]

Yes, the call to toString is in PrintStream

[elharo]

What if you did System.out.println((Object) A.f).toString());

[suztomo]

The compiler does not complain in that case.

[suztomo]

Memo:
Earlier, I wrote this:

  void f(A.B b) { // This should fail. How?
    System.out.println(b);
  }

It turned out that this assumption was incorrect. JVM does not fail the code even if A.B is private at runtime. Just having a variable or function parameter which is private (inaccessible) at runtime does not throw a runtime error.

Experiment

What if the accessor of A.B changes from public to private in below? (public at the time of compile and private at the runtime) Does it create an error?

public class C {
  public static void main(String[] arguments) {
    A.B f = A.f;
    m(f);
  }

  private static void m(A.B b) {
    System.out.println(b);
  }
}

Changing A$B to private did not give a runtime error.

# Compile bar.C with public A$B and keep it as b-public.jar
suztomo-macbookpro44% mvn package  
suztomo-macbookpro44% mv target/linkage-checker-test-1.0-SNAPSHOT.jar ./b-public.jar

# Update A$B as private, then compile the class file.

suztomo-macbookpro44% mvn compile   

suztomo-macbookpro44% rm -rf target/classes/bar

# Run the private A$B with bar.C. This does not throw a runtime error
suztomo-macbookpro44% java -cp target/classes:./b-public.jar bar.C 
foo.A$B@42a57993
suztomo-macbookpro44% javap -private -cp target/classes:./b-public.jar bar.C
Compiled from "C.java"
public class bar.C {
  public bar.C();
  public static void main(java.lang.String[]);
  private static void m(foo.A$B);
}

If bar.C tries to access a public method of a now private class A$B, it throws IllegalAccessError.

suztomo-macbookpro44% java -cp target/classes:./b-public.jar bar.C
Exception in thread "main" java.lang.IllegalAccessError: tried to access class foo.A$B from class bar.C
	at bar.C.m(C.java:31)
	at bar.C.main(C.java:27)

This observation falls in https://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.3.3

IllegalAccessError: A symbolic reference has been encountered that specifies ... or invocation of a method ... to which the code containing the reference does not have access ... because the class was not declared public.

Relaxing Class Reference Validation

IllegalAccessError: A symbolic reference has been encountered that specifies a use or assignment of a field, or invocation of a method, or creation of an instance of a class, to which the code containing the reference does not have access because the field or method was declared with private, protected, or default access (not public), or because the class was not declared public.
This can occur, for example, if a field that is originally declared public is changed to be private after another class that refers to the field has been compiled (§13.4.7).

I propose that Linkage Checker should invalidate a class reference's access (whether the referencing class has enough access to the referenced class) only when the reference is used in

• Method references (invocation of a method)

• Field references (a use or assignment of a field)

• Instantiation of the class (creation of an instance of a class)

  Is constructor considered one of method references?

[suztomo]

@elharo @netdpb I appreciate if you can give me feedback on http://go/jdd-class-reference-validation