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.

Sign up for GitHub

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

Jump to bottom

Update the value of access flag ACC_VALUE #15081

Closed
5 tasks done
hangshao0 opened this issue May 17, 2022 · 4 comments
Closed
5 tasks done

Update the value of access flag ACC_VALUE #15081

hangshao0 opened this issue May 17, 2022 · 4 comments
Labels
comp:vm project:valhalla Used to track Project Valhalla related work

Comments

@hangshao0
Copy link
Contributor

hangshao0 commented May 17, 2022 •
edited
Loading

There is an update to the value object JEP yesterday that class access flag ACC_VALUE is changed from 0x0100 to 0x0040.

Once the corresponding code change is made by OpenJDK and merged into https://github.com/ibmruntimes/openj9-openjdk-jdk.valuetypes, OpenJ9 needs to change

openj9/runtime/oti/cfr.h

Line 861 in e9e305a

#define CFR_ACC_VALUE_TYPE 0x00000100
to 0x0040.

There are more change from http://cr.openjdk.java.net/~dlsmith/jep8277163/jep8277163-20220519/specs/value-objects-jvms.html.

In summary, the changes are:
@hangshao0 hangshao0 added comp:vm project:valhalla Used to track Project Valhalla related work labels May 17, 2022
@hangshao0
Copy link
Contributor Author

ACC_IDENTITY (0x0020) is added, it reuses the value of ACC_SUPER

OpenJDK has added ACC_IDENTITY, but its asm code is not updated yet. Currently it does not allow ACC_IDENTITY to be set, which is causing some of our tests to fail:

java.lang.IllegalArgumentException: Invalid access flags: 58
	at java.base/jdk.internal.org.objectweb.asm.util.CheckClassAdapter.checkAccess(CheckClassAdapter.java:573)
	at java.base/jdk.internal.org.objectweb.asm.util.CheckClassAdapter.visitInnerClass(CheckClassAdapter.java:395)
	at java.base/jdk.internal.org.objectweb.asm.ClassReader.accept(ClassReader.java:749)
	at java.base/jdk.internal.org.objectweb.asm.ClassReader.accept(ClassReader.java:456)
	at org.openj9.test.javaagenttest.util.Transformer.transform(Transformer.java:69)
	at org.openj9.test.javaagenttest.RefreshGCCache_ExtendedHCR_Test.test_EXTENDED_HCR(RefreshGCCache_ExtendedHCR_Test.java:49)

We need to exclude these tests in vt standard build until OpenJDK update asm to include ACC_IDENTITY.

@hangshao0 hangshao0 mentioned this issue May 26, 2022
Merged
@hangshao0 hangshao0 mentioned this issue May 26, 2022
Open
hangshao0 added a commit to hangshao0/openj9 that referenced this issue May 26, 2022
@hangshao0
Update the value of ACC_VALUE and remove j.l.IdentityObject
13a8dfb 
1. Update the value of ACC_VALUE
2. remove ACC_PERMITS_VALUE
3. Add ACC_IDENTITY
4. remove IdentityObject
5. Update test code
6. Re-include PackageAnnotation tests which were excluded because of 
the injected j.l.IdentityObject.
7. OpenJDK ASM code is not updated for ACC_IDENTITY yet, it does not
allow ACC_IDENTITY to be set. Exclude tests testGCRetransform and
test_EXTENDED_HCR which failed on access flags check of ACC_IDENTITY

issue eclipse-openj9#15081, eclipse-openj9#12878

Signed-off-by: Hang Shao <[email protected]>
@hangshao0
Copy link
Contributor Author

This issue can be closed.

@pshipton
Copy link
Member

@hangshao0 can the test excluded label be removed from this issues since we have #12878 for tracking? We shouldn't have any closed issues with test excluded.

@hangshao0
Copy link
Contributor Author

can the test excluded label be removed from this issues

Yes. I will remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
comp:vm project:valhalla Used to track Project Valhalla related work
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hangshao0 @pshipton