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
Disable the flag check on method without stackmaps #5736
Disable the flag check on method without stackmaps #5736
Conversation
Reviewer: @DanHeidinga FYI: @pshipton |
Jenkins test sanity,extended xlinux jdk8 |
Jenkins test sanity zlinux jdk11 |
Jenkins test extended xlinux jdk8 |
runtime/bcverify/rtverify.c
Outdated
@@ -307,7 +307,7 @@ matchStack(J9BytecodeVerificationData * verifyData, J9BranchTargetStack *liveSta | |||
|
|||
/* Jazz103: 120689 */ | |||
/* Target stack frame flag needs to be subset of ours. See JVM sepc 4.10.1.4 */ | |||
if (liveStack->uninitializedThis && !targetStack->uninitializedThis) { | |||
if (!verifyData->createdStackMap && liveStack->uninitializedThis && !targetStack->uninitializedThis) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have a comment in the code explaining why it's not applicable to the createdStackMap case.
The code may end up looking cleaner if written in this form once the comment is added:
/* Long comment describing
* why this check isn't applicable to
* createdstackMaps
*/
if (!verifyData->createdStackMap) {
/* Target stack frame flag needs to be subset of ours. See JVM sepc 4.10.1.4 */
if (liveStack->uninitializedThis && !targetStack->uninitializedThis) {
.....
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already updated the code & comment against the suggestion above.
056d75c
to
190dbc6
Compare
The change is to disable the check on the uninitializedThis flag on old version classfiles (< 51) as such check is only applied to classfiles with stackmaps which was introduced since Java 7(class version is 51). Fix: eclipse-openj9#5676 Signed-off-by: Cheng Jin <jincheng@ca.ibm.com>
190dbc6
to
151ebcf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtml
Jenkins test extended xlinux jdk8 |
Jenkins test sanity zlinux jdk11 |
@ChengJin01 Can you cherrypick this commit into a PR against the |
Already created the PR against |
PR for the 0.14.2 release: #5740 |
The change is to disable the check on the uninitializedThis
flag on old version classfiles (< 51) as such check is only
applied to classfiles with stackmaps which was introduced
since Java 7(class version is 51).
Fix: #5676
Signed-off-by: Cheng Jin jincheng@ca.ibm.com