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

Implement class relationship recording and validating #7089

Merged
merged 5 commits into from
Sep 16, 2019

Conversation

sharon-wang
Copy link
Contributor

@sharon-wang sharon-wang commented Sep 16, 2019

Store class relationships to delay verifying class compatibility (child, parent relationships) until triggered by class loading, instead of verifying compatibility upfront. Validate relationships for a class during class initialization, throwing a Verify Error if validation fails.

Enable using -XX:[+/-]ClassRelationshipVerifier. Cannot be used with -Xfuture.
Documentation issue: eclipse-openj9/openj9-docs#360

Testing will be added in a future PR.

sharon-wang and others added 4 commits September 16, 2019 08:43
Store class relationships (child and its superclass/superinterface
relationships) in a hash table (saved in the class loader) instead of
loading the classes and verifying them upfront.

Each child (source class) table entry contains a list of its parents
(target classes), with parent nodes being added as they are
encountered.

Enable feature using `-XX:+ClassRelationshipVerifier`.
Disable feature using `-XX:-ClassRelationshipVerifier`.

Throw error if `-Xfuture` is used with `-XX:+ClassRelationshipVerifier`.
Note that `-Xverify:all` also enables `-Xfuture`.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
Currently, the code always adds the relationship even if it has already
been added.  This can lead to very long lists of parents which delays
processing them.

The insight here is that by keeping the list in the order of the
classname length, we can more quickly weed out duplicates as we only
need to walk the list until there is a longer name, or the same name
already present, before inserting.

Additionally, avoid the parent node allocation until we know we’re
adding something to the list.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
When a class is being initialized, validate that it holds the expected
relationship with each parent in its list.

Free memory for the child hash table entry and parent nodes once
validation is complete.

Throw java/lang/VerifyError if validation of class relationships fails.

Note that this validation check can be moved to any point prior to
a class being made available.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
Instead of allocating a parent node for Throwable, set a bit in the
childEntry flags J9RELATIONSHIP_PARENT_IS_THROWABLE.

During validation, check if the bit is set. If so, retrieve the Throwable
class and check that it is a superclass of the child.

Throwable seems to occur frequently as a parent class, so this change
will avoid the duplicate memory allocations done for Throwable.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

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

This has previously been reviewed desk-side. Minor formatting changes to the prototype descriptions would be good.

runtime/oti/bcverify_api.h Outdated Show resolved Hide resolved
@DanHeidinga
Copy link
Member

Jenkins test sanity plinux,aix,zlinux jdk8

@DanHeidinga
Copy link
Member

Jenkins test sanity win,osx,xlinux jdk11

@DanHeidinga
Copy link
Member

I see all the tests have passed. @sharon-wang can you push the doc update?

Reduce amount of white space used between parameters and their
descriptions.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
@sharon-wang
Copy link
Contributor Author

@DanHeidinga reformatting changes are now pushed

Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

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

lgtm

@DanHeidinga
Copy link
Member

The travis compile has passed so I won't require another rebuild

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

Successfully merging this pull request may close these issues.

3 participants