From c25c4896ad9ef031e3cddec493aef66ff87c48a7 Mon Sep 17 00:00:00 2001 From: Adam Sotona Date: Fri, 19 Jul 2024 13:09:45 +0000 Subject: [PATCH] 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors Reviewed-by: liach --- .../classfile/impl/BoundAttribute.java | 12 +++++++-- .../classfile/impl/ClassFileImpl.java | 6 ++++- .../classfile/impl/verifier/VerifierImpl.java | 7 ++--- test/jdk/jdk/classfile/VerifierSelfTest.java | 26 ++++++++++++++++++- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java b/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java index 175c4adb927..6a23be54f48 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java @@ -286,7 +286,11 @@ public abstract sealed class BoundAttribute> public BoundLocalVariableTableAttribute(AttributedElement enclosing, ClassReader cf, AttributeMapper mapper, int pos) { super(cf, mapper, pos); - codeAttribute = (CodeImpl) enclosing; + if (enclosing instanceof CodeImpl ci) { + this.codeAttribute = ci; + } else { + throw new IllegalArgumentException("Invalid LocalVariableTable attribute location"); + } } @Override @@ -313,7 +317,11 @@ public abstract sealed class BoundAttribute> public BoundLocalVariableTypeTableAttribute(AttributedElement enclosing, ClassReader cf, AttributeMapper mapper, int pos) { super(cf, mapper, pos); - this.codeAttribute = (CodeImpl) enclosing; + if (enclosing instanceof CodeImpl ci) { + this.codeAttribute = ci; + } else { + throw new IllegalArgumentException("Invalid LocalVariableTypeTable attribute location"); + } } @Override diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java index e4408d6c7b2..8de62e7a12b 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java @@ -132,7 +132,11 @@ public record ClassFileImpl(StackMapsOption stackMapsOption, @Override public List verify(ClassModel model) { - return VerifierImpl.verify(model, classHierarchyResolverOption().classHierarchyResolver(), null); + try { + return VerifierImpl.verify(model, classHierarchyResolverOption().classHierarchyResolver(), null); + } catch (IllegalArgumentException verifierInitializationError) { + return List.of(new VerifyError(verifierInitializationError.getMessage())); + } } @Override diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java index 11e7256751e..33733c5572a 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java @@ -114,9 +114,10 @@ public final class VerifierImpl { } public static List verify(ClassModel classModel, ClassHierarchyResolver classHierarchyResolver, Consumer logger) { - var klass = new VerificationWrapper(classModel); - log_info(logger, "Start class verification for: %s", klass.thisClassName()); + String clsName = classModel.thisClass().asInternalName(); + log_info(logger, "Start class verification for: %s", clsName); try { + var klass = new VerificationWrapper(classModel); var errors = new ArrayList(); errors.addAll(new ParserVerifier(classModel).verify()); if (is_eligible_for_verification(klass)) { @@ -134,7 +135,7 @@ public final class VerifierImpl { } return errors; } finally { - log_info(logger, "End class verification for: %s", klass.thisClassName()); + log_info(logger, "End class verification for: %s", clsName); } } diff --git a/test/jdk/jdk/classfile/VerifierSelfTest.java b/test/jdk/jdk/classfile/VerifierSelfTest.java index 26513a54335..84a5ded1610 100644 --- a/test/jdk/jdk/classfile/VerifierSelfTest.java +++ b/test/jdk/jdk/classfile/VerifierSelfTest.java @@ -24,6 +24,7 @@ /* * @test * @summary Testing ClassFile Verifier. + * @bug 8333812 * @enablePreview * @run junit VerifierSelfTest */ @@ -46,8 +47,10 @@ import java.lang.classfile.*; import java.lang.classfile.attribute.*; import java.lang.classfile.components.ClassPrinter; import java.lang.constant.ModuleDesc; +import jdk.internal.classfile.impl.DirectClassBuilder; +import jdk.internal.classfile.impl.UnboundAttribute; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; class VerifierSelfTest { @@ -94,6 +97,27 @@ class VerifierSelfTest { } } + @Test + void testInvalidAttrLocation() { + var cc = ClassFile.of(); + var bytes = cc.build(ClassDesc.of("InvalidAttrLocationClass"), cb -> + ((DirectClassBuilder)cb).writeAttribute(new UnboundAttribute.AdHocAttribute(Attributes.localVariableTable()) { + @Override + public void writeBody(BufWriter b) { + b.writeU2(0); + } + })); + assertTrue(cc.verify(bytes).stream().anyMatch(e -> e.getMessage().contains("Invalid LocalVariableTable attribute location"))); + } + + @Test + void testInvalidClassNameEntry() { + var cc = ClassFile.of(); + var bytes = cc.parse(new byte[]{(byte)0xCA, (byte)0xFE, (byte)0xBA, (byte)0xBE, + 0, 0, 0, 0, 0, 2, ClassFile.TAG_INTEGER, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}); + assertTrue(cc.verify(bytes).stream().anyMatch(e -> e.getMessage().contains("expected ClassEntry"))); + } + @Test void testParserVerification() { var cc = ClassFile.of();