From bb1fd0dcc02cf2b58a0bffdda969e088e7308456 Mon Sep 17 00:00:00 2001 From: liach Date: Wed, 12 Mar 2025 09:09:46 -0500 Subject: [PATCH 1/5] 8351362: Post-process @Strict annotation for testing --- .../StrictFinalInstanceFieldsTest.java | 5 +- .../jdk/test/lib/StrictCompilerTest.java | 60 +++++++++ .../lib/compiler/InMemoryJavaCompiler.java | 8 +- test/lib/jdk/test/lib/value/Strict.java | 38 ++++++ .../jdk/test/lib/value/StrictCompiler.java | 121 ++++++++++++++++++ 5 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 test/lib-test/jdk/test/lib/StrictCompilerTest.java create mode 100644 test/lib/jdk/test/lib/value/Strict.java create mode 100644 test/lib/jdk/test/lib/value/StrictCompiler.java diff --git a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java index 2da9a413105..27c8b3af69f 100644 --- a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java +++ b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java @@ -24,11 +24,12 @@ /* * @test * @enablePreview - * @compile --add-exports=java.base/jdk.internal.vm.annotation=ALL-UNNAMED -XDgenerateAssertUnsetFieldsFrame StrictFinalInstanceFieldsTest.java + * @library /test/lib + * @run main/othervm jdk.test.lib.value.StrictCompiler StrictFinalInstanceFieldsTest.java * @run main/othervm -Xlog:verification StrictFinalInstanceFieldsTest */ -import jdk.internal.vm.annotation.Strict; +import jdk.test.lib.value.Strict; public class StrictFinalInstanceFieldsTest { public static void main(String[] args) { diff --git a/test/lib-test/jdk/test/lib/StrictCompilerTest.java b/test/lib-test/jdk/test/lib/StrictCompilerTest.java new file mode 100644 index 00000000000..3caa45079f7 --- /dev/null +++ b/test/lib-test/jdk/test/lib/StrictCompilerTest.java @@ -0,0 +1,60 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* @test + * @bug 8351362 + * @summary Unit Test for StrictCompiler + * @enablePreview + * @library /test/lib + * @run main/othervm jdk.test.lib.value.StrictCompiler StrictCompilerTest.java + * @run junit StrictCompilerTest + */ + +import jdk.test.lib.value.Strict; +import org.junit.jupiter.api.Test; + +import static java.lang.classfile.ClassFile.ACC_FINAL; +import static java.lang.classfile.ClassFile.ACC_STRICT; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class StrictCompilerTest { + @Test + void testReflectMyself() throws Throwable { + for (var field : One.class.getDeclaredFields()) { + assertEquals(ACC_STRICT | ACC_FINAL, field.getModifiers(), () -> field.getName()); + } + } +} + +class One { + @Strict + final int a; + @Strict + final Object b; + + One() { + this.a = 1; + this.b = 2392352234L; + super(); + } +} diff --git a/test/lib/jdk/test/lib/compiler/InMemoryJavaCompiler.java b/test/lib/jdk/test/lib/compiler/InMemoryJavaCompiler.java index 4722ef3b67a..befc235d529 100644 --- a/test/lib/jdk/test/lib/compiler/InMemoryJavaCompiler.java +++ b/test/lib/jdk/test/lib/compiler/InMemoryJavaCompiler.java @@ -216,7 +216,11 @@ public String getClassName() { * @return The resulting byte code from the compilation */ public static Map compile(Map inputMap) { - Collection sourceFiles = new LinkedList(); + return compile(inputMap, new String[0]); + } + + public static Map compile(Map inputMap, String... options) { + Collection sourceFiles = new ArrayList<>(); for (Entry entry : inputMap.entrySet()) { sourceFiles.add(new SourceFile(entry.getKey(), entry.getValue())); } @@ -225,7 +229,7 @@ public static Map compile(Map in FileManager fileManager = new FileManager(compiler.getStandardFileManager(null, null, null)); Writer writer = new StringWriter(); - Boolean exitCode = compiler.getTask(writer, fileManager, null, null, null, sourceFiles).call(); + Boolean exitCode = compiler.getTask(writer, fileManager, null, Arrays.asList(options), null, sourceFiles).call(); if (!exitCode) { System.out.println("*********** javac output begin ***********"); System.out.println(writer.toString()); diff --git a/test/lib/jdk/test/lib/value/Strict.java b/test/lib/jdk/test/lib/value/Strict.java new file mode 100644 index 00000000000..5977f6eaae3 --- /dev/null +++ b/test/lib/jdk/test/lib/value/Strict.java @@ -0,0 +1,38 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.test.lib.value; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation to indicate the compiler that the ACC_STRICT flag should be set to + * the annotated field. Used by StrictTransformer. + */ +@Target(ElementType.FIELD) +@Retention(RetentionPolicy.RUNTIME) +public @interface Strict { +} diff --git a/test/lib/jdk/test/lib/value/StrictCompiler.java b/test/lib/jdk/test/lib/value/StrictCompiler.java new file mode 100644 index 00000000000..5785f8e6969 --- /dev/null +++ b/test/lib/jdk/test/lib/value/StrictCompiler.java @@ -0,0 +1,121 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.test.lib.value; + +import java.io.IOException; +import java.lang.classfile.*; +import java.lang.classfile.attribute.RuntimeVisibleAnnotationsAttribute; +import java.lang.constant.ClassDesc; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import jdk.test.lib.compiler.InMemoryJavaCompiler; + +import static java.lang.classfile.ClassFile.ACC_STRICT; + +/** + * Compile a java file with InMemoryJavaCompiler, and then modify the resulting + * class file to include strict modifier and null restriction attributes. + */ +public final class StrictCompiler { + public static final String TEST_SRC = System.getProperty("test.src", "").trim(); + public static final String TEST_CLASSES = System.getProperty("test.classes", "").trim(); + private static final ClassDesc CD_Strict = ClassDesc.of("jdk.test.lib.value.Strict"); + // NR will stay in jdk.internal for now until we expose as a more formal feature + private static final ClassDesc CD_NullRestricted = ClassDesc.of("jdk.internal.vm.annotation.NullRestricted"); + + /** + * @param args source and destination + * @throws IOException if an I/O error occurs + */ + public static void main(String[] args) throws IOException { + Map ins = new HashMap<>(); + List opts = new ArrayList<>(); + for (var a : args) { + if (a.endsWith(".java")) { + String className = a.substring(0, a.length() - 5); + Path src = Path.of(TEST_SRC, a); + ins.put(className, Files.readString(src)); + } else { + opts.add(a); + } + } + if (!opts.contains("--source")) { + opts.add("--source"); + opts.add(String.valueOf(Runtime.version().feature())); + } + if (!opts.contains("--enable-preview")) { + opts.add("--enable-preview"); + } + var classes = InMemoryJavaCompiler.compile(ins, opts.toArray(String[]::new)); + Files.createDirectories(Path.of(TEST_CLASSES)); + for (var entry : classes.entrySet()) { + dumpClass(entry.getKey(), entry.getValue()); + } + } + + private static void dumpClass(String name, byte[] rawBytes) throws IOException { + var cm = ClassFile.of().parse(rawBytes); + var transformed = ClassFile.of().transformClass(cm, ClassTransform.transformingFields(new FieldTransform() { + int oldAccessFlags; + boolean nullRestricted; + boolean strict; + + @Override + public void accept(FieldBuilder builder, FieldElement element) { + if (element instanceof AccessFlags af) { + oldAccessFlags = af.flagsMask(); + return; + } + builder.with(element); + if (element instanceof RuntimeVisibleAnnotationsAttribute rvaa) { + for (var anno : rvaa.annotations()) { + var descString = anno.className(); + if (descString.equalsString(CD_Strict.descriptorString())) { + strict = true; + } else if (descString.equalsString(CD_NullRestricted.descriptorString())) { + nullRestricted = true; + } + } + } + } + + @Override + public void atEnd(FieldBuilder builder) { + if (strict) { + oldAccessFlags |= ACC_STRICT; + } + builder.withFlags(oldAccessFlags); + assert !nullRestricted || strict : name; + } + })); + + Path dst = Path.of(TEST_CLASSES, name + ".class"); + Files.write(dst, transformed); + } +} From 30d78fb9224f8074803914451476bd6eea8daa70 Mon Sep 17 00:00:00 2001 From: liach Date: Mon, 17 Mar 2025 15:55:39 -0500 Subject: [PATCH 2/5] Add very primitive super rewrite functionality --- .../jdk/test/lib/StrictCompilerSuperTest.java | 85 +++++++++++ .../jdk/test/lib/StrictCompilerTest.java | 22 +-- .../jdk/test/lib/value/StrictCompiler.java | 135 ++++++++++++++++-- 3 files changed, 220 insertions(+), 22 deletions(-) create mode 100644 test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java diff --git a/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java b/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java new file mode 100644 index 00000000000..0270317ff65 --- /dev/null +++ b/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java @@ -0,0 +1,85 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* @test + * @bug 8351362 + * @summary Unit Test for StrictCompiler super rewrite + * @enablePreview + * @library /test/lib + * @run main/othervm jdk.test.lib.value.StrictCompiler --deferSuperCall StrictCompilerSuperTest.java + * @run junit StrictCompilerSuperTest + */ + +import java.lang.classfile.Attributes; +import java.lang.classfile.ClassFile; +import java.lang.classfile.ClassModel; +import java.lang.classfile.Instruction; +import java.lang.classfile.Opcode; +import java.util.ArrayList; + +import jdk.test.lib.value.Strict; +import org.junit.jupiter.api.Test; + +import static java.lang.classfile.ClassFile.*; +import static java.lang.constant.ConstantDescs.INIT_NAME; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +class StrictCompilerSuperTest { + @Test + void testReflectRewrittenRecord() throws Throwable { + for (var field : Rec.class.getDeclaredFields()) { + assertEquals(ACC_PRIVATE | ACC_STRICT | ACC_FINAL, field.getModifiers(), () -> "For field: " + field.getName()); + } + } + + @Test + void testRewrittenStrictAccessInClassFile() throws Throwable { + ClassModel cm; + try (var in = StrictCompilerSuperTest.class.getResourceAsStream("/StrictCompilerSuperTest$Rec.class")) { + cm = ClassFile.of().parse(in.readAllBytes()); + } + for (var f : cm.fields()) { + assertEquals(ACC_PRIVATE | ACC_STRICT | ACC_FINAL, f.flags().flagsMask(), () -> "Field " + f); + } + } + + @Test + void testRewrittenCtorBytecode() throws Throwable { + ClassModel cm; + try (var in = StrictCompilerSuperTest.class.getResourceAsStream("/StrictCompilerSuperTest$Rec.class")) { + cm = ClassFile.of().parse(in.readAllBytes()); + } + var ctor = cm.methods().stream().filter(m -> m.methodName().equalsString(INIT_NAME)).findFirst().orElseThrow(); + var insts = new ArrayList(); + ctor.findAttribute(Attributes.code()).orElseThrow().forEach(ce -> { + if (ce instanceof Instruction inst) { + insts.add(inst); + } + }); + assertSame(Opcode.RETURN, insts.getLast().opcode()); + assertSame(Opcode.INVOKESPECIAL, insts.get(insts.size() - 2).opcode()); + } + + record Rec(@Strict int a, @Strict long b) {} +} diff --git a/test/lib-test/jdk/test/lib/StrictCompilerTest.java b/test/lib-test/jdk/test/lib/StrictCompilerTest.java index 3caa45079f7..34c2ba2fc17 100644 --- a/test/lib-test/jdk/test/lib/StrictCompilerTest.java +++ b/test/lib-test/jdk/test/lib/StrictCompilerTest.java @@ -40,21 +40,21 @@ class StrictCompilerTest { @Test void testReflectMyself() throws Throwable { - for (var field : One.class.getDeclaredFields()) { + for (var field : StrictTarget.class.getDeclaredFields()) { assertEquals(ACC_STRICT | ACC_FINAL, field.getModifiers(), () -> field.getName()); } } -} -class One { - @Strict - final int a; - @Strict - final Object b; + static final class StrictTarget { + @Strict + final int a; + @Strict + final Object b; - One() { - this.a = 1; - this.b = 2392352234L; - super(); + StrictTarget() { + this.a = 1; + this.b = 2392352234L; + super(); + } } } diff --git a/test/lib/jdk/test/lib/value/StrictCompiler.java b/test/lib/jdk/test/lib/value/StrictCompiler.java index 5785f8e6969..a9f8403ebf2 100644 --- a/test/lib/jdk/test/lib/value/StrictCompiler.java +++ b/test/lib/jdk/test/lib/value/StrictCompiler.java @@ -26,17 +26,24 @@ import java.io.IOException; import java.lang.classfile.*; import java.lang.classfile.attribute.RuntimeVisibleAnnotationsAttribute; +import java.lang.classfile.constantpool.Utf8Entry; +import java.lang.classfile.instruction.FieldInstruction; +import java.lang.classfile.instruction.InvokeInstruction; +import java.lang.classfile.instruction.ReturnInstruction; import java.lang.constant.ClassDesc; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import jdk.test.lib.compiler.InMemoryJavaCompiler; -import static java.lang.classfile.ClassFile.ACC_STRICT; +import static java.lang.classfile.ClassFile.*; +import static java.lang.constant.ConstantDescs.INIT_NAME; /** * Compile a java file with InMemoryJavaCompiler, and then modify the resulting @@ -55,28 +62,131 @@ public final class StrictCompiler { */ public static void main(String[] args) throws IOException { Map ins = new HashMap<>(); - List opts = new ArrayList<>(); + List javacOpts = new ArrayList<>(); + boolean encounteredSeparator = false; + boolean deferSuperCall = false; for (var a : args) { + if (encounteredSeparator) { + javacOpts.add(a); + continue; + } if (a.endsWith(".java")) { String className = a.substring(0, a.length() - 5); Path src = Path.of(TEST_SRC, a); ins.put(className, Files.readString(src)); - } else { - opts.add(a); + continue; + } + switch (a) { + case "--" -> encounteredSeparator = true; + case "--deferSuperCall" -> deferSuperCall = true; + default -> throw new IllegalArgumentException("Unknown option " + a); } } - if (!opts.contains("--source")) { - opts.add("--source"); - opts.add(String.valueOf(Runtime.version().feature())); + if (!javacOpts.contains("--source")) { + javacOpts.add("--source"); + javacOpts.add(String.valueOf(Runtime.version().feature())); } - if (!opts.contains("--enable-preview")) { - opts.add("--enable-preview"); + if (!javacOpts.contains("--enable-preview")) { + javacOpts.add("--enable-preview"); } - var classes = InMemoryJavaCompiler.compile(ins, opts.toArray(String[]::new)); + System.out.println(javacOpts); + var classes = InMemoryJavaCompiler.compile(ins, javacOpts.toArray(String[]::new)); Files.createDirectories(Path.of(TEST_CLASSES)); for (var entry : classes.entrySet()) { - dumpClass(entry.getKey(), entry.getValue()); + if (deferSuperCall) { + fixSuperAndDumpClass(entry.getKey(), entry.getValue()); + } else { + dumpClass(entry.getKey(), entry.getValue()); + } + } + } + + private static void fixSuperAndDumpClass(String name, byte[] rawBytes) throws IOException { + var cm = ClassFile.of().parse(rawBytes); + record FieldKey(Utf8Entry name, Utf8Entry type) {} + Set strictFinals = new HashSet<>(); + for (var f : cm.fields()) { + var rvaa = f.findAttribute(Attributes.runtimeVisibleAnnotations()); + if (rvaa.isPresent()) { + for (var anno : rvaa.get().annotations()) { + var descString = anno.className(); + if (descString.equalsString(CD_Strict.descriptorString())) { + strictFinals.add(new FieldKey(f.fieldName(), f.fieldType())); + } + } + } } + + var thisClass = cm.thisClass(); + var superName = cm.superclass().orElseThrow().name(); + + var rewritten = ClassFile.of().transformClass(cm, (clb, cle) -> { + cond: + if (cle instanceof MethodModel mm + && mm.methodName().equalsString(INIT_NAME)) { + var code = mm.findAttribute(Attributes.code()).orElseThrow(); + var elements = code.elementList(); + int len = elements.size(); + int superCallPos = -1; + int returnPos = -1; + boolean deferSuperCall = false; + for (int i = 0; i < len; i++) { + var e = elements.get(i); + if (superCallPos == -1) { + if (e instanceof InvokeInstruction inv && + inv.opcode() == Opcode.INVOKESPECIAL && + inv.method().name().equalsString(INIT_NAME) && + inv.method().type().equalsString("()V") && + inv.owner().name().equals(superName)) { + // Assume we are calling on uninitializedThis... + superCallPos = i; + } + } else if (!deferSuperCall) { + if (e instanceof FieldInstruction ins && + ins.opcode() == Opcode.PUTFIELD && + ins.owner().equals(thisClass) && + strictFinals.contains(new FieldKey(ins.name(), ins.type()))) { + deferSuperCall = true; + } + } + if (e instanceof ReturnInstruction inst && inst.opcode() == Opcode.RETURN) { + if (returnPos != -1) { + throw new IllegalArgumentException("Control flow too complex"); + } else { + returnPos = i; + } + } + } + if (elements.reversed().stream() + .mapMulti((e, sink) -> { + if (e instanceof Instruction i) { + sink.accept(i); + } + }) + .findFirst() + .orElseThrow() + .opcode() != Opcode.RETURN) { + throw new IllegalArgumentException("Control flow too complex"); + } + if (!deferSuperCall) { + break cond; + } + var suppliedElements = new ArrayList<>(elements); + var foundLoad = suppliedElements.remove(superCallPos - 1); + var foundSuperCall = suppliedElements.remove(superCallPos - 1); + var foundReturnInst = suppliedElements.remove(returnPos - 2); + suppliedElements.add(foundLoad); + suppliedElements.add(foundSuperCall); + suppliedElements.add(foundReturnInst); + clb.withMethod(INIT_NAME, mm.methodTypeSymbol(), mm.flags().flagsMask(), mb -> mb + .transform(mm, MethodTransform.dropping(ce -> ce instanceof CodeModel)) + .withCode(suppliedElements::forEach)); + return; + } + clb.with(cle); + }); + + dumpClass(name, rewritten); } private static void dumpClass(String name, byte[] rawBytes) throws IOException { @@ -115,6 +225,9 @@ public void atEnd(FieldBuilder builder) { } })); + // Force preview + transformed[4] = (byte) 0xFF; + transformed[5] = (byte) 0xFF; Path dst = Path.of(TEST_CLASSES, name + ".class"); Files.write(dst, transformed); } From a44c269c196dc334f76c12faa0bb101aa52365d7 Mon Sep 17 00:00:00 2001 From: liach Date: Mon, 17 Mar 2025 16:19:19 -0500 Subject: [PATCH 3/5] Fix a bug and enhance tests --- .../jdk/test/lib/StrictCompilerSuperTest.java | 80 +++++++++++++++---- .../jdk/test/lib/value/StrictCompiler.java | 13 +-- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java b/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java index 0270317ff65..2f689cc8cdd 100644 --- a/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java +++ b/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java @@ -30,15 +30,21 @@ * @run junit StrictCompilerSuperTest */ +import java.io.IOException; +import java.io.UncheckedIOException; import java.lang.classfile.Attributes; import java.lang.classfile.ClassFile; import java.lang.classfile.ClassModel; import java.lang.classfile.Instruction; import java.lang.classfile.Opcode; +import java.lang.reflect.AccessFlag; +import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.stream.Stream; import jdk.test.lib.value.Strict; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import static java.lang.classfile.ClassFile.*; import static java.lang.constant.ConstantDescs.INIT_NAME; @@ -46,30 +52,43 @@ import static org.junit.jupiter.api.Assertions.assertSame; class StrictCompilerSuperTest { - @Test - void testReflectRewrittenRecord() throws Throwable { - for (var field : Rec.class.getDeclaredFields()) { + static Stream> testClasses() { + return Stream.of(Rec.class, Exp.class, Inner.class); + } + + static Stream testClassModels() { + return testClasses().map(cls -> { + try (var in = StrictCompilerSuperTest.class.getResourceAsStream("/" + cls.getName() + ".class")) { + return ClassFile.of().parse(in.readAllBytes()); + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + }); + } + + @MethodSource("testClasses") + @ParameterizedTest + void testReflectRewrittenRecord(Class cls) throws Throwable { + for (var field : cls.getDeclaredFields()) { + if (Modifier.isStatic(field.getModifiers()) || field.isSynthetic()) + continue; assertEquals(ACC_PRIVATE | ACC_STRICT | ACC_FINAL, field.getModifiers(), () -> "For field: " + field.getName()); } } - @Test - void testRewrittenStrictAccessInClassFile() throws Throwable { - ClassModel cm; - try (var in = StrictCompilerSuperTest.class.getResourceAsStream("/StrictCompilerSuperTest$Rec.class")) { - cm = ClassFile.of().parse(in.readAllBytes()); - } + @MethodSource("testClassModels") + @ParameterizedTest + void testRewrittenStrictAccessInClassFile(ClassModel cm) throws Throwable { for (var f : cm.fields()) { + if (f.flags().has(AccessFlag.STATIC) || f.flags().has(AccessFlag.SYNTHETIC)) + continue; assertEquals(ACC_PRIVATE | ACC_STRICT | ACC_FINAL, f.flags().flagsMask(), () -> "Field " + f); } } - @Test - void testRewrittenCtorBytecode() throws Throwable { - ClassModel cm; - try (var in = StrictCompilerSuperTest.class.getResourceAsStream("/StrictCompilerSuperTest$Rec.class")) { - cm = ClassFile.of().parse(in.readAllBytes()); - } + @MethodSource("testClassModels") + @ParameterizedTest + void testRewrittenCtorBytecode(ClassModel cm) throws Throwable { var ctor = cm.methods().stream().filter(m -> m.methodName().equalsString(INIT_NAME)).findFirst().orElseThrow(); var insts = new ArrayList(); ctor.findAttribute(Attributes.code()).orElseThrow().forEach(ce -> { @@ -81,5 +100,32 @@ void testRewrittenCtorBytecode() throws Throwable { assertSame(Opcode.INVOKESPECIAL, insts.get(insts.size() - 2).opcode()); } - record Rec(@Strict int a, @Strict long b) {} + record Rec(@Strict int a, @Strict long b) { + static final String NOISE = "noise"; + } + + static class Exp { + private @Strict final int a; + private @Strict final long b; + + Exp(int a, long b) { + this.a = a; + this.b = b; + } + } + + class Inner { + private @Strict final int a; + private @Strict final long b; + + Inner(int a, long b) { + this.a = a; + this.b = b; + } + + @Override + public String toString() { + return a + " " + StrictCompilerSuperTest.this + " " + b; + } + } } diff --git a/test/lib/jdk/test/lib/value/StrictCompiler.java b/test/lib/jdk/test/lib/value/StrictCompiler.java index a9f8403ebf2..d4e05390599 100644 --- a/test/lib/jdk/test/lib/value/StrictCompiler.java +++ b/test/lib/jdk/test/lib/value/StrictCompiler.java @@ -31,6 +31,7 @@ import java.lang.classfile.instruction.InvokeInstruction; import java.lang.classfile.instruction.ReturnInstruction; import java.lang.constant.ClassDesc; +import java.lang.reflect.AccessFlag; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -104,14 +105,16 @@ public static void main(String[] args) throws IOException { private static void fixSuperAndDumpClass(String name, byte[] rawBytes) throws IOException { var cm = ClassFile.of().parse(rawBytes); record FieldKey(Utf8Entry name, Utf8Entry type) {} - Set strictFinals = new HashSet<>(); + Set strictInstances = new HashSet<>(); for (var f : cm.fields()) { + if (f.flags().has(AccessFlag.STATIC)) + continue; var rvaa = f.findAttribute(Attributes.runtimeVisibleAnnotations()); if (rvaa.isPresent()) { for (var anno : rvaa.get().annotations()) { var descString = anno.className(); if (descString.equalsString(CD_Strict.descriptorString())) { - strictFinals.add(new FieldKey(f.fieldName(), f.fieldType())); + strictInstances.add(new FieldKey(f.fieldName(), f.fieldType())); } } } @@ -145,7 +148,7 @@ record FieldKey(Utf8Entry name, Utf8Entry type) {} if (e instanceof FieldInstruction ins && ins.opcode() == Opcode.PUTFIELD && ins.owner().equals(thisClass) && - strictFinals.contains(new FieldKey(ins.name(), ins.type()))) { + strictInstances.contains(new FieldKey(ins.name(), ins.type()))) { deferSuperCall = true; } } @@ -191,7 +194,7 @@ record FieldKey(Utf8Entry name, Utf8Entry type) {} private static void dumpClass(String name, byte[] rawBytes) throws IOException { var cm = ClassFile.of().parse(rawBytes); - var transformed = ClassFile.of().transformClass(cm, ClassTransform.transformingFields(new FieldTransform() { + var transformed = ClassFile.of().transformClass(cm, ClassTransform.transformingFields(FieldTransform.ofStateful(() -> new FieldTransform() { int oldAccessFlags; boolean nullRestricted; boolean strict; @@ -223,7 +226,7 @@ public void atEnd(FieldBuilder builder) { builder.withFlags(oldAccessFlags); assert !nullRestricted || strict : name; } - })); + }))); // Force preview transformed[4] = (byte) 0xFF; From 06894d39cf253ca1c745ee6b74661d7455614ab1 Mon Sep 17 00:00:00 2001 From: liach Date: Wed, 9 Apr 2025 04:13:35 -0500 Subject: [PATCH 4/5] Revert back to the jdk internal strict --- .../jdk/internal/vm/annotation/Strict.java | 2 +- .../StrictFinalInstanceFieldsTest.java | 2 +- test/lib/jdk/test/lib/value/Strict.java | 38 ------------------- .../jdk/test/lib/value/StrictCompiler.java | 6 ++- 4 files changed, 7 insertions(+), 41 deletions(-) delete mode 100644 test/lib/jdk/test/lib/value/Strict.java diff --git a/src/java.base/share/classes/jdk/internal/vm/annotation/Strict.java b/src/java.base/share/classes/jdk/internal/vm/annotation/Strict.java index e5f29d85bbc..ca22ba8adab 100644 --- a/src/java.base/share/classes/jdk/internal/vm/annotation/Strict.java +++ b/src/java.base/share/classes/jdk/internal/vm/annotation/Strict.java @@ -32,6 +32,6 @@ * Internal and experimental use only */ @Target(ElementType.FIELD) -@Retention(RetentionPolicy.SOURCE) +@Retention(RetentionPolicy.RUNTIME) public @interface Strict { } diff --git a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java index 68321a4fb17..fe0c042b059 100644 --- a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java +++ b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java @@ -29,7 +29,7 @@ * @run main/othervm -Xlog:verification StrictFinalInstanceFieldsTest */ -import jdk.test.lib.value.Strict; +import jdk.internal.vm.annotation.Strict; public class StrictFinalInstanceFieldsTest { public static void main(String[] args) { diff --git a/test/lib/jdk/test/lib/value/Strict.java b/test/lib/jdk/test/lib/value/Strict.java deleted file mode 100644 index 5977f6eaae3..00000000000 --- a/test/lib/jdk/test/lib/value/Strict.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright (c) 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 - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package jdk.test.lib.value; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Annotation to indicate the compiler that the ACC_STRICT flag should be set to - * the annotated field. Used by StrictTransformer. - */ -@Target(ElementType.FIELD) -@Retention(RetentionPolicy.RUNTIME) -public @interface Strict { -} diff --git a/test/lib/jdk/test/lib/value/StrictCompiler.java b/test/lib/jdk/test/lib/value/StrictCompiler.java index d4e05390599..0c9ceb01fec 100644 --- a/test/lib/jdk/test/lib/value/StrictCompiler.java +++ b/test/lib/jdk/test/lib/value/StrictCompiler.java @@ -53,7 +53,7 @@ public final class StrictCompiler { public static final String TEST_SRC = System.getProperty("test.src", "").trim(); public static final String TEST_CLASSES = System.getProperty("test.classes", "").trim(); - private static final ClassDesc CD_Strict = ClassDesc.of("jdk.test.lib.value.Strict"); + private static final ClassDesc CD_Strict = ClassDesc.of("jdk.internal.vm.annotation.Strict"); // NR will stay in jdk.internal for now until we expose as a more formal feature private static final ClassDesc CD_NullRestricted = ClassDesc.of("jdk.internal.vm.annotation.NullRestricted"); @@ -90,6 +90,10 @@ public static void main(String[] args) throws IOException { if (!javacOpts.contains("--enable-preview")) { javacOpts.add("--enable-preview"); } + if (!javacOpts.contains("java.base/jdk.internal.vm.annotation=ALL-UNNAMED")) { + javacOpts.add("--add-exports"); + javacOpts.add("java.base/jdk.internal.vm.annotation=ALL-UNNAMED"); + } System.out.println(javacOpts); var classes = InMemoryJavaCompiler.compile(ins, javacOpts.toArray(String[]::new)); Files.createDirectories(Path.of(TEST_CLASSES)); From 84bdb7a66e0f1f19fa1dc0d733d72ff5182b4c48 Mon Sep 17 00:00:00 2001 From: liach Date: Wed, 9 Apr 2025 04:42:33 -0500 Subject: [PATCH 5/5] Rollback to the jdk internal strict annotation --- .../inlinetypes/verifier/StrictFinalInstanceFieldsTest.java | 1 + test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java | 3 ++- test/lib-test/jdk/test/lib/StrictCompilerTest.java | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java index fe0c042b059..914c3d31dd1 100644 --- a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java +++ b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/verifier/StrictFinalInstanceFieldsTest.java @@ -25,6 +25,7 @@ * @test * @enablePreview * @library /test/lib + * @modules java.base/jdk.internal.vm.annotation * @run main/othervm jdk.test.lib.value.StrictCompiler StrictFinalInstanceFieldsTest.java -- -XDnoLocalProxyVars * @run main/othervm -Xlog:verification StrictFinalInstanceFieldsTest */ diff --git a/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java b/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java index 2f689cc8cdd..1e3ad861789 100644 --- a/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java +++ b/test/lib-test/jdk/test/lib/StrictCompilerSuperTest.java @@ -26,6 +26,7 @@ * @summary Unit Test for StrictCompiler super rewrite * @enablePreview * @library /test/lib + * @modules java.base/jdk.internal.vm.annotation * @run main/othervm jdk.test.lib.value.StrictCompiler --deferSuperCall StrictCompilerSuperTest.java * @run junit StrictCompilerSuperTest */ @@ -42,7 +43,7 @@ import java.util.ArrayList; import java.util.stream.Stream; -import jdk.test.lib.value.Strict; +import jdk.internal.vm.annotation.Strict; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; diff --git a/test/lib-test/jdk/test/lib/StrictCompilerTest.java b/test/lib-test/jdk/test/lib/StrictCompilerTest.java index 34c2ba2fc17..2233360169a 100644 --- a/test/lib-test/jdk/test/lib/StrictCompilerTest.java +++ b/test/lib-test/jdk/test/lib/StrictCompilerTest.java @@ -26,11 +26,12 @@ * @summary Unit Test for StrictCompiler * @enablePreview * @library /test/lib + * @modules java.base/jdk.internal.vm.annotation * @run main/othervm jdk.test.lib.value.StrictCompiler StrictCompilerTest.java * @run junit StrictCompilerTest */ -import jdk.test.lib.value.Strict; +import jdk.internal.vm.annotation.Strict; import org.junit.jupiter.api.Test; import static java.lang.classfile.ClassFile.ACC_FINAL;