From 6fb6f3d39b321e2a1c1fa2cef2c19222a6dcf7b9 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Fri, 15 Aug 2025 04:25:37 +0000 Subject: [PATCH] 8361638: java.lang.classfile.CodeBuilder.CatchBuilder should not throw IllegalArgumentException for representable exception handlers Reviewed-by: asotona --- .../java/lang/classfile/CodeBuilder.java | 28 +++++++++++---- .../classfile/instruction/ExceptionCatch.java | 6 ++-- .../classfile/impl/CatchBuilderImpl.java | 35 ++++++++++--------- .../jdk/classfile/BuilderTryCatchTest.java | 34 ++++++++++++++---- 4 files changed, 71 insertions(+), 32 deletions(-) diff --git a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java index fe82d5d0bf0..c1070bbcc77 100644 --- a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java +++ b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java @@ -330,6 +330,11 @@ public sealed interface CodeBuilder /** * A builder to add catch blocks. + *

+ * The order of catch blocks is significant. When an exception is thrown + * by the try block, the first catch block whose exception type is {@linkplain + * Class#isAssignableFrom(Class) the same class as or a superclass of} the + * class of exception thrown is branched to (JVMS {@jvms 2.10}). * * @see #trying * @see ExceptionCatch @@ -348,13 +353,16 @@ public sealed interface CodeBuilder * If the type of exception is {@code null} then the catch block catches * all exceptions. * + * @apiNote + * If the type of exception to catch is already handled by previous + * catch blocks, this block will never be executed. + * * @param exceptionType the type of exception to catch, may be {@code null} * @param catchHandler handler that receives a {@link BlockCodeBuilder} to * generate the body of the catch block * @return this builder - * @throws IllegalArgumentException if an existing catch block catches - * an exception of the given type or {@code exceptionType} - * represents a primitive type + * @throws IllegalArgumentException if {@code exceptionType} represents + * a primitive type * @see #catchingMulti * @see #catchingAll */ @@ -372,12 +380,16 @@ public sealed interface CodeBuilder * If list of exception types is empty then the catch block catches all * exceptions. * + * @apiNote + * If every type of exception to catch is already handled by previous + * catch blocks, this block will never be executed. + * * @param exceptionTypes the types of exception to catch * @param catchHandler handler that receives a {@link BlockCodeBuilder} * to generate the body of the catch block * @return this builder - * @throws IllegalArgumentException if an existing catch block catches - * one or more exceptions of the given types + * @throws IllegalArgumentException if any exception type represents a + * primitive type * @see #catching * @see #catchingAll */ @@ -392,10 +404,12 @@ public sealed interface CodeBuilder * The caught exception will be on top of the operand stack when the * catch block is entered. * + * @apiNote + * Since this block intercepts all exceptions, all subsequent catch + * blocks will never be executed. + * * @param catchAllHandler handler that receives a {@link BlockCodeBuilder} * to generate the body of the catch block - * @throws IllegalArgumentException if an existing catch block catches - * all exceptions * @see #catching * @see #catchingMulti */ diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ExceptionCatch.java b/src/java.base/share/classes/java/lang/classfile/instruction/ExceptionCatch.java index e924ca718e7..b0e5af2941e 100644 --- a/src/java.base/share/classes/java/lang/classfile/instruction/ExceptionCatch.java +++ b/src/java.base/share/classes/java/lang/classfile/instruction/ExceptionCatch.java @@ -39,8 +39,10 @@ import jdk.internal.classfile.impl.AbstractPseudoInstruction; * A pseudo-instruction modeling an entry in the {@code exception_table} array * of a {@link CodeAttribute Code} attribute. Catch (JVMS {@jvms 3.12}) and * finally (JVMS {@jvms 3.14}) blocks in Java source code compile to exception - * table entries. Delivered as a {@link CodeElement} when traversing the - * contents of a {@link CodeModel}. + * table entries. The order of exception table entries is significant: when an + * exception is thrown in a method, execution branches to the first matching + * exception handler if such a handler exists (JVMS {@jvms 2.10}). Delivered as + * a {@link CodeElement} when traversing the contents of a {@link CodeModel}. *

