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 fd63788336fd3..75413011731e4 100644 --- a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java +++ b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java @@ -604,23 +604,6 @@ default CodeBuilder conversion(TypeKind fromType, TypeKind toType) { return this; } - /** - * Generate an instruction pushing a constant onto the operand stack - * @see Opcode.Kind#CONSTANT - * @param opcode the constant instruction opcode - * @param value the constant value - * @return this builder - * @since 23 - */ - default CodeBuilder loadConstant(Opcode opcode, ConstantDesc value) { - BytecodeHelpers.validateValue(opcode, value); - return with(switch (opcode) { - case SIPUSH, BIPUSH -> ConstantInstruction.ofArgument(opcode, ((Number)value).intValue()); - case LDC, LDC_W, LDC2_W -> ConstantInstruction.ofLoad(opcode, BytecodeHelpers.constantEntry(constantPool(), value)); - default -> ConstantInstruction.ofIntrinsic(opcode); - }); - } - /** * Generate an instruction pushing a constant onto the operand stack * @param value the constant value @@ -931,12 +914,12 @@ default CodeBuilder bastore() { } /** - * Generate an instruction pushing a byte onto the operand stack - * @param b the byte + * Generate an instruction pushing an int in the range of byte onto the operand stack. + * @param b the int in the range of byte * @return this builder */ default CodeBuilder bipush(int b) { - return loadConstant(Opcode.BIPUSH, b); + return with(ConstantInstruction.ofArgument(Opcode.BIPUSH, b)); } /** @@ -2396,12 +2379,12 @@ default CodeBuilder sastore() { } /** - * Generate an instruction pushing a short onto the operand stack - * @param s the short + * Generate an instruction pushing an int in the range of short onto the operand stack. + * @param s the int in the range of short * @return this builder */ default CodeBuilder sipush(int s) { - return loadConstant(Opcode.SIPUSH, s); + return with(ConstantInstruction.ofArgument(Opcode.SIPUSH, s)); } /** diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java index a7ba9e2a6a10f..022c45fdeef4d 100644 --- a/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java +++ b/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2024, 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 @@ -33,6 +33,7 @@ import java.lang.classfile.TypeKind; import java.lang.classfile.constantpool.LoadableConstantEntry; import jdk.internal.classfile.impl.AbstractInstruction; +import jdk.internal.classfile.impl.BytecodeHelpers; import jdk.internal.classfile.impl.Util; import jdk.internal.javac.PreviewFeature; @@ -144,16 +145,21 @@ static IntrinsicConstantInstruction ofIntrinsic(Opcode op) { /** * {@return an argument constant instruction} * - * @param op the opcode for the specific type of intrinsic constant instruction, - * which must be of kind {@link Opcode.Kind#CONSTANT} + * @param op the opcode for the specific type of argument constant instruction, + * which must be {@link Opcode#BIPUSH} or {@link Opcode#SIPUSH} * @param value the constant value * @throws IllegalArgumentException if the opcode is not {@link Opcode#BIPUSH} - * or {@link Opcode#SIPUSH} + * or {@link Opcode#SIPUSH}, or if the constant value is out of range + * for the opcode */ static ArgumentConstantInstruction ofArgument(Opcode op, int value) { - Util.checkKind(op, Opcode.Kind.CONSTANT); - if (op != Opcode.BIPUSH && op != Opcode.SIPUSH) + if (op == Opcode.BIPUSH) { + BytecodeHelpers.validateBipush(value); + } else if (op == Opcode.SIPUSH) { + BytecodeHelpers.validateSipush(value); + } else { throw new IllegalArgumentException(String.format("Wrong opcode specified; found %s, expected BIPUSH or SIPUSH", op)); + } return new AbstractInstruction.UnboundArgumentConstantInstruction(op, value); } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java b/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java index 058be41198b73..e52d198e1f423 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java @@ -277,40 +277,20 @@ public static Opcode convertOpcode(TypeKind from, TypeKind to) { }; } - static void validateSipush(long value) { - if (value < Short.MIN_VALUE || Short.MAX_VALUE < value) + public static void validateSipush(int value) { + if (value != (short) value) throw new IllegalArgumentException( "SIPUSH: value must be within: Short.MIN_VALUE <= value <= Short.MAX_VALUE, found: " .concat(Long.toString(value))); } - static void validateBipush(long value) { - if (value < Byte.MIN_VALUE || Byte.MAX_VALUE < value) + public static void validateBipush(int value) { + if (value != (byte) value) throw new IllegalArgumentException( "BIPUSH: value must be within: Byte.MIN_VALUE <= value <= Byte.MAX_VALUE, found: " .concat(Long.toString(value))); } - static void validateSipush(ConstantDesc d) { - if (d instanceof Integer iVal) { - validateSipush(iVal.longValue()); - } else if (d instanceof Long lVal) { - validateSipush(lVal.longValue()); - } else { - throw new IllegalArgumentException("SIPUSH: not an integral number: ".concat(d.toString())); - } - } - - static void validateBipush(ConstantDesc d) { - if (d instanceof Integer iVal) { - validateBipush(iVal.longValue()); - } else if (d instanceof Long lVal) { - validateBipush(lVal.longValue()); - } else { - throw new IllegalArgumentException("BIPUSH: not an integral number: ".concat(d.toString())); - } - } - public static MethodHandleEntry handleDescToHandleInfo(ConstantPoolBuilder constantPool, DirectMethodHandleDesc bootstrapMethod) { ClassEntry bsOwner = constantPool.classEntry(bootstrapMethod.owner()); NameAndTypeEntry bsNameAndType = constantPool.nameAndTypeEntry(constantPool.utf8Entry(bootstrapMethod.methodName()), @@ -341,32 +321,6 @@ static ConstantDynamicEntry handleConstantDescToHandleInfo(ConstantPoolBuilder c desc.constantType())); } - public static void validateValue(Opcode opcode, ConstantDesc v) { - switch (opcode) { - case ACONST_NULL -> { - if (v != null && v != ConstantDescs.NULL) - throw new IllegalArgumentException("value must be null or ConstantDescs.NULL with opcode ACONST_NULL"); - } - case SIPUSH -> - validateSipush(v); - case BIPUSH -> - validateBipush(v); - case LDC, LDC_W, LDC2_W -> { - if (v == null) - throw new IllegalArgumentException("`null` must use ACONST_NULL"); - } - default -> { - var exp = opcode.constantValue(); - if (exp == null) - throw new IllegalArgumentException("Can not use Opcode: " + opcode + " with constant()"); - if (v == null || !(v.equals(exp) || (exp instanceof Long l && v.equals(l.intValue())))) { - var t = (exp instanceof Long) ? "L" : (exp instanceof Float) ? "f" : (exp instanceof Double) ? "d" : ""; - throw new IllegalArgumentException("value must be " + exp + t + " with opcode " + opcode.name()); - } - } - } - } - public static Opcode ldcOpcode(LoadableConstantEntry entry) { return entry.typeKind().slotSize() == 2 ? Opcode.LDC2_W : entry.index() > 0xff ? Opcode.LDC_W diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java index 9b5e7639fa2a4..1d1d028531ecb 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java @@ -258,8 +258,7 @@ public CodeTransform asCodeTransform() { cob.localVariableType(c.slot(), c.name().stringValue(), mapSignature(c.signatureSymbol()), c.startScope(), c.endScope()); case LoadConstantInstruction ldc -> - cob.loadConstant(ldc.opcode(), - mapConstantValue(ldc.constantValue())); + cob.ldc(mapConstantValue(ldc.constantValue())); case RuntimeVisibleTypeAnnotationsAttribute aa -> cob.with(RuntimeVisibleTypeAnnotationsAttribute.of( mapTypeAnnotations(aa.annotations()))); diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java b/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java index e2dcc982e6b5b..1b950f28d004b 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java @@ -831,21 +831,6 @@ public CodeBuilder branch(Opcode op, Label target) { return this; } - @Override - public CodeBuilder loadConstant(Opcode opcode, ConstantDesc value) { - BytecodeHelpers.validateValue(opcode, value); - // avoid non-local enum switch for bootstrap - if (opcode == BIPUSH || opcode == SIPUSH) { - writeArgumentConstant(opcode, ((Number) value).intValue()); - } else if (opcode == LDC || opcode == LDC_W || opcode == LDC2_W) { - writeLoadConstant(opcode, BytecodeHelpers.constantEntry(constantPool(), value)); - } else { - // intrinsics - writeBytecode(opcode); - } - return this; - } - @Override public CodeBuilder nop() { writeBytecode(NOP); diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java index 5521336f93859..71dc97d114e2c 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java @@ -562,7 +562,7 @@ void updateStaticCommit(BlockCodeBuilder blockCodeBuilder, Label excluded) { // write begin event getEventConfiguration(blockCodeBuilder); // stack: [EW], [EW], [EventConfiguration] - blockCodeBuilder.loadConstant(Opcode.LDC2_W, eventTypeId); + blockCodeBuilder.loadConstant(eventTypeId); // stack: [EW], [EW], [EventConfiguration] [long] invokevirtual(blockCodeBuilder, TYPE_EVENT_WRITER, EventWriterMethod.BEGIN_EVENT.method()); // stack: [EW], [integer] @@ -676,7 +676,7 @@ void updateInstanceCommit(BlockCodeBuilder blockCodeBuilder, Label end, Label ex // stack: [EW] [EW] getEventConfiguration(blockCodeBuilder); // stack: [EW] [EW] [EC] - blockCodeBuilder.loadConstant(Opcode.LDC2_W, eventTypeId); + blockCodeBuilder.loadConstant(eventTypeId); invokevirtual(blockCodeBuilder, TYPE_EVENT_WRITER, EventWriterMethod.BEGIN_EVENT.method()); // stack: [EW] [int] blockCodeBuilder.ifeq(excluded); diff --git a/test/jdk/jdk/classfile/AdaptCodeTest.java b/test/jdk/jdk/classfile/AdaptCodeTest.java index ca9145d688521..80c526c63c736 100644 --- a/test/jdk/jdk/classfile/AdaptCodeTest.java +++ b/test/jdk/jdk/classfile/AdaptCodeTest.java @@ -95,7 +95,7 @@ void testSevenOfThirteenIterator() throws Exception { if ((val instanceof Integer) && ((Integer) val) == 13) { val = 7; } - codeB.loadConstant(i.opcode(), val); + codeB.loadConstant(val); } default -> codeB.with(codeE); } diff --git a/test/jdk/jdk/classfile/LDCTest.java b/test/jdk/jdk/classfile/LDCTest.java index 7379218840e30..207d53e88204c 100644 --- a/test/jdk/jdk/classfile/LDCTest.java +++ b/test/jdk/jdk/classfile/LDCTest.java @@ -64,9 +64,9 @@ void testLDCisConvertedToLDCW() throws Exception { for (int i = 0; i <= 256/2 + 2; i++) { // two entries per String StringEntry s = cpb.stringEntry("string" + i); } - c0.loadConstant(LDC, "string0") - .loadConstant(LDC, "string131") - .loadConstant(LDC, "string50") + c0.ldc("string0") + .ldc("string131") + .ldc("string50") .loadConstant(-0.0f) .loadConstant(-0.0d) //non-LDC test cases diff --git a/test/jdk/jdk/classfile/OpcodesValidationTest.java b/test/jdk/jdk/classfile/OpcodesValidationTest.java index f44bdfd272559..2470fcf132c54 100644 --- a/test/jdk/jdk/classfile/OpcodesValidationTest.java +++ b/test/jdk/jdk/classfile/OpcodesValidationTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2024, 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,107 +23,29 @@ /* * @test - * @summary Testing ClassFile constant instruction opcodes. + * @summary Testing ClassFile constant instruction argument validation. * @run junit OpcodesValidationTest */ -import java.lang.constant.ClassDesc; -import java.lang.constant.ConstantDesc; -import static java.lang.constant.ConstantDescs.CD_void; -import java.lang.constant.MethodTypeDesc; +import java.lang.classfile.instruction.ConstantInstruction; +import org.junit.jupiter.api.Test; -import java.lang.reflect.AccessFlag; -import java.lang.classfile.ClassFile; -import java.lang.classfile.Opcode; -import org.junit.jupiter.api.*; -import static org.junit.jupiter.api.DynamicTest.dynamicTest; import static org.junit.jupiter.api.Assertions.*; import static java.lang.classfile.Opcode.*; -import java.util.stream.Stream; -public class OpcodesValidationTest { - - record Case(Opcode opcode, Object constant) {} - - static Stream positiveCases() { - return Stream.of( - new Case(ACONST_NULL, null), - new Case(SIPUSH, (int)Short.MIN_VALUE), - new Case(SIPUSH, (int)Short.MAX_VALUE), - new Case(BIPUSH, (int)Byte.MIN_VALUE), - new Case(BIPUSH, (int)Byte.MAX_VALUE), - new Case(ICONST_M1, -1), - new Case(ICONST_0, 0), - new Case(ICONST_1, 1), - new Case(ICONST_2, 2), - new Case(ICONST_3, 3), - new Case(ICONST_4, 4), - new Case(ICONST_5, 5), - new Case(LCONST_0, 0l), - new Case(LCONST_0, 0), - new Case(LCONST_1, 1l), - new Case(LCONST_1, 1), - new Case(FCONST_0, 0.0f), - new Case(FCONST_1, 1.0f), - new Case(FCONST_2, 2.0f), - new Case(DCONST_0, 0.0d), - new Case(DCONST_1, 1.0d) - ); - } - - static Stream negativeCases() { - return Stream.of( - new Case(ACONST_NULL, 0), - new Case(SIPUSH, (int)Short.MIN_VALUE - 1), - new Case(SIPUSH, (int)Short.MAX_VALUE + 1), - new Case(BIPUSH, (int)Byte.MIN_VALUE - 1), - new Case(BIPUSH, (int)Byte.MAX_VALUE + 1), - new Case(ICONST_M1, -1l), - new Case(ICONST_0, 0l), - new Case(ICONST_1, 1l), - new Case(ICONST_2, 2l), - new Case(ICONST_3, 3l), - new Case(ICONST_4, 4l), - new Case(ICONST_5, 5l), - new Case(LCONST_0, null), - new Case(LCONST_0, 1l), - new Case(LCONST_1, 1.0d), - new Case(LCONST_1, 0), - new Case(FCONST_0, 0.0d), - new Case(FCONST_1, 1.01f), - new Case(FCONST_2, 2), - new Case(DCONST_0, 0.0f), - new Case(DCONST_1, 1.0f), - new Case(DCONST_1, 1) - ); - } - - @TestFactory - Stream testPositiveCases() { - return positiveCases().map(c -> dynamicTest(c.toString(), () -> testPositiveCase(c.opcode, c.constant))); - } - - private void testPositiveCase(Opcode opcode, Object constant) { - ClassFile.of().build(ClassDesc.of("MyClass"), - cb -> cb.withFlags(AccessFlag.PUBLIC) - .withMethod("", MethodTypeDesc.of(CD_void), 0, - mb -> mb.withCode( - codeb -> codeb.loadConstant(opcode, (ConstantDesc) constant)))); - } - - - @TestFactory - Stream testNegativeCases() { - return negativeCases().map(c -> dynamicTest( - c.toString(), - () -> assertThrows(IllegalArgumentException.class, () -> testNegativeCase(c.opcode, c.constant)) - )); - } - - private void testNegativeCase(Opcode opcode, Object constant) { - ClassFile.of().build(ClassDesc.of("MyClass"), - cb -> cb.withFlags(AccessFlag.PUBLIC) - .withMethod("", MethodTypeDesc.of(CD_void), 0, - mb -> mb .withCode( - codeb -> codeb.loadConstant(opcode, (ConstantDesc)constant)))); +class OpcodesValidationTest { + + @Test + void testArgumentConstant() { + assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, 0)); + assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, Short.MIN_VALUE)); + assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, Short.MAX_VALUE)); + assertDoesNotThrow(() -> ConstantInstruction.ofArgument(BIPUSH, 0)); + assertDoesNotThrow(() -> ConstantInstruction.ofArgument(BIPUSH, Byte.MIN_VALUE)); + assertDoesNotThrow(() -> ConstantInstruction.ofArgument(BIPUSH, Byte.MAX_VALUE)); + + assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(SIPUSH, (int)Short.MIN_VALUE - 1)); + 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)); } } diff --git a/test/jdk/jdk/classfile/helpers/InstructionModelToCodeBuilder.java b/test/jdk/jdk/classfile/helpers/InstructionModelToCodeBuilder.java index d7d9e9c267f51..d865ddad5edb2 100644 --- a/test/jdk/jdk/classfile/helpers/InstructionModelToCodeBuilder.java +++ b/test/jdk/jdk/classfile/helpers/InstructionModelToCodeBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2024, 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 @@ -77,7 +77,7 @@ public static void toBuilder(CodeElement model, CodeBuilder cb) { case OperatorInstruction im -> cb.with(OperatorInstruction.of(im.opcode())); case ConstantInstruction im -> - cb.loadConstant(im.opcode(), im.constantValue()); + cb.loadConstant(im.constantValue()); case MonitorInstruction im -> cb.with(MonitorInstruction.of(im.opcode())); case NopInstruction im ->