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

[JDK18] InternalError: Caller is not annotated as @CallerSensitive #13998

Closed
babsingh opened this issue Nov 24, 2021 · 9 comments · Fixed by ibmruntimes/openj9-openjdk-jdk#382

Comments

@babsingh
Copy link
Contributor

babsingh commented Nov 24, 2021

OpenJDK Tests

TEST: jdk/internal/reflect/Reflection/GetCallerClassTest.java#id0
TEST: jdk/internal/reflect/Reflection/GetCallerClassTest.java#id2
TEST: jdk/internal/reflect/Reflection/GetCallerClassTest.java#id1
TEST: jdk/internal/reflect/Reflection/GetCallerClassTest.java#id3

Error

[2021-11-17T18:35:06.056Z] testMissingCallerSensitiveAnnotation...
[2021-11-17T18:35:06.056Z] STDERR:
[2021-11-17T18:35:06.056Z] java.lang.InternalError: Caller is not annotated as @sun.reflect.CallerSensitive.
[2021-11-17T18:35:06.056Z]      at java.base/jdk.internal.reflect.Reflection.getCallerClass(Native Method)
[2021-11-17T18:35:06.056Z]      at boot.GetCallerClass.missingCallerSensitiveAnnotation(GetCallerClass.java:35)
[2021-11-17T18:35:06.056Z]      at GetCallerClassTest.testMissingCallerSensitiveAnnotation(GetCallerClassTest.java:118)
[2021-11-17T18:35:06.056Z]      at GetCallerClassTest.main(GetCallerClassTest.java:89)
[2021-11-17T18:35:06.056Z]      at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[2021-11-17T18:35:06.056Z]      at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:76)
[2021-11-17T18:35:06.056Z]      at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:52)
[2021-11-17T18:35:06.056Z]      at java.base/java.lang.reflect.Method.invoke(Method.java:577)
[2021-11-17T18:35:06.056Z]      at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
[2021-11-17T18:35:06.056Z]      at java.base/java.lang.Thread.run(Thread.java:884)

Related: #13946

@babsingh babsingh added this to the Release 0.31 (Java 18) milestone Nov 24, 2021
@babsingh babsingh changed the title [JDK18] InternalError: Caller is not annotated as @sun.reflect.CallerSensitive [JDK18] InternalError: Caller is not annotated as @CallerSensitive Nov 24, 2021
babsingh added a commit to babsingh/aqa-tests that referenced this issue Nov 30, 2021
Tests failures documented in the below issues have been excluded:
1) eclipse-openj9/openj9#13995
2) eclipse-openj9/openj9#13996
3) eclipse-openj9/openj9#13997
4) eclipse-openj9/openj9#13998

Parent issue: eclipse-openj9/openj9#13946

The tests will be re-enabled once they are fixed.

Signed-off-by: Babneet Singh <[email protected]>
@babsingh
Copy link
Contributor Author

babsingh commented Nov 30, 2021

Test code: openjdk-next...GetCallerClassTest.java#L121

Test code expects the InternalError message to be CallerSensitive annotation expected.

OpenJ9's InternalError has the following message: Caller is not annotated as @sun.reflect.CallerSensitive.

J9NLS_VM_CALLER_NOT_ANNOTATED_AS_CALLERSENSITIVE=Caller is not annotated as @sun.reflect.CallerSensitive.

This failure can be resolved if the InternalError message mismatch between OpenJ9 and test code is fixed.

Three potential solutions:

  1. Update OpenJDK test code
  2. Update OpenJ9 trace point message
  3. Similar testing exists in OpenJ9: openj9...GetCallerClassTests.java. So, we keep the failing OpenJDK tests excluded, and rely upon similar OpenJ9 testing.

@pshipton We have chosen Solution 3 for similar cases in the past. Do we want to take the same path for this issue?

@pshipton
Copy link
Member

Although we don't want the patches, the safest approach is to update the OpenJDK test code. I'm concerned we might miss something otherwise with all the changes that are occurring. Although it would be a better solution, I'm concerned copying the OpenJDK message is a license violation.

llxia pushed a commit to adoptium/aqa-tests that referenced this issue Dec 1, 2021
Tests failures documented in the below issues have been excluded:
1) eclipse-openj9/openj9#13995
2) eclipse-openj9/openj9#13996
3) eclipse-openj9/openj9#13997
4) eclipse-openj9/openj9#13998

Parent issue: eclipse-openj9/openj9#13946

The tests will be re-enabled once they are fixed.

Signed-off-by: Babneet Singh <[email protected]>
@gacholio
Copy link
Contributor

gacholio commented Dec 1, 2021