* An exception table entry is composite: * {@snippet lang=text : diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/CatchBuilderImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/CatchBuilderImpl.java index d520753326e..89d43135551 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/CatchBuilderImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/CatchBuilderImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -27,8 +27,9 @@ package jdk.internal.classfile.impl; import java.lang.classfile.CodeBuilder; import java.lang.classfile.Label; import java.lang.classfile.Opcode; +import java.lang.classfile.constantpool.ClassEntry; import java.lang.constant.ClassDesc; -import java.lang.constant.ConstantDesc; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -39,14 +40,12 @@ public final class CatchBuilderImpl implements CodeBuilder.CatchBuilder { final CodeBuilder b; final BlockCodeBuilderImpl tryBlock; final Label tryCatchEnd; - final Set catchTypes; BlockCodeBuilderImpl catchBlock; public CatchBuilderImpl(CodeBuilder b, BlockCodeBuilderImpl tryBlock, Label tryCatchEnd) { this.b = b; this.tryBlock = tryBlock; this.tryCatchEnd = tryCatchEnd; - this.catchTypes = new HashSet<>(); } @Override @@ -59,18 +58,24 @@ public final class CatchBuilderImpl implements CodeBuilder.CatchBuilder { Objects.requireNonNull(exceptionTypes); Objects.requireNonNull(catchHandler); + // nullable list of CP entries - null means catching all (0) + List entries = new ArrayList<>(Math.max(1, exceptionTypes.size())); + if (exceptionTypes.isEmpty()) { + entries.add(null); + } else { + for (var exceptionType : exceptionTypes) { + var entry = b.constantPool().classEntry(exceptionType); // throws IAE + entries.add(entry); + } + } + // End validation + if (catchBlock == null) { if (tryBlock.reachable()) { b.branch(Opcode.GOTO, tryCatchEnd); } } - for (var exceptionType : exceptionTypes) { - if (!catchTypes.add(exceptionType)) { - throw new IllegalArgumentException("Existing catch block catches exception of type: " + exceptionType); - } - } - // Finish prior catch block if (catchBlock != null) { catchBlock.end(); @@ -82,13 +87,9 @@ public final class CatchBuilderImpl implements CodeBuilder.CatchBuilder { catchBlock = new BlockCodeBuilderImpl(b, tryCatchEnd); Label tryStart = tryBlock.startLabel(); Label tryEnd = tryBlock.endLabel(); - if (exceptionTypes.isEmpty()) { - catchBlock.exceptionCatchAll(tryStart, tryEnd, catchBlock.startLabel()); - } - else { - for (var exceptionType : exceptionTypes) { - catchBlock.exceptionCatch(tryStart, tryEnd, catchBlock.startLabel(), exceptionType); - } + for (var entry : entries) { + // This accepts null for catching all + catchBlock.exceptionCatch(tryStart, tryEnd, catchBlock.startLabel(), entry); } catchBlock.start(); catchHandler.accept(catchBlock); diff --git a/test/jdk/jdk/classfile/BuilderTryCatchTest.java b/test/jdk/jdk/classfile/BuilderTryCatchTest.java index 3de12559163..0a966e2a655 100644 --- a/test/jdk/jdk/classfile/BuilderTryCatchTest.java +++ b/test/jdk/jdk/classfile/BuilderTryCatchTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,7 @@ /* * @test + * @bug 8361638 * @summary Testing ClassFile builder blocks. * @run junit BuilderTryCatchTest */ @@ -37,6 +38,7 @@ import java.lang.classfile.instruction.ExceptionCatch; import static java.lang.classfile.ClassFile.ACC_PUBLIC; import static java.lang.classfile.ClassFile.ACC_STATIC; +import static java.lang.constant.ConstantDescs.*; import static org.junit.jupiter.api.Assertions.*; import org.junit.jupiter.api.Test; @@ -47,20 +49,40 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collections; import java.util.List; import java.util.function.Consumer; -import static java.lang.constant.ConstantDescs.CD_Double; -import static java.lang.constant.ConstantDescs.CD_Integer; -import static java.lang.constant.ConstantDescs.CD_Object; -import static java.lang.constant.ConstantDescs.CD_String; - class BuilderTryCatchTest { static final ClassDesc CD_IOOBE = IndexOutOfBoundsException.class.describeConstable().get(); static final ClassDesc CD_NPE = NullPointerException.class.describeConstable().get(); static final MethodTypeDesc MTD_String = MethodType.methodType(String.class).describeConstable().get(); + @Test + void testExceptionalContracts() throws Throwable { + generateTryCatchMethod(catchBuilder -> { + Consumer handler = tb -> tb.pop().aconst_null().areturn(); + assertThrows(NullPointerException.class, () -> catchBuilder.catching(CD_NPE, null)); + assertThrows(NullPointerException.class, () -> catchBuilder.catchingMulti(null, handler)); + assertThrows(NullPointerException.class, () -> catchBuilder.catchingMulti(List.of(), null)); + assertThrows(NullPointerException.class, () -> catchBuilder.catchingMulti(Collections.singletonList(null), null)); + assertThrows(NullPointerException.class, () -> catchBuilder.catchingAll(null)); + catchBuilder.catchingMulti(List.of(CD_IOOBE, CD_NPE), tb -> { + tb.invokevirtual(CD_Object, "toString", MTD_String); + tb.astore(1); + }); + catchBuilder.catchingAll(tb -> tb.pop().loadConstant("all").areturn()); + assertThrows(IllegalArgumentException.class, () -> catchBuilder.catching(CD_int, handler)); + assertDoesNotThrow(() -> catchBuilder.catching(CD_NPE, handler)); + assertDoesNotThrow(() -> catchBuilder.catching(null, handler)); + assertDoesNotThrow(() -> catchBuilder.catchingMulti(List.of(), handler)); + assertDoesNotThrow(() -> catchBuilder.catchingMulti(List.of(CD_Exception, CD_IOOBE), handler)); + assertThrows(IllegalArgumentException.class, () -> catchBuilder.catchingMulti(List.of(CD_long, CD_Throwable), handler)); + assertDoesNotThrow(() -> catchBuilder.catchingAll(handler)); + }); + } + @Test void testTryCatchCatchAll() throws Throwable { byte[] bytes = generateTryCatchMethod(catchBuilder -> {