Re: RFR (S) : JDK-8007736 - VerifyError for use of static method in interface
Bharadwaj, I'm including core-libs-dev so that they know this fix is coming. http://cr.openjdk.java.net/~bharadwaj/8007736/jdk8-tl-jdk/webrev/ The jdk change looks okay to me. Minor nit: L1250-1251: some identation will help. You can follow the original formatting - breaking '?' and ':" in the newlines. Can you include the new regression test in the jdk repo as well (test/vm/verifier directory)? That'll make sure this test is run by core-libs and JPRT job (it's part of the default testset). Thanks Mandy On 2/12/2013 7:44 PM, S. Bharadwaj Yadavalli wrote: On 2/12/2013 2:17 PM, Karen Kinnear wrote: Code looks good. I like this length of test too :-) Thanks for the review, Karen. Please check -XX:-UseSplitVerifier to see if we also need to fix the old verifier for new classfiles. I checked with -XX:-UseSplitVerifier and found that I needed to fix the old verifier. I updated the webrev with my changes which now need to be made to jdk tree as well as hotspot tree. The changes to hotspot tree remain the same as reviewed previously by David Holmes, Krystal Mo and Remi Forax. Please review the jdk tree changes (to the old verifier). Updated webrevs hotspot-rt tree : http://cr.openjdk.java.net/~bharadwaj/8007736/hotspot-rt/webrev jdk8/tl/jdl tree : http://cr.openjdk.java.net/~bharadwaj/8007736/jdk8-tl-jdk/webrev/ Bug fixed: https://jbs.oracle.com/bugs/browse/JDK-8007736 Summary of changes : Java 8 allows public static interface methods. To accommodate for this, the proposed change modifies bytecode verification of Java 8 classfiles to allow invokestatic to refer to static interface methods in CONSTANT_InterfaceMethodref as well as static class methods in CONSTANT_Methodref. New test added : A new jtreg test in hotspot/test/runtime is added. Thanks, Bharadwaj
Re: RFR (S) : JDK-8007736 - VerifyError for use of static method in interface
Mandy, Thanks for the review. I made the suggested changes and updated the webrev. Thanks, Bharadwaj On 2/13/2013 12:29 PM, Mandy Chung wrote: Bharadwaj, I'm including core-libs-dev so that they know this fix is coming. http://cr.openjdk.java.net/~bharadwaj/8007736/jdk8-tl-jdk/webrev/ The jdk change looks okay to me. Minor nit: L1250-1251: some identation will help. You can follow the original formatting - breaking '?' and ':" in the newlines. Can you include the new regression test in the jdk repo as well (test/vm/verifier directory)? That'll make sure this test is run by core-libs and JPRT job (it's part of the default testset). Thanks Mandy On 2/12/2013 7:44 PM, S. Bharadwaj Yadavalli wrote: On 2/12/2013 2:17 PM, Karen Kinnear wrote: Code looks good. I like this length of test too :-) Thanks for the review, Karen. Please check -XX:-UseSplitVerifier to see if we also need to fix the old verifier for new classfiles. I checked with -XX:-UseSplitVerifier and found that I needed to fix the old verifier. I updated the webrev with my changes which now need to be made to jdk tree as well as hotspot tree. The changes to hotspot tree remain the same as reviewed previously by David Holmes, Krystal Mo and Remi Forax. Please review the jdk tree changes (to the old verifier). Updated webrevs hotspot-rt tree : http://cr.openjdk.java.net/~bharadwaj/8007736/hotspot-rt/webrev jdk8/tl/jdl tree : http://cr.openjdk.java.net/~bharadwaj/8007736/jdk8-tl-jdk/webrev/ Bug fixed: https://jbs.oracle.com/bugs/browse/JDK-8007736 Summary of changes : Java 8 allows public static interface methods. To accommodate for this, the proposed change modifies bytecode verification of Java 8 classfiles to allow invokestatic to refer to static interface methods in CONSTANT_InterfaceMethodref as well as static class methods in CONSTANT_Methodref. New test added : A new jtreg test in hotspot/test/runtime is added. Thanks, Bharadwaj
Re: RFR (S) : JDK-8007736 - VerifyError for use of static method in interface
Thumbs up. Thanks for adding the regression test. Is there any test in jdk/test/ProblemList.txt excluded due to this bug? AFAIK no but just to double check. Thanks Mandy On 2/13/2013 10:04 AM, S. Bharadwaj Yadavalli wrote: Mandy, Thanks for the review. I made the suggested changes and updated the webrev. Thanks, Bharadwaj On 2/13/2013 12:29 PM, Mandy Chung wrote: Bharadwaj, I'm including core-libs-dev so that they know this fix is coming. http://cr.openjdk.java.net/~bharadwaj/8007736/jdk8-tl-jdk/webrev/ The jdk change looks okay to me. Minor nit: L1250-1251: some identation will help. You can follow the original formatting - breaking '?' and ':" in the newlines. Can you include the new regression test in the jdk repo as well (test/vm/verifier directory)? That'll make sure this test is run by core-libs and JPRT job (it's part of the default testset). Thanks Mandy On 2/12/2013 7:44 PM, S. Bharadwaj Yadavalli wrote: On 2/12/2013 2:17 PM, Karen Kinnear wrote: Code looks good. I like this length of test too :-) Thanks for the review, Karen. Please check -XX:-UseSplitVerifier to see if we also need to fix the old verifier for new classfiles. I checked with -XX:-UseSplitVerifier and found that I needed to fix the old verifier. I updated the webrev with my changes which now need to be made to jdk tree as well as hotspot tree. The changes to hotspot tree remain the same as reviewed previously by David Holmes, Krystal Mo and Remi Forax. Please review the jdk tree changes (to the old verifier). Updated webrevs hotspot-rt tree : http://cr.openjdk.java.net/~bharadwaj/8007736/hotspot-rt/webrev jdk8/tl/jdl tree : http://cr.openjdk.java.net/~bharadwaj/8007736/jdk8-tl-jdk/webrev/ Bug fixed: https://jbs.oracle.com/bugs/browse/JDK-8007736 Summary of changes : Java 8 allows public static interface methods. To accommodate for this, the proposed change modifies bytecode verification of Java 8 classfiles to allow invokestatic to refer to static interface methods in CONSTANT_InterfaceMethodref as well as static class methods in CONSTANT_Methodref. New test added : A new jtreg test in hotspot/test/runtime is added. Thanks, Bharadwaj