All of the tests which check for an exact exception message match are invalid. We long ago decided against blindly copying OpenJDK messages. Patching 100s of tests seems unreasonable.

@babsingh
Copy link
Contributor Author

babsingh commented Dec 3, 2021

I agree that we should not update OpenJ9 code. I don't think the test in question is invalid.

I also agree that we may miss something critical if we exclude the OpenJDK test.

In this case, we can update the OpenJDK test with a regex so that it accepts both OpenJ9 and OJDK messages. We will need to only update two locations:

  1. openjdk-next...GetCallerClassTest.java#L121
  2. openjdk-next...GetCallerClassTest.java#L136

@pshipton @gacholio For resolving this issue, shall we proceed with updating the OpenJDK test?

@gacholio
Copy link
Contributor

gacholio commented Dec 3, 2021

If you mean updating it in the OpenJDK repo, yes. If you mean a local patch, then no I would not approve that.

@pshipton
Copy link
Member

pshipton commented Dec 3, 2021

This is updating in the OpenJDK repo, which I agree with.

@gacholio
Copy link
Contributor

gacholio commented Dec 3, 2021

It looks like an IBM repo, which is not the OpenJDK repo, so we'll be maintaining patches. This is foolish, but not my problem, I suppose.

@babsingh
Copy link
Contributor Author

babsingh commented Dec 3, 2021

In the future, we can encounter conflicts in the extensions repo for our patches. The chances of the test code to be updated are pretty low. So, these patches should have a low risk for a potential conflict.

@pshipton
Copy link
Member

pshipton commented Dec 3, 2021

It seems more foolish to allow a bug to occur because we didn't run the tests and something was missed. While it would be nice to contribute the test fix upstream, it seems unlikely to be accepted. Although we could try.

babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this issue Dec 6, 2021
InternalError message in GetCallerClassTest differs between JVMs:
- OpenJ9: "Caller is not annotated as @sun.reflect.CallerSensitive"
- OpenJDK: "CallerSensitive annotation expected"

GetCallerClassTest has been updated to accept OpenJ9's InternalError message.

Closes: eclipse-openj9/openj9#13998

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this issue Dec 6, 2021
InternalError message in GetCallerClassTest differs between JVMs:
- OpenJ9: "Caller is not annotated as @sun.reflect.CallerSensitive"
- OpenJDK: "CallerSensitive annotation expected"

GetCallerClassTest has been updated to accept OpenJ9's InternalError message.

Closes: eclipse-openj9/openj9#13998

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this issue Dec 6, 2021
InternalError message in GetCallerClassTest differs between JVMs:
- OpenJ9: "Caller is not annotated as @sun.reflect.CallerSensitive"
- OpenJDK: "CallerSensitive annotation expected"

GetCallerClassTest has been updated to accept OpenJ9's InternalError message.

Closes: eclipse-openj9/openj9#13998

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this issue Dec 6, 2021
InternalError message in GetCallerClassTest differs between JVMs:
- OpenJ9: "Caller is not annotated as @sun.reflect.CallerSensitive"
- OpenJDK: "CallerSensitive annotation expected"

GetCallerClassTest has been updated to accept OpenJ9's InternalError message.

Closes: eclipse-openj9/openj9#13998

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this issue Dec 6, 2021
InternalError message in GetCallerClassTest differs between JVMs:
- OpenJ9: "Caller is not annotated as @sun.reflect.CallerSensitive"
- OpenJDK: "CallerSensitive annotation expected"

GetCallerClassTest has been updated to accept OpenJ9's InternalError message.

Closes: eclipse-openj9/openj9#13998

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this issue Dec 6, 2021
InternalError message in GetCallerClassTest differs between JVMs:
- OpenJ9: "Caller is not annotated as @sun.reflect.CallerSensitive"
- OpenJDK: "CallerSensitive annotation expected"

GetCallerClassTest has been updated to accept OpenJ9's InternalError message.

Closes: eclipse-openj9/openj9#13998

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/aqa-tests that referenced this issue Dec 13, 2021
eclipse-openj9/openj9#13996 is fixed via eclipse-openj9/openj9#14125.
eclipse-openj9/openj9#13997 is fixed via eclipse-openj9/openj9#14125.
eclipse-openj9/openj9#13998 is fixed via ibmruntimes/openj9-openjdk-jdk#382.

GetCallerClassTest does not run successfully yet. Currently, it fails due to
eclipse-openj9/openj9#14097. So, it remains excluded.

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/aqa-tests that referenced this issue Dec 13, 2021
babsingh added a commit to babsingh/aqa-tests that referenced this issue Dec 15, 2021
llxia pushed a commit to adoptium/aqa-tests that referenced this issue Dec 15, 2021
@babsingh babsingh self-assigned this Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants