infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-05-11 14:11:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

8361102: java.lang.classfile.CodeBuilder.branch(Opcode op, Label target) doesn't throw IllegalArgumentException - if op is not of Opcode.Kind.BRANCH

Browse Source 
Reviewed-by: asotona
This commit is contained in:
Chen Liang 2025-07-11 22:52:10 +00:00
parent 46988e1073
commit 3f59eae3d0
5 changed files with 144 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java
View File

@ -399,6 +399,7 @@ public final class BufWriterImpl implements BufWriter {
writeU2(cpIndex(entry));
}
// Null checks entry
public void writeIndex(int bytecode, PoolEntry entry) {
writeU1U2(bytecode, cpIndex(entry));
}

40
src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
View File

@ -546,8 +546,8 @@ public final class DirectCodeBuilder
}
private void writeShortJump(int bytecode, Label target) {
int targetBci = labelToBci(target); // implicit null check
int instructionPc = curPc();
int targetBci = labelToBci(target);
// algebraic union of jump | (instructionPc, target), distinguished by null == target.
int jumpOrInstructionPc;
@ -570,6 +570,7 @@ public final class DirectCodeBuilder
}
private void writeLongJump(int bytecode, Label target) {
Objects.requireNonNull(target); // before any write
int instructionPc = curPc();
bytecodesBufWriter.writeU1(bytecode);
writeLongLabelOffset(instructionPc, target);
@ -617,6 +618,17 @@ public final class DirectCodeBuilder
}
public void writeLookupSwitch(Label defaultTarget, List<SwitchCase> cases) {
cases = new ArrayList<>(cases); // cases may be untrusted
for (var each : cases) {
Objects.requireNonNull(each); // single null case may exist
}
cases.sort(new Comparator<>() {
@Override
public int compare(SwitchCase c1, SwitchCase c2) {
return Integer.compare(c1.caseValue(), c2.caseValue());
}
});
// validation end
int instructionPc = curPc();
bytecodesBufWriter.writeU1(LOOKUPSWITCH);
int pad = 4 - (curPc() % 4);
@ -624,13 +636,6 @@ public final class DirectCodeBuilder
bytecodesBufWriter.skip(pad); // padding content can be anything
writeLongLabelOffset(instructionPc, defaultTarget);
bytecodesBufWriter.writeInt(cases.size());
cases = new ArrayList<>(cases);
cases.sort(new Comparator<>() {
@Override
public int compare(SwitchCase c1, SwitchCase c2) {
return Integer.compare(c1.caseValue(), c2.caseValue());
}
});
for (var c : cases) {
bytecodesBufWriter.writeInt(c.caseValue());
var target = c.target();
@ -639,6 +644,11 @@ public final class DirectCodeBuilder
}
public void writeTableSwitch(int low, int high, Label defaultTarget, List<SwitchCase> cases) {
var caseMap = new HashMap<Integer, Label>(cases.size()); // cases may be untrusted
for (var c : cases) {
caseMap.put(c.caseValue(), c.target());
}
// validation end
int instructionPc = curPc();
bytecodesBufWriter.writeU1(TABLESWITCH);
int pad = 4 - (curPc() % 4);
@ -646,10 +656,6 @@ public final class DirectCodeBuilder
bytecodesBufWriter.skip(pad); // padding content can be anything
writeLongLabelOffset(instructionPc, defaultTarget);
bytecodesBufWriter.writeIntInt(low, high);
var caseMap = new HashMap<Integer, Label>(cases.size());
for (var c : cases) {
caseMap.put(c.caseValue(), c.target());
}
for (long l = low; l<=high; l++) {
var target = caseMap.getOrDefault((int)l, defaultTarget);
writeLongLabelOffset(instructionPc, target);
@ -928,6 +934,7 @@ public final class DirectCodeBuilder
int slots = Util.parameterSlots(Util.methodTypeSymbol(ref.type())) + 1;
writeInvokeInterface(opcode, (InterfaceMethodRefEntry) ref, slots);
} else {
Util.checkKind(opcode, Opcode.Kind.INVOKE);
writeInvokeNormal(opcode, ref);
}
return this;
@ -959,7 +966,8 @@ public final class DirectCodeBuilder
@Override
public CodeBuilder fieldAccess(Opcode opcode, FieldRefEntry ref) {
bytecodesBufWriter.writeIndex(opcode.bytecode(), ref);
Util.checkKind(opcode, Opcode.Kind.FIELD_ACCESS);
writeFieldAccess(opcode, ref);
return this;
}
@ -977,6 +985,7 @@ public final class DirectCodeBuilder
@Override
public CodeBuilder branch(Opcode op, Label target) {
Util.checkKind(op, Opcode.Kind.BRANCH);
writeBranch(op, target);
return this;
}
@ -1640,6 +1649,8 @@ public final class DirectCodeBuilder
@Override
public CodeBuilder lookupswitch(Label defaultTarget, List<SwitchCase> cases) {
Objects.requireNonNull(defaultTarget);
// check cases when we sort them
writeLookupSwitch(defaultTarget, cases);
return this;
}
@ -1799,6 +1810,7 @@ public final class DirectCodeBuilder
@Override
public CodeBuilder multianewarray(ClassEntry array, int dims) {
BytecodeHelpers.validateMultiArrayDimensions(dims);
writeNewMultidimensionalArray(dims, array);
return this;
}
@ -1845,6 +1857,8 @@ public final class DirectCodeBuilder
@Override
public CodeBuilder tableswitch(int low, int high, Label defaultTarget, List<SwitchCase> cases) {
Objects.requireNonNull(defaultTarget);
// check cases when we write them
writeTableSwitch(low, high, defaultTarget, cases);
return this;
}

108
test/jdk/jdk/classfile/InstructionValidationTest.java
View File

@ -23,7 +23,7 @@
/*
* @test
* @bug 8341277
* @bug 8341277 8361102
* @summary Testing ClassFile instruction argument validation.
* @run junit InstructionValidationTest
*/
@ -32,24 +32,77 @@ import java.lang.classfile.*;
import java.lang.classfile.constantpool.ClassEntry;
import java.lang.classfile.constantpool.ConstantPoolBuilder;
import java.lang.classfile.instruction.*;
import java.lang.constant.ClassDesc;
import java.lang.reflect.Parameter;
import java.util.Collections;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.ObjIntConsumer;
import java.util.stream.Stream;
import helpers.TestUtil;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
import static java.lang.classfile.ClassFile.ACC_STATIC;
import static java.lang.constant.ConstantDescs.*;
import static helpers.TestConstants.*;
import static org.junit.jupiter.api.Assertions.*;
import static java.lang.classfile.Opcode.*;
import static org.junit.jupiter.api.Assertions.assertThrows;
class InstructionValidationTest {
@Test
void testOpcodeInCodeBuilder() {
TestUtil.runCodeHandler(cob -> {
var mref = cob.constantPool().methodRefEntry(CD_System, "exit", MTD_INT_VOID);
var fref = cob.constantPool().fieldRefEntry(CD_System, "out", CD_PrintStream);
var label = cob.newLabel();
// Sanity
cob.iconst_0();
assertDoesNotThrow(() -> cob.invoke(INVOKESTATIC, mref));
assertDoesNotThrow(() -> cob.fieldAccess(GETSTATIC, fref));
cob.pop();
assertDoesNotThrow(() -> cob.branch(GOTO, label));
// Opcode NPE
assertThrows(NullPointerException.class, () -> cob.invoke(null, mref));
assertThrows(NullPointerException.class, () -> cob.fieldAccess(null, fref));
assertThrows(NullPointerException.class, () -> cob.branch(null, label));
// Opcode IAE
assertThrows(IllegalArgumentException.class, () -> cob.invoke(IFNE, mref));
assertThrows(IllegalArgumentException.class, () -> cob.fieldAccess(JSR, fref));
assertThrows(IllegalArgumentException.class, () -> cob.branch(CHECKCAST, label));
// Wrap up
cob.labelBinding(label);
cob.return_();
});
}
@Test
void testLongJump() {
TestUtil.runCodeHandler(cob -> {
assertThrows(NullPointerException.class, () -> cob.goto_w(null));
// Ensures nothing redundant is written in case of failure
cob.return_();
});
}
@Test
void testSwitch() {
TestUtil.runCodeHandler(cob -> {
assertThrows(NullPointerException.class, () -> cob.tableswitch(-1, 1, cob.startLabel(), null));
assertThrows(NullPointerException.class, () -> cob.lookupswitch(cob.startLabel(), null));
assertThrows(NullPointerException.class, () -> cob.tableswitch(-1, 1, cob.startLabel(), Collections.singletonList(null)));
assertThrows(NullPointerException.class, () -> cob.lookupswitch(cob.startLabel(), Collections.singletonList(null)));
assertThrows(NullPointerException.class, () -> cob.tableswitch(-1, 1, null, List.of()));
assertThrows(NullPointerException.class, () -> cob.lookupswitch(null, List.of()));
// Ensures nothing redundant is written in case of failure
cob.return_();
});
}
@Test
void testArgumentConstant() {
assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, 0));
@ -63,6 +116,14 @@ class InstructionValidationTest {
assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(SIPUSH, (int) Short.MAX_VALUE + 1));
assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(BIPUSH, (int) Byte.MIN_VALUE - 1));
assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(BIPUSH, (int) Byte.MAX_VALUE + 1));
TestUtil.runCodeHandler(cob -> {
assertThrows(IllegalArgumentException.class, () -> cob.sipush((int) Short.MIN_VALUE - 1));
assertThrows(IllegalArgumentException.class, () -> cob.sipush((int) Short.MAX_VALUE + 1));
assertThrows(IllegalArgumentException.class, () -> cob.bipush((int) Byte.MIN_VALUE - 1));
assertThrows(IllegalArgumentException.class, () -> cob.bipush((int) Byte.MAX_VALUE + 1));
cob.return_();
});
}
/**
@ -163,26 +224,15 @@ class InstructionValidationTest {
// CodeBuilder can fail with IAE due to other reasons, so we cannot check
// "success" but can ensure things fail fast
static void ensureFailFast(int value, Consumer<CodeBuilder> action) {
// Can fail with AssertionError instead of IAE
Consumer<CodeBuilder> checkedAction = cob -> {
action.accept(cob);
fail("Bad slot access did not fail fast: " + value);
assertThrows(IllegalArgumentException.class, () -> action.accept(cob));
cob.return_();
};
var cf = ClassFile.of();
CodeTransform noopCodeTransform = CodeBuilder::with;
// Direct builders
assertThrows(IllegalArgumentException.class, () -> cf.build(ClassDesc.of("Test"), clb -> clb
.withMethodBody("test", MTD_void, ACC_STATIC, checkedAction)));
// Chained builders
assertThrows(IllegalArgumentException.class, () -> cf.build(ClassDesc.of("Test"), clb -> clb
.withMethodBody("test", MTD_void, ACC_STATIC, cob -> cob
.transforming(CodeTransform.ACCEPT_ALL, checkedAction))));
var classTemplate = cf.build(ClassDesc.of("Test"), clb -> clb
.withMethodBody("test", MTD_void, ACC_STATIC, CodeBuilder::return_));
// Indirect builders
assertThrows(IllegalArgumentException.class, () -> cf.transformClass(cf.parse(classTemplate),
ClassTransform.transformingMethodBodies(CodeTransform.endHandler(checkedAction)
.andThen(noopCodeTransform))));
try {
TestUtil.runCodeHandler(checkedAction);
} catch (Throwable _) {
System.out.printf("Erroneous value %d%n", value);
}
}
static void check(boolean fails, Executable exec) {
@ -200,7 +250,10 @@ class InstructionValidationTest {
IncrementInstruction.of(0, Short.MIN_VALUE);
for (int i : new int[] {Short.MIN_VALUE - 1, Short.MAX_VALUE + 1}) {
assertThrows(IllegalArgumentException.class, () -> IncrementInstruction.of(0, i));
ensureFailFast(i, cob -> cob.iinc(0, i));
TestUtil.runCodeHandler(cob -> {
assertThrows(IllegalArgumentException.class, () -> cob.iinc(0, i));
cob.return_();
});
}
}
@ -215,5 +268,14 @@ class InstructionValidationTest {
assertThrows(IllegalArgumentException.class, () -> NewMultiArrayInstruction.of(ce, -1));
assertThrows(IllegalArgumentException.class, () -> NewMultiArrayInstruction.of(ce, Integer.MIN_VALUE));
assertThrows(IllegalArgumentException.class, () -> NewMultiArrayInstruction.of(ce, Integer.MAX_VALUE));
TestUtil.runCodeHandler(cob -> {
assertThrows(IllegalArgumentException.class, () -> cob.multianewarray(ce, 0));
assertThrows(IllegalArgumentException.class, () -> cob.multianewarray(ce, 0x100));
assertThrows(IllegalArgumentException.class, () -> cob.multianewarray(ce, -1));
assertThrows(IllegalArgumentException.class, () -> cob.multianewarray(ce, Integer.MIN_VALUE));
assertThrows(IllegalArgumentException.class, () -> cob.multianewarray(ce, Integer.MAX_VALUE));
cob.return_();
});
}
}

3
test/jdk/jdk/classfile/TEST.properties
View File

@ -2,4 +2,5 @@ maxOutputSize = 2500000
modules = \
java.base/jdk.internal.classfile.components \
java.base/jdk.internal.classfile.impl \
java.base/jdk.internal.classfile.impl.verifier
java.base/jdk.internal.classfile.impl.verifier
lib.dirs = helpers

43
test/jdk/jdk/classfile/helpers/TestUtil.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 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
@ -24,29 +24,44 @@ package helpers;
import jdk.internal.classfile.impl.LabelContext;
import jdk.internal.classfile.impl.LabelImpl;
import java.lang.classfile.ClassFile;
import java.lang.classfile.ClassTransform;
import java.lang.classfile.CodeBuilder;
import java.lang.classfile.CodeTransform;
import java.lang.classfile.instruction.LocalVariable;
import java.lang.classfile.instruction.LocalVariableType;
import java.io.FileOutputStream;
import java.lang.constant.ClassDesc;
import java.util.Collection;
import java.util.function.Consumer;
public class TestUtil {
import static java.lang.classfile.ClassFile.ACC_STATIC;
import static java.lang.constant.ConstantDescs.MTD_void;
public final class TestUtil {
/// Run a code handler in different builders.
public static void runCodeHandler(Consumer<CodeBuilder> handler) {
ClassFile cf = ClassFile.of();
// Direct builders
cf.build(ClassDesc.of("Test"), clb -> clb.withMethodBody("test", MTD_void, ACC_STATIC, handler));
// Chained builders
cf.build(ClassDesc.of("Test"), clb -> clb
.withMethodBody("test", MTD_void, ACC_STATIC, cob -> cob
.transforming(CodeTransform.ACCEPT_ALL, handler)));
// Indirect builders
var classTemplate = cf.build(ClassDesc.of("Test"), clb -> clb
.withMethodBody("test", MTD_void, ACC_STATIC, CodeBuilder::return_));
cf.transformClass(cf.parse(classTemplate),
ClassTransform.transformingMethodBodies(CodeTransform.endHandler(handler)
.andThen(CodeBuilder::with)));
}
public static void assertEmpty(Collection<?> col) {
if (!col.isEmpty()) throw new AssertionError(col);
}
public static void writeClass(byte[] bytes, String fn) {
try {
FileOutputStream out = new FileOutputStream(fn);
out.write(bytes);
out.close();
} catch (Exception ex) {
throw new InternalError(ex);
}
}
public static class ExpectedLvRecord {
int slot;
String desc;