Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Proxy support for JPF on Java 11 #356

Merged
merged 5 commits into from
Jun 2, 2023
Merged

Add Proxy support for JPF on Java 11 #356

merged 5 commits into from
Jun 2, 2023

Conversation

quadhier
Copy link
Collaborator

This patch adds Proxy support for JPF on Java 11 and some unit tests. It should fix the following two failing unit tests, which are parts of #274.

gov.nasa.jpf.test.vm.reflection.ProxyTest::testAnnoProxy
gov.nasa.jpf.test.vm.reflection.ProxyTest::testBasicProxy

I have tested locally and found that it fixes these two test cases with no more regressions (7 failing tests left now).

Problem Analysis

In OpenJDK 11, Proxy.newInstance() involves too much code that JPF currently does not fully support; we need to add a model class for java.lang.reflect.Proxy and use JPF's way to implement a simpler one but with the same functionality.

Basic Idea

According to doc of getProxyClass(), a Proxy is defined by its classloader and the List of interfaces to implement (the order is important). But in fact, a Proxy's implementation is solely defined by the List of interfaces it implements (which can be evident by the signature of JDK's proxy generation method ProxyGenerator.generateProxyClass(final String name, Class<?>[] interfaces, int accessFlags)).

We can use the List of interfaces to get a unique name of a Proxy class, then use JDK's ProxyGenerator.generateProxyClass to generate the classfile of it (and cache it in the resolvedClasses), and finally define the class.

Discussions

Why do these two tests pass on master branch (on OpenJDK 8)?

In OpenJDK 8's implementation, Proxy.newProxyInstance() doesn't involve that much code which JPF cannot handle. All we need to do is to implement the native method Proxy.defineClass0().

Copy link
Member

@cyrille-artho cyrille-artho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comments. For the first one, we need to create an issue (and have some discussions), so we cannot resolve that right away.

@quadhier
Copy link
Collaborator Author

quadhier commented Jun 1, 2023

Please see my comments. For the first one, we need to create an issue (and have some discussions), so we cannot resolve that right away.

Ok, thanks for your reviews @cyrille-artho !

@cyrille-artho
Copy link
Member

Great, thank you for your work and updating the tests! I will merge the PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants