8361638: java.lang.classfile.CodeBuilder.CatchBuilder should not throw IllegalArgumentException for representable exception handlers

Reviewed-by: asotona
This commit is contained in:
Chen Liang 2025-08-15 04:25:37 +00:00
parent 44b19c01ac
commit 6fb6f3d39b
4 changed files with 71 additions and 32 deletions

View File

@ -330,6 +330,11 @@ public sealed interface CodeBuilder
/**
* A builder to add catch blocks.
* <p>
* 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
*/

View File

@ -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}.
* <p>
* An exception table entry is composite:
* {@snippet lang=text :

View File

@ -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<ConstantDesc> 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<ClassEntry> 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);

View File

@ -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<CodeBuilder.BlockCodeBuilder> 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 -> {