Bug 576571 - indirect static imports may result in IllegalAccessError at runtime
Summary: indirect static imports may result in IllegalAccessError at runtime
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.21   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-12 06:54 EDT by Stephan Herrmann CLA
Modified: 2023-10-03 17:26 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2021-10-12 06:54:11 EDT
Originally reported in https://github.com/jzy3d/panama-gl/issues/3

-------------------8<-----------------------
Maurizio added

Performed some more tests:

// pkg/A.java
package pkg;

class A {
    public static void foo() { }
}

// pkg/B.java
package pkg;

public class B extends A { }

// Test.java
import static pkg.B.foo;

class Test {
     public static void main(String[] args) {
         foo();
     }
}

I compiled this with both javac and the Eclipse compiler. With javac,
Test::main has the following bytecode:

          0: invokestatic  #2                  // Method pkg/B.foo:()V

With Eclipse, I see the following:

          0: invokestatic  #12                 // Method pkg/A.foo:()V

Of course, the code compiled with javac runs, whereas the one compiler
with Eclipse fails with this:

Exception in thread "main" java.lang.IllegalAccessError: failed to
access class pkg.A from class Test (pkg.A and Test are in unnamed module
of loader 'app')
     at Test.main(Test.java:5)

Which seems like a bug in the Eclipse compiler, and an unfortunate one
too, since jextract relies on this pattern quite a lot.
-------------------8<-----------------------
Comment 1 Stephan Herrmann CLA 2021-10-12 07:22:31 EDT
On a related note bug 475444 claims that an indirect static import (i.e., via a sub class) is not possible in Eclipse. With today's HEAD I cannot reproduce that problem. Maybe it was a red herring or it got fixed in the interim.
Comment 2 Stephan Herrmann CLA 2021-10-12 07:26:40 EDT
In private email with Maurizio I received the following hints regarding where this topic would be specified:

--------8<-----------
it seems like 13.1 has the answer:

Given a method invocation expression or a method reference expression in a class or interface C, referencing a method named m declared (or implicitly declared (ยง9.2)) in a (possibly distinct) class or interface D, we define the qualifying class or interface of the method invocation as follows: 

Then:

If the expression is referenced by a simple name, then if f is a member of the current class or interface, C, then let Q be C. Otherwise, let Q be the innermost lexically enclosing class or interface declaration of which f is a member. In either case, Q is the qualifying class or interface of the reference. 

What is the "innermost lexically enclosing class or interface declaration of which foo is a member"? Given that (per 7.5.3) import static pkg.B.foo; "imports all accessible static members with a given simple name from a class or interface",  would say foo is a member of pkg.B here (even if `foo` is declared in a superclass). So, since B is imported into Test, it seems to imply that B should be the "innermost lexically enclosing class or interface declaration of which foo is a member".

As it's often the case in this area, the spec has a roundabout way to go about things - because it has to specify the binary form of a name w/o entering too much in JVMS territory. But I believe that generating a bytecode reference to an inaccessible member (when that member is inherired through an accessible type) seem to be the norm. And, if the code is tweaked so that, instead of relying on static imports, the method call is fully qualified as in `pkg.B.foo`, then Eclipse seems to be doing the right thing - which seems to point at a quirk with static imports.
--------8<-----------
Comment 3 Stephan Herrmann CLA 2021-10-12 07:36:58 EDT
Looking at our implementation it seems that ecj has no notion that B.foo() from the example would even "exist". There's only A.foo() (which may be *found* via B) and some code locations (like "pkg.B.foo()") tweak code gen to consider the receiver to determine the bytecode-symbol for representing the method. For static imports there is no such tweak, we resolve foo() to pkg.A.foo() and that's the symbol code gen uses. 

The mentioned tweaks can be found by searching for callers of MessageSend.setActualReceiverType(ReferenceBinding)


I don't see an easy way to invoke setActualReceiverType() with the correct type, when a method was found via getImplicitMethod() using a static import. This could imply that we need to synthesize a method binding for B.foo() indeed. Not sure if this may be persisted (in B's methods?) or should be made for one-time use by the given MessageSend only.

Use of setActualReceiverType() during getImplicitMethod() goes back to https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=a120c3eba1289fa5bdc2d2b34c4bf1b18bf73e31 comment: *** empty log message ***. The commit contains a build-notes change: https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/diff/org.eclipse.jdt.core/buildnotes_jdt-core.html?id=a120c3eba1289fa5bdc2d2b34c4bf1b18bf73e31 which however looks unrelated. Anyway, this was long before the introduction of import static.
Comment 4 Eclipse Genie CLA 2023-10-03 17:26:45 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.