From 08816b79814a07a97ba368c2038649b431eed9d0 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 21:48:12 -0700 Subject: [PATCH 01/18] GROOVY-12568: SC: call a protected or package-private constructor with new directly only within the compilation unit Under static compilation, new A(...) of a protected or package-private constructor of a class with the same package name was compiled to NEW and INVOKESPECIAL. The JVM allows that call only within the same run-time package, which is the package name and the class loader, so it failed with IllegalAccessError when A came from another class loader: a class of an earlier parse of the same GroovyShell, which GroovyClassLoader defines with another InnerLoader, or a precompiled class used by a script. Dynamic and type-checked code create such an object through the meta-class and work. Such a constructor is now called directly only when its class is compiled in the same compilation unit, which defines all its classes with one class loader, as for a constructor reference. Otherwise the object is created through the meta-class, which selects the constructor at run time. This also applies to the no-argument constructor that a map-style construction such as new A(name: 'x') calls before it sets the properties. Public constructors, and constructors of classes of the same compilation unit, are still called directly. The constructor reference writer uses the same check. --- .../asm/sc/StaticInvocationWriter.java | 20 +++ .../ConstructorCallTransformer.java | 31 +++- .../sc/StaticCompileRuntimePackageTest.groovy | 169 ++++++++++++++++++ 3 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index 1211ab7c56d..080b84081e9 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -57,6 +57,7 @@ import org.codehaus.groovy.classgen.asm.TypeChooser; import org.codehaus.groovy.classgen.asm.VariableSlotLoader; import org.codehaus.groovy.classgen.asm.WriterController; +import org.codehaus.groovy.runtime.ScriptBytecodeAdapter; import org.codehaus.groovy.syntax.SyntaxException; import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys; import org.codehaus.groovy.transform.sc.StaticCompilationVisitor; @@ -81,6 +82,7 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.formatTypeName; import static org.apache.groovy.ast.tools.ClassNodeUtils.isSubtype; +import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName; import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant; import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression; import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression; @@ -115,6 +117,8 @@ */ public class StaticInvocationWriter extends InvocationWriter { + private static final MethodCallerMultiAdapter invokeNew = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "invokeNew", true, true); + private final AtomicInteger labelCounter = new AtomicInteger(); /** @@ -149,6 +153,10 @@ public void writeInvokeConstructor(final ConstructorCallExpression call) { } ClassNode declaringClass = cn.getDeclaringClass(); ClassNode enclosingClass = controller.getClassNode(); + if (!cn.isPrivate() && !isDirectlyAccessible(cn, enclosingClass) && samePackageName(declaringClass, enclosingClass)) { + writeDynamicConstructorCall(call, declaringClass); + return; + } List argList = call.getArguments() instanceof TupleExpression ? ((TupleExpression) call.getArguments()).getExpressions() : List.of(call.getArguments()); if (!AsmClassGenerator.isMemberDirectlyAccessible(cn.getModifiers(), declaringClass, enclosingClass)) { @@ -198,6 +206,18 @@ static boolean isDirectlyAccessible(final ConstructorNode ctor, final ClassNode return AsmClassGenerator.isInSameRuntimePackage(declaringClass, accessingClass); } + /** + * Creates the object through the meta-class, which selects the constructor + * at run time, for a constructor that the current class cannot call directly. + */ + private void writeDynamicConstructorCall(final ConstructorCallExpression call, final ClassNode type) { + Expression arguments = call.getArguments(); + makeUncachedCall(call, classX(controller.getClassNode()), classX(type), null, arguments, + invokeNew, false, false, false, AsmClassGenerator.containsSpreadExpression(arguments)); + BytecodeHelper.doCast(controller.getMethodVisitor(), type); + controller.getOperandStack().replace(type); + } + /** {@inheritDoc} */ @Override public void writeSpecialConstructorCall(final ConstructorCallExpression call) { diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java index 71a9c71268c..76578813ba5 100644 --- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java +++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java @@ -42,8 +42,10 @@ import java.util.List; +import static org.codehaus.groovy.ast.tools.GeneralUtils.args; import static org.codehaus.groovy.ast.tools.GeneralUtils.assignX; import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX; import static org.codehaus.groovy.ast.tools.GeneralUtils.propX; import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DIRECT_METHOD_CALL_TARGET; @@ -181,16 +183,27 @@ public void visit(final MethodVisitor mv) { } // create temporary variable to store new object instance int tmpObj = compileStack.defineTemporaryVariable("tmpObj", ctorType, false); - String ctorTypeName = BytecodeHelper.getClassInternalName(ctorType); - mv.visitTypeInsn(NEW, ctorTypeName); - mv.visitInsn(DUP); - String signature = "()V"; - if (innerClassCall && ctorType.getOuterClass() != null) { - acg.visitVariableExpression(varX("this")); // GROOVY-11122 - Parameter[] params = {new Parameter(ctorType.getOuterClass(),"$p$")}; - signature = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, params); + boolean outer = innerClassCall && ctorType.getOuterClass() != null; + Parameter[] params = outer ? new Parameter[]{new Parameter(ctorType.getOuterClass(), "$p$")} : Parameter.EMPTY_ARRAY; + ConstructorNode ctor = ctorType.isPrimaryClassNode() ? null : ctorType.getDeclaredConstructor(params); + if (ctor != null && !ctor.isPublic()) { + // the invocation writer checks if the constructor of a class of another compilation unit can be called directly + ConstructorCallExpression newObject = outer ? ctorX(ctorType, args(varX("this"))) : ctorX(ctorType); + newObject.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, ctor); + newObject.setSourcePosition(originalCall); + newObject.visit(acg); + acg.getController().getOperandStack().remove(1); + } else { + String ctorTypeName = BytecodeHelper.getClassInternalName(ctorType); + mv.visitTypeInsn(NEW, ctorTypeName); + mv.visitInsn(DUP); + String signature = "()V"; + if (outer) { + acg.visitVariableExpression(varX("this")); // GROOVY-11122 + signature = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, params); + } + mv.visitMethodInsn(INVOKESPECIAL, ctorTypeName, "", signature, false); } - mv.visitMethodInsn(INVOKESPECIAL, ctorTypeName, "", signature, false); mv.visitVarInsn(ASTORE, tmpObj); if (map instanceof MapExpression mapExpression) { diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy new file mode 100644 index 00000000000..ecd181fd39e --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -0,0 +1,169 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.classgen.asm.sc + +import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase +import org.codehaus.groovy.classgen.asm.InstructionSequence +import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.Phases +import org.junit.jupiter.api.Test + +/** + * Unit tests for static compilation: protected and package-private members of + * a class with the same package name, compiled by another compilation unit. + *

+ * The JVM allows such access only within the same run-time package, which is + * the package name and the class loader. GroovyClassLoader defines the classes + * of each compilation unit with its own class loader, so a class of an earlier + * parse of the same GroovyShell, or a precompiled class used by a script, is + * in another run-time package. + */ +final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { + + private final GroovyShell shell = new GroovyShell() + + private static final String LIBRARY = '''package p + class A { + final String value + protected A(String value) { this.value = 'prot:' + value } + @groovy.transform.PackageScope A(Integer i) { this.value = 'pkg:' + i } + @groovy.transform.PackageScope A() { this.value = 'none' } + String name + } + true + ''' + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateConstructorOfClassOfAnotherClassLoader() { + shell.evaluate LIBRARY + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + class C { // same package name as A, but another class loader + static A prot() { new A('x') } + static A pkg() { new A(1) } + static A none() { new A() } + static List closure() { [1, 2].collect { Integer i -> new A(i) } } + static A lambda() { java.util.function.Supplier s = () -> new A('y'); s.get() } + } + [C.prot(), C.pkg(), C.none(), *C.closure(), C.lambda()]*.value + ''') == ['prot:x', 'pkg:1', 'none', 'pkg:1', 'pkg:2', 'prot:y'] + } + + // GROOVY-12568 + @Test + void testPackagePrivateConstructorOfPrecompiledClassUsedByScript() { + def unit = new CompilationUnit() + unit.addSource('Library.groovy', LIBRARY) + unit.compile(Phases.CLASS_GENERATION) + def libraryLoader = new GroovyClassLoader() + unit.classes.each { libraryLoader.defineClass(it.name, it.bytes) } + + assert new GroovyShell(libraryLoader).evaluate('''package p + @groovy.transform.CompileStatic + A make() { new A(1) } + make().value + ''') == 'pkg:1' + } + + // GROOVY-12568 + @Test + void testMapStyleConstructionWithPackagePrivateConstructorOfClassOfAnotherClassLoader() { + shell.evaluate LIBRARY + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + A make() { new A(name: 'n') } + def a = make() + a.value + ':' + a.name + ''') == 'none:n' + } + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateConstructorsOfSameCompilationUnitAreCalledDirectly() { + def bytecode = compile(method: 'make', classNamePattern: 'p.C', '''package p + class A { + protected A(String s) { } + @groovy.transform.PackageScope A(Integer i) { } + } + @groovy.transform.CompileStatic + class C { + static List make() { + [new A('x'), new A(1)] + } + } + ''') + assert bytecode.hasSequence(['NEW p/A', 'DUP', 'LDC "x"', 'INVOKESPECIAL p/A. (Ljava/lang/String;)V']) + assert bytecode.hasSequence(['NEW p/A', 'DUP', 'ICONST_1', 'INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;', 'INVOKESPECIAL p/A. (Ljava/lang/Integer;)V']) + assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invokeNew']) + } + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateConstructorsOfClassOfAnotherCompilationUnitAreSelectedAtRuntime() { + def bytecode = compileAgainstLibrary(LIBRARY, 'p.C', 'make', '''package p + @groovy.transform.CompileStatic + class C { + static List make() { + [new A('x'), new A(1)] + } + } + ''') + assert bytecode.hasSequence(['INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeNewN (Ljava/lang/Class;Ljava/lang/Class;Ljava/lang/Object;)Ljava/lang/Object;', 'CHECKCAST p/A']) + assert !bytecode.hasSequence(['INVOKESPECIAL p/A.']) + } + + // GROOVY-12568 + @Test + void testPublicConstructorOfClassOfAnotherCompilationUnitIsCalledDirectly() { + def bytecode = compileAgainstLibrary('''package p + class B { + B(String s) { } + } + ''', 'p.C', 'make', '''package p + @groovy.transform.CompileStatic + class C { + static B make() { + new B('x') + } + } + ''') + assert bytecode.hasSequence(['NEW p/B', 'DUP', 'LDC "x"', 'INVOKESPECIAL p/B. (Ljava/lang/String;)V']) + assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invokeNew']) + } + + /** + * Compiles {@code script} against the classes of {@code library}, which are + * compiled before, by another compilation unit and class loader. + */ + private InstructionSequence compileAgainstLibrary(final String library, final String className, final String method, final String script) { + def loader = new GroovyClassLoader() + loader.parseClass(library) + def unit = new CompilationUnit(null, null, loader) + unit.addSource('script', script) + unit.compile(Phases.CLASS_GENERATION) + def generated = unit.classes.find { it.name == className } + assert generated != null : "class $className not found" + def sequence = extractSequence(generated.bytes, [method: method]) + List instructions = sequence.instructions + int begin = instructions.indexOf('--BEGIN--'), end = instructions.indexOf('--END--') + assert begin >= 0 && end > begin : "method $method not found" + new InstructionSequence(instructions: instructions[(begin + 1).. Date: Sat, 24 Oct 2026 21:55:31 -0700 Subject: [PATCH 02/18] GROOVY-12568: SC: call a protected or package-private method directly only within the compilation unit Under static compilation, a call to a protected or package-private method of a class with the same package name was compiled to a direct INVOKEVIRTUAL, INVOKESTATIC or INVOKESPECIAL. As for a constructor, the JVM allows that call only within the same run-time package, so it failed with IllegalAccessError when the class came from another class loader, for example a class of an earlier parse of the same GroovyShell. This also hit a protected or package-private getter or setter used as a property, such as a.name or a.name = value. Such a method is now called directly only when its class is compiled in the same compilation unit, or when the JVM allows the call from any package: a protected method called from a subclass on this, super or a receiver of the subclass type, or a protected static method called from a subclass. Otherwise the method is selected at run time through the meta-class, as dynamic code does. Public methods, and methods of classes of the same compilation unit, are still called directly. --- .../asm/sc/StaticInvocationWriter.java | 9 ++ .../sc/StaticCompileRuntimePackageTest.groovy | 109 ++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index 080b84081e9..dc0a062234d 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -375,6 +375,15 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i receiverType = receiverType.getGenericsTypes()[0].getType(); // GROOVY-11694 } + if (!target.isPublic() && !target.isPrivate() && !declaringClass.isPrimaryClassNode() + && samePackageName(declaringClass, enclosingClass) + && !(target.isProtected() && enclosingClass.isDerivedFrom(declaringClass) + && (target.isStatic() || isSuperExpression(receiver) || isSubtype(enclosingClass, receiverType)))) { + // a protected or package-private method of a class of another compilation unit may be in another run-time + // package, which is the package name and the class loader: select the method at run time + return false; + } + if (AsmClassGenerator.isMemberDirectlyAccessible(target.getModifiers(), declaringClass, enclosingClass) && !(target.isProtected() && !inSamePackage(declaringClass, enclosingClass) && !isSubtype(receiverType, enclosingClass))) { // GROOVY-7325 boolean isThisImplicit = implicitThis; diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index ecd181fd39e..02f571c8d64 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -49,6 +49,22 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { true ''' + private static final String MEMBERS = '''package p + class M { + @groovy.transform.PackageScope String pm() { 'pm' } + protected String qm() { 'qm' } + @groovy.transform.PackageScope static String spm() { 'spm' } + protected static String sqm() { 'sqm' } + @groovy.transform.PackageScope int count(int i) { i + 1 } + protected String getQp() { 'qp' } + @groovy.transform.PackageScope String getPp() { 'pp' } + private String hidden = 'h' + protected void setQs(String s) { hidden = 'qs:' + s } + String getHidden() { hidden } + } + true + ''' + // GROOVY-12568 @Test void testProtectedAndPackagePrivateConstructorOfClassOfAnotherClassLoader() { @@ -148,6 +164,99 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invokeNew']) } + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateMethodsOfClassOfAnotherClassLoader() { + shell.evaluate MEMBERS + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + class C { // same package name as M, but another class loader + static List test(M m) { + int n = m.count(1) + [m.pm(), m.qm(), M.spm(), M.sqm(), n, m?.pm(), [m]*.qm(), m.qp, m.pp, { -> m.pm() }(), m.with { it.qm() }] + } + static String setter(M m) { + m.qs = 'v' + m.hidden + } + } + C.test(new M()) << C.setter(new M()) + ''') == ['pm', 'qm', 'spm', 'sqm', 2, 'pm', ['qm'], 'qp', 'pp', 'pm', 'qm', 'qs:v'] + } + + // GROOVY-12568 + @Test + void testInheritedProtectedAndPackagePrivateMethodsOfClassOfAnotherClassLoader() { + shell.evaluate MEMBERS + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + class B extends M { + List test() { + [pm(), qm(), spm(), sqm(), this.pm(), super.qm(), qp, pp, { -> pm() + qm() }()] + } + static List statics() { + [spm(), sqm()] + } + } + new B().test() + B.statics() + ''') == ['pm', 'qm', 'spm', 'sqm', 'pm', 'qm', 'qp', 'pp', 'pmqm', 'spm', 'sqm'] + } + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateMethodsOfSameCompilationUnitAreCalledDirectly() { + def bytecode = compile(method: 'test', classNamePattern: 'p.C', MEMBERS + ''' + @groovy.transform.CompileStatic + class C { + static List test(M m) { + [m.pm(), m.qm(), M.spm()] + } + } + ''') + assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.pm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.qm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKESTATIC p/M.spm ()Ljava/lang/String;']) + assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invokeMethod']) + } + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateMethodsOfClassOfAnotherCompilationUnitAreSelectedAtRuntime() { + def bytecode = compileAgainstLibrary(MEMBERS, 'p.C', 'test', '''package p + @groovy.transform.CompileStatic + class C { + static List test(M m) { + [m.pm(), m.qm(), M.spm(), m.hidden] + } + } + ''') + assert bytecode.hasSequence(['LDC "pm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethod0']) + assert bytecode.hasSequence(['LDC "qm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethod0']) + assert bytecode.hasSequence(['LDC "spm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethod0']) + assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.getHidden ()Ljava/lang/String;']) // public + assert !bytecode.hasSequence(['INVOKEVIRTUAL p/M.pm']) + assert !bytecode.hasSequence(['INVOKEVIRTUAL p/M.qm']) + assert !bytecode.hasSequence(['INVOKESTATIC p/M.spm']) + } + + // GROOVY-12568 + @Test + void testProtectedMethodsOfSuperclassOfAnotherCompilationUnitAreCalledDirectly() { + def bytecode = compileAgainstLibrary(MEMBERS, 'p.B', 'test', '''package p + @groovy.transform.CompileStatic + class B extends M { + List test(B b) { + [qm(), this.qm(), super.qm(), b.qm(), sqm(), pm()] + } + } + ''') + assert bytecode.hasSequence(['ALOAD 0', 'INVOKEVIRTUAL p/B.qm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['ALOAD 0', 'INVOKESPECIAL p/M.qm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['ALOAD 1', 'INVOKEVIRTUAL p/B.qm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKESTATIC p/M.sqm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['LDC "pm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethodOnCurrent0']) + } + /** * Compiles {@code script} against the classes of {@code library}, which are * compiled before, by another compilation unit and class loader. From 611b517dec31019ec0b2a416a89bb0e5c1ba8d20 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 22:01:13 -0700 Subject: [PATCH 03/18] GROOVY-12568: SC: refer to a protected or package-private method of another compilation unit through a synthetic method A statically compiled method reference such as m::pm, M::pm or M::spm gives LambdaMetafactory a method handle for the method. For a protected or package-private method of a class with the same package name, the JVM resolves that handle only within the same run-time package, so the reference failed with IllegalAccessError when the class came from another class loader, for example a class of an earlier parse of the same GroovyShell. For such a method of a class of another compilation unit, the reference now targets a synthetic static method of the current class that calls the method. The invocation writer compiles that call directly when the JVM allows it from any package (a protected method called through the subclass), or else selects the method at run time. Method references to methods of classes of the same compilation unit, and to public methods, still use a method handle for the method itself. --- ...cTypesMethodReferenceExpressionWriter.java | 58 +++++++++++++++++ .../sc/StaticCompileRuntimePackageTest.groovy | 64 +++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java index 32f90166669..feb9f99a6c0 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java @@ -55,6 +55,7 @@ import static java.util.Comparator.comparingInt; import static java.util.stream.Collectors.joining; +import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName; import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; import static org.codehaus.groovy.ast.tools.GeneralUtils.classX; import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX; @@ -273,11 +274,68 @@ private ResolvedMethodReference adaptMethodReference(final FunctionalInterfaceCo if (methodRefMethod.isStatic() && !referenceTarget.targetIsArgument()) { referenceTarget = referenceTarget.asClassTarget(methodRefMethod.getDeclaringClass()); } + } else if (mayBeInAnotherRuntimePackage(methodRefMethod)) { + if (!referenceTarget.classExpression() && !methodRefMethod.isStatic()) { + referenceTarget = referenceTarget.markTargetAsArgument(); + } + methodRefMethod = addSyntheticMethodForRuntimeSelection(methodRefMethod, referenceTarget.type()); + if (!referenceTarget.targetIsArgument()) { + referenceTarget = referenceTarget.asClassTarget(methodRefMethod.getDeclaringClass()); + } } return resolvedMethodReference.with(referenceTarget, methodRefMethod); } + /** + * Determines if the method is a protected or package-private method of a + * class with the same package name but of another compilation unit. Such a + * class may come from another class loader, so it may be in another + * run-time package, and the JVM would then refuse a method handle for the + * method in the current class. + */ + private boolean mayBeInAnotherRuntimePackage(final MethodNode mn) { + ClassNode declaringClass = mn.getDeclaringClass(); + return !mn.isPublic() && !mn.isPrivate() && !declaringClass.isPrimaryClassNode() + && samePackageName(declaringClass, controller.getClassNode()); + } + + /** + * Creates a method of the current class that calls the method of the + * reference. The invocation writer calls it directly if the current class + * can, or else selects it at run time. + */ + private MethodNode addSyntheticMethodForRuntimeSelection(final MethodNode mn, final ClassNode targetType) { + List parameters = new ArrayList<>(); + Expression receiver; + if (mn.isStatic()) { + receiver = classX(mn.getDeclaringClass()); + } else { + Parameter target = new Parameter(targetType.isDerivedFrom(mn.getDeclaringClass()) ? targetType : mn.getDeclaringClass(), "target"); + parameters.add(target); + receiver = varX(target); + } + List arguments = new ArrayList<>(); + Parameter[] mnParameters = mn.getParameters(); + for (int i = 0; i < mnParameters.length; i += 1) { + Parameter parameter = new Parameter(mnParameters[i].getType(), "p" + i); + parameters.add(parameter); + arguments.add(varX(parameter)); + } + + MethodCallExpression methodCall = callX(receiver, mn.getName(), new ArgumentListExpression(arguments)); + methodCall.setImplicitThis(false); + methodCall.setMethodTarget(mn); + methodCall.putNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET, mn); + + String methodName = createSyntheticMethodName("access", mn.getDeclaringClass(), mn.getName()); + + MethodNode delegateMethod = addSyntheticMethod(methodName, mn.getReturnType(), methodCall, parameters.toArray(Parameter[]::new), mn.getExceptions()); + delegateMethod.putNodeMetaData(StaticCompilationMetadataKeys.STATIC_COMPILE_NODE, Boolean.TRUE); + delegateMethod.setGenericsTypes(mn.getGenericsTypes()); + return delegateMethod; + } + private boolean needsVariadicAdapter(final FunctionalInterfaceContext functionalInterface, final MethodReferenceTarget referenceTarget, final MethodNode methodRefMethod) { diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index 02f571c8d64..b1878ed4b23 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -257,6 +257,70 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { assert bytecode.hasSequence(['LDC "pm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethodOnCurrent0']) } + // GROOVY-12568 + @Test + void testMethodReferencesToProtectedAndPackagePrivateMethodsOfClassOfAnotherClassLoader() { + shell.evaluate MEMBERS + assert shell.evaluate('''package p + import java.util.function.* + + @groovy.transform.CompileStatic + class C { // same package name as M, but another class loader + static List test(M m) { + Supplier bound = m::pm + Function unbound = M::qm + Supplier stat = M::spm + IntUnaryOperator prim = m::count + BiFunction unboundPrim = M::count + [bound.get(), unbound.apply(m), stat.get(), prim.applyAsInt(1), unboundPrim.apply(m, 2)] + } + } + @groovy.transform.CompileStatic + class B extends M { + List test() { + Supplier pkg = this::pm, prot = this::qm, stat = M::sqm + [pkg.get(), prot.get(), stat.get()] + } + } + C.test(new M()) + new B().test() + ''') == ['pm', 'qm', 'spm', 2, 3, 'pm', 'qm', 'sqm'] + } + + // GROOVY-12568 + @Test + void testMethodReferenceToPackagePrivateMethodOfSameCompilationUnitIsDirect() { + def bytecode = compile(method: 'test', classNamePattern: 'p.C', MEMBERS + ''' + @groovy.transform.CompileStatic + class C { + static java.util.function.Supplier test(M m) { + m::pm + } + } + ''') + assert bytecode.hasSequence(['// handle kind 0x5 : INVOKEVIRTUAL', 'p/M.pm()Ljava/lang/String;']) + assert !bytecode.hasSequence(['p/C.access$M$pm$']) + } + + // GROOVY-12568 + @Test + void testMethodReferenceToPackagePrivateMethodOfClassOfAnotherCompilationUnitIsSelectedAtRuntime() { + def script = '''package p + @groovy.transform.CompileStatic + class C { + static java.util.function.Supplier test(M m) { + m::pm + } + } + ''' + def bytecode = compileAgainstLibrary(MEMBERS, 'p.C', 'test', script) + assert bytecode.hasSequence(['// handle kind 0x6 : INVOKESTATIC', 'p/C.access$M$pm$']) + assert !bytecode.hasSequence(['p/M.pm()Ljava/lang/String;']) + + String method = bytecode.instructions.find { it.startsWith('p/C.access$M$pm$') }.with { it.substring(4, it.indexOf('(')) } + def access = compileAgainstLibrary(MEMBERS, 'p.C', method, script) + assert access.hasSequence(['LDC "pm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethod0']) + } + /** * Compiles {@code script} against the classes of {@code library}, which are * compiled before, by another compilation unit and class loader. From 0e88869c51a5c3f81cb672648541326bc28bb023 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 22:09:20 -0700 Subject: [PATCH 04/18] GROOVY-12568: access a protected or package-private field directly only within the compilation unit AsmClassGenerator.isFieldDirectlyAccessible decided by the package name if a protected or package-private field can be read or written with GETFIELD, PUTFIELD, GETSTATIC or PUTSTATIC. As for methods and constructors, the JVM allows that only within the same run-time package, so the access failed with IllegalAccessError when the class came from another class loader, for example a class of an earlier parse of the same GroovyShell. Under static compilation this hit a.field, a.@field, Type.field, an implicit field of a superclass, compound assignments and increments. Dynamic code hit it for super.field, super.@field and this.@field of a superclass. A protected field of a superclass with the same package name but of another compilation unit was also read through a receiver of the superclass type, which fails verification ("Bad access to protected data in getfield") when the classes are in different run-time packages. Such a field is now accessed directly only when its class is compiled in the same compilation unit, or through the subclass rule for a protected field (with a receiver of the current class). Otherwise it is read and written through the meta-class, as dynamic code does. Public fields, and fields of classes of the same compilation unit, are still accessed directly. --- .../groovy/classgen/AsmClassGenerator.java | 16 +- .../asm/sc/StaticTypesCallSiteWriter.java | 30 +++- .../sc/StaticCompileRuntimePackageTest.groovy | 153 ++++++++++++++++++ 3 files changed, 195 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index 51595a53815..cd57d86925f 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -159,7 +159,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; import static org.codehaus.groovy.ast.tools.GeneralUtils.classX; import static org.codehaus.groovy.ast.tools.GeneralUtils.fieldX; -import static org.codehaus.groovy.ast.tools.GeneralUtils.inSamePackage; import static org.codehaus.groovy.ast.tools.GeneralUtils.maybeFallsThrough; import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX; import static org.codehaus.groovy.ast.tools.ParameterUtils.isVargs; @@ -1308,9 +1307,20 @@ private static String getStaticFieldName(final ClassNode type) { /** * Determines if the given field can be directly accessed by the given class * (via {@code GETFIELD}, {@code GETSTATIC}, etc. bytecode instructions). + *

+ * A protected or package-private field of a class with the same package + * name is accessible only within the same run-time package, which is the + * package name and the class loader. GroovyClassLoader defines all the + * classes of a compilation unit with one class loader, but a class of + * another compilation unit may come from another class loader, so its + * field is directly accessible only through the subclass rule for a + * protected field. */ public static boolean isFieldDirectlyAccessible(final FieldNode field, final ClassNode accessingClass) { - return field != null && isMemberDirectlyAccessible(field.getModifiers(), field.getDeclaringClass(), accessingClass); + if (field == null || !isMemberDirectlyAccessible(field.getModifiers(), field.getDeclaringClass(), accessingClass)) return false; + ClassNode declaringClass = field.getDeclaringClass(); + return field.isPublic() || field.isPrivate() || declaringClass.isPrimaryClassNode() + || (field.isProtected() && accessingClass.isDerivedFrom(declaringClass)); } /** @@ -1519,7 +1529,7 @@ public void visitPropertyExpression(final PropertyExpression expression) { } else { fieldNode = classNode.getSuperClass().getDeclaredField(name); // GROOVY-4497, GROOVY-11764: do not visit super class field if it is private or package-private - if (fieldNode != null && (fieldNode.isPrivate() || !(fieldNode.isPublic() || fieldNode.isProtected() || inSamePackage(classNode, classNode.getSuperClass())))) { + if (fieldNode != null && (fieldNode.isPrivate() || !(fieldNode.isPublic() || fieldNode.isProtected() || isFieldDirectlyAccessible(fieldNode, classNode)))) { fieldNode = null; } } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java index 724b29d2d88..27043977f6f 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java @@ -234,11 +234,29 @@ && getField(receiverType, propertyName, FieldNode::isPublic) == null) { } } + if (isFieldOfAnotherCompilationUnit(receiverType, propertyName)) { + makeDynamicGetProperty(receiver, propertyName, safe); + return; + } + addPropertyAccessError(receiver, propertyName, receiverType); controller.getMethodVisitor().visitInsn(ACONST_NULL); controller.getOperandStack().push(OBJECT_TYPE); } + /** + * Determines if the property is a protected or package-private field of a + * class with the same package name but of another compilation unit, which + * may be in another run-time package. Such a field is read through the + * meta-class when the current class cannot access it directly. + */ + private boolean isFieldOfAnotherCompilationUnit(final ClassNode receiverType, final String propertyName) { + FieldNode field = getField(receiverType, propertyName); + return field != null && !field.isPublic() && !field.isPrivate() + && !field.getDeclaringClass().isPrimaryClassNode() + && samePackageName(field.getDeclaringClass(), controller.getClassNode()); + } + private void makeDynamicGetProperty(final Expression receiver, final String propertyName, final boolean safe) { MethodNode target = safe ? INVOKERHELPER_GETPROPERTYSAFE_METHOD : INVOKERHELPER_GETPROPERTY_METHOD; MethodCallExpression call = callX( @@ -406,6 +424,11 @@ && getField(receiverType, propertyName, FieldNode::isPublic) == null return; } + if (isFieldOfAnotherCompilationUnit(receiverType, propertyName)) { + makeDynamicGetProperty(receiver, propertyName, safe); + return; + } + boolean isScriptVariable = (receiverType.isScript() && receiver instanceof VariableExpression && ((VariableExpression) receiver).getAccessedVariable() == null); // a packed closure's hoisted body keeps the closure-context contract: like the inner // closure class it replaces, it falls through to the dynamic property sequence (e.g. @@ -537,7 +560,8 @@ boolean makeGetField(final Expression receiver, final ClassNode receiverType, fi ClassNode currentClass = controller.getClassNode(); ClassNode castType = field.getOwner(); if (field.isProtected() && !currentClass.equals(field.getOwner()) - && !samePackageName(currentClass, field.getOwner())) { + // a class of another compilation unit may come from another class loader + && !(samePackageName(currentClass, field.getOwner()) && field.getOwner().isPrimaryClassNode())) { if (receiverType.isDerivedFrom(currentClass)) castType = currentClass; else return false; } @@ -862,6 +886,10 @@ private boolean setField(final PropertyExpression expression, final Expression o FieldNode fn = AsmClassGenerator.getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(controller.getClassNode(), receiverType, name, false); if (fn == null) return false; + if (!fn.isStatic() && fn.isProtected() && !fn.getOwner().isPrimaryClassNode() + && samePackageName(controller.getClassNode(), fn.getOwner()) && !receiverType.isDerivedFrom(controller.getClassNode())) { + return false; // JVMS 4.10.1.8: the class may be in another run-time package, so the receiver must be of the current class + } OperandStack os = controller.getOperandStack(); os.doGroovyCast(fn.getType()); diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index b1878ed4b23..b9a155bf6e0 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -21,6 +21,7 @@ package org.codehaus.groovy.classgen.asm.sc import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase import org.codehaus.groovy.classgen.asm.InstructionSequence import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration import org.codehaus.groovy.control.Phases import org.junit.jupiter.api.Test @@ -65,6 +66,17 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { true ''' + private static final String FIELDS = '''package p + class F { + @groovy.transform.PackageScope String pf = 'pf' + protected String qf = 'qf' + @groovy.transform.PackageScope static String spf = 'spf' + protected static String sqf = 'sqf' + @groovy.transform.PackageScope int n = 1 + } + true + ''' + // GROOVY-12568 @Test void testProtectedAndPackagePrivateConstructorOfClassOfAnotherClassLoader() { @@ -321,6 +333,147 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { assert access.hasSequence(['LDC "pm"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeMethod0']) } + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateFieldsOfClassOfAnotherClassLoader() { + shell.evaluate FIELDS + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + class C { // same package name as F, but another class loader + static List read(F f) { + [f.pf, f.qf, F.spf, F.sqf, f.@pf, f.@qf, f?.pf, [f]*.pf, { -> f.pf }(), f.with { it.qf }] + } + static List write(F f) { + f.pf = 'w' + f.qf = 'x' + F.spf = 'y' + f.@n = 5 + f.n += 2 + f.n++ + int m = ++f.n + [f.pf, f.qf, F.spf, f.n, m] + } + } + C.read(new F()) + C.write(new F()) + ''') == ['pf', 'qf', 'spf', 'sqf', 'pf', 'qf', 'pf', ['pf'], 'pf', 'qf', 'w', 'x', 'y', 9, 9] + } + + // GROOVY-12568 + @Test + void testInheritedProtectedAndPackagePrivateFieldsOfClassOfAnotherClassLoader() { + shell.evaluate FIELDS + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + class B extends F { + List read(F other) { + [pf, qf, this.pf, this.qf, super.pf, this.@pf, super.@pf, spf, sqf, { -> pf + qf }(), other.qf, other.pf] + } + List write(F other) { + pf = 'a' + this.@qf = 'b' + other.qf = 'c' + other.pf = 'd' + [pf, qf, other.qf, other.pf] + } + } + class D extends F { // dynamic + List read() { + [super.pf, super.@pf, this.@pf] + } + List write() { + super.pf = 'e' + def x = pf + this.@pf = 'f' + [x, pf] + } + } + new B().read(new F()) + new B().write(new F()) + new D().read() + new D().write() + ''') == ['pf', 'qf', 'pf', 'qf', 'pf', 'pf', 'pf', 'spf', 'sqf', 'pfqf', 'qf', 'pf', 'a', 'b', 'c', 'd', 'pf', 'pf', 'pf', 'e', 'f'] + } + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateFieldsOfSameCompilationUnitAreAccessedDirectly() { + def bytecode = compile(method: 'test', classNamePattern: 'p.C', FIELDS + ''' + @groovy.transform.CompileStatic + class C { + static List test(F f) { + f.@pf = 'w' + [f.pf, f.qf, F.spf] + } + } + ''') + assert bytecode.hasSequence(['PUTFIELD p/F.pf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETFIELD p/F.pf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETFIELD p/F.qf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETSTATIC p/F.spf : Ljava/lang/String;']) + assert !bytecode.hasSequence(['INVOKESTATIC org/codehaus/groovy/runtime/InvokerHelper.getProperty']) + } + + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateFieldsOfClassOfAnotherCompilationUnitAreAccessedAtRuntime() { + def bytecode = compileAgainstLibrary(FIELDS, 'p.C', 'test', '''package p + @groovy.transform.CompileStatic + class C { + static List test(F f) { + f.@pf = 'w' + [f.pf, f.qf, F.spf] + } + } + ''') + assert bytecode.hasSequence(['LDC "pf"', 'INVOKESTATIC org/codehaus/groovy/runtime/InvokerHelper.getProperty']) + assert bytecode.hasSequence(['LDC "qf"', 'INVOKESTATIC org/codehaus/groovy/runtime/InvokerHelper.getProperty']) + assert bytecode.hasSequence(['LDC "spf"', 'INVOKESTATIC org/codehaus/groovy/runtime/InvokerHelper.getProperty']) + assert bytecode.hasSequence(['LDC "pf"', 'CHECKCAST java/lang/String', 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.setGroovyObjectField']) + assert !bytecode.hasSequence(['GETFIELD p/F']) + assert !bytecode.hasSequence(['PUTFIELD p/F']) + assert !bytecode.hasSequence(['GETSTATIC p/F']) + } + + // GROOVY-12568 + @Test + void testProtectedFieldOfSuperclassOfAnotherCompilationUnitIsAccessedDirectly() { + def bytecode = compileAgainstLibrary(FIELDS, 'p.B', 'test', '''package p + @groovy.transform.CompileStatic + class B extends F { + List test() { + this.@qf = 'x' + [qf, sqf, pf] + } + } + ''') + assert bytecode.hasSequence(['PUTFIELD p/F.qf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETFIELD p/F.qf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETSTATIC p/B.sqf : Ljava/lang/String;']) + assert bytecode.hasSequence(['LDC "pf"', 'INVOKESTATIC org/codehaus/groovy/runtime/InvokerHelper.getProperty']) + } + + // GROOVY-12568 + @Test + void testMembersOfClassOfAnotherClassLoaderWithoutInvokeDynamic() { + def config = new CompilerConfiguration() + config.optimizationOptions[CompilerConfiguration.INVOKEDYNAMIC] = false + def shell = new GroovyShell(this.class.classLoader, new Binding(), config) + shell.evaluate LIBRARY + shell.evaluate MEMBERS + shell.evaluate FIELDS + assert shell.evaluate('''package p + @groovy.transform.CompileStatic + class C { + static List test(M m, F f) { + java.util.function.Supplier ref = m::pm + f.pf = 'w' + [new A(1).value, m.pm(), M.spm(), m.qp, ref.get(), f.pf, F.spf, dynamic()] + } + static Object dynamic() { + 'dynamic'.toUpperCase().with { it } + } + } + C.test(new M(), new F()) + ''') == ['pkg:1', 'pm', 'spm', 'qp', 'pm', 'w', 'spf', 'DYNAMIC'] + } + /** * Compiles {@code script} against the classes of {@code library}, which are * compiled before, by another compilation unit and class loader. From 690390c2d7acfe8141a733f8cbef793e455ad831 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 22:23:57 -0700 Subject: [PATCH 05/18] GROOVY-12568: SC: report new of a protected constructor of another package instead of failing verification Outside its package, a protected constructor can only be called by super(...) or by creating an anonymous inner class (JLS 6.6.2.2). The type checker accepts new A(...) of such a constructor in a subclass of A, as it accepts other protected members there, and type-checked code creates the object through the meta-class. Under static compilation, the call was compiled to NEW and INVOKESPECIAL, and the class failed verification when it was loaded: "VerifyError: Bad access to protected method". The same happened for a map-style construction such as new A([:]), which calls the protected no-argument constructor. The static compiler now reports "Cannot call protected constructor for A from class B", as it reports a protected method called on a receiver that is not the subclass (GROOVY-7325). super(...), this(...) and new A(...) {} still call the constructor directly. A constructor reference A::new to such a constructor still selects it at run time. --- .../asm/sc/StaticInvocationWriter.java | 14 ++++- .../ConstructorCallTransformer.java | 4 +- .../transform/stc/ConstructorsSTCTest.groovy | 38 ++++++++++++ .../sc/StaticCompileConstructorsTest.groovy | 58 +++++++++++++++++++ 4 files changed, 109 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index dc0a062234d..e3c3923aba2 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -153,9 +153,17 @@ public void writeInvokeConstructor(final ConstructorCallExpression call) { } ClassNode declaringClass = cn.getDeclaringClass(); ClassNode enclosingClass = controller.getClassNode(); - if (!cn.isPrivate() && !isDirectlyAccessible(cn, enclosingClass) && samePackageName(declaringClass, enclosingClass)) { - writeDynamicConstructorCall(call, declaringClass); - return; + if (!cn.isPrivate() && !isDirectlyAccessible(cn, enclosingClass)) { + if (samePackageName(declaringClass, enclosingClass)) { + writeDynamicConstructorCall(call, declaringClass); + return; + } + // JLS 6.6.2.2: outside its package, only super(...) or an anonymous inner class can call a protected constructor + controller.getSourceUnit().addError(new SyntaxException( + "Cannot call " + (cn.isProtected() ? "protected" : "package-private") + " constructor for " + + declaringClass.toString(false) + " from class " + enclosingClass.toString(false), + call + )); } List argList = call.getArguments() instanceof TupleExpression ? ((TupleExpression) call.getArguments()).getExpressions() : List.of(call.getArguments()); diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java index 76578813ba5..2aa5b8c6713 100644 --- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java +++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java @@ -185,9 +185,9 @@ public void visit(final MethodVisitor mv) { int tmpObj = compileStack.defineTemporaryVariable("tmpObj", ctorType, false); boolean outer = innerClassCall && ctorType.getOuterClass() != null; Parameter[] params = outer ? new Parameter[]{new Parameter(ctorType.getOuterClass(), "$p$")} : Parameter.EMPTY_ARRAY; - ConstructorNode ctor = ctorType.isPrimaryClassNode() ? null : ctorType.getDeclaredConstructor(params); + ConstructorNode ctor = ctorType.getDeclaredConstructor(params); if (ctor != null && !ctor.isPublic()) { - // the invocation writer checks if the constructor of a class of another compilation unit can be called directly + // the invocation writer checks if the current class can call the constructor directly ConstructorCallExpression newObject = outer ? ctorX(ctorType, args(varX("this"))) : ctorX(ctorType); newObject.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, ctor); newObject.setSourcePosition(originalCall); diff --git a/src/test/groovy/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/groovy/transform/stc/ConstructorsSTCTest.groovy index 5167c42a663..1db0d308bf6 100644 --- a/src/test/groovy/groovy/transform/stc/ConstructorsSTCTest.groovy +++ b/src/test/groovy/groovy/transform/stc/ConstructorsSTCTest.groovy @@ -305,6 +305,44 @@ import static groovy.test.GroovyAssert.shouldFail 'Dynamic keys in map-style constructors are unsupported' } + // GROOVY-12568 + @Test + void testProtectedConstructorOfAnotherPackage() { + assertScript '''package p + class A { + final String value + protected A(String value) { this.value = value } + } + true + ''' + assertScript '''package q + class C extends p.A { + C() { super('c') } + static p.A make() { + new p.A('x') // only super(...) or an anonymous inner class can call it in Java + } + } + assert C.make().value == 'x' + ''' + } + + // GROOVY-12568 + @Test + void testProtectedConstructorOfAnotherPackageThroughSuperOrAnonymousInnerClass() { + assertScript ''' + class C extends ObjectInputStream { + C() { super() } + C(int i) { this() } + static ObjectInputStream make() { + new ObjectInputStream() {} + } + } + assert new C() instanceof ObjectInputStream + assert new C(1) instanceof ObjectInputStream + assert C.make() instanceof ObjectInputStream + ''' + } + @Test void testConstructWithMapAndInheritance() { assertScript ''' diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy index 279ededbf3f..29a4d8a4ee9 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy @@ -61,6 +61,64 @@ final class StaticCompileConstructorsTest extends ConstructorsSTCTest implements assert method.count('ACONST_NULL') == 0 } + // GROOVY-12568 + @Override + @Test + void testProtectedConstructorOfAnotherPackage() { + assertScript '''package p + class A { + final String value + protected A(String value) { this.value = value } + } + true + ''' + shouldFailWithMessages '''package q + class C extends p.A { + C() { super('c') } + static p.A make() { + new p.A('x') // only super(...) or an anonymous inner class can call it + } + } + ''', + 'Cannot call protected constructor for p.A from class q.C' + + shouldFailWithMessages ''' + class C extends ObjectInputStream { + static ObjectInputStream make() { + new ObjectInputStream() // only super() or an anonymous inner class can call it + } + } + ''', + 'Cannot call protected constructor for java.io.ObjectInputStream from class C' + } + + // GROOVY-12568 + @Test + void testMapStyleConstructionWithProtectedConstructorOfAnotherPackage() { + shouldFailWithMessages ''' + class C extends ObjectOutputStream { + C() { super() } + static ObjectOutputStream make() { + new ObjectOutputStream([:]) // map-style construction calls the protected ObjectOutputStream() + } + } + ''', + 'Cannot call protected constructor for java.io.ObjectOutputStream from class C' + } + + // GROOVY-12568 + @Test + void testProtectedConstructorOfAnotherPackageIsCalledDirectlyThroughSuperOrAnonymousInnerClass() { + testProtectedConstructorOfAnotherPackageThroughSuperOrAnonymousInnerClass() + + String c = astTrees['C'][1] + assert c.contains('INVOKESPECIAL java/io/ObjectInputStream. ()V') + assert c.contains('NEW C$1') + String aic = astTrees['C$1'][1] + assert aic.contains('INVOKESPECIAL java/io/ObjectInputStream. ()V') + assert !(c + aic).contains('ScriptBytecodeAdapter.invokeNew') + } + // GROOVY-12453 @Test void testMapConstructorWithinPowerAssert() { From a91dab229bd00196961538de73305539017297ca Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 22:31:46 -0700 Subject: [PATCH 06/18] GROOVY-12568: SC: report a protected method of another package called through a superclass receiver Outside its package, a protected instance method or field can be used only through a receiver of the subclass (JLS 6.6.2.1, JVMS 4.10.1.8). For a method, the static compiler meant to report such a call (GROOVY-7325), but its check compared the types the wrong way round: it tested whether the current class is a subtype of the receiver type instead of the reverse. So a.m() with a receiver of the superclass type, in a subclass in another package, was compiled to a direct call and the class failed verification: "VerifyError: Bad access to protected data in invokevirtual". This also hit a protected getter or setter used as a property (a.name, a.name = value). A method reference such as a::m or A::m to such a method failed with BootstrapMethodError, and an attribute assignment a.@field = value failed verification in putfield. The check now requires the receiver to be of the current class (or this, super or a static method), so such a call or method reference is reported as "Cannot access method", as GROOVY-7325 intended and as javac does. An attribute assignment through such a receiver is now made through the meta-class, as the attribute read a.@field already was. Calls through this, super or a receiver of the subclass type are still direct. --- .../asm/sc/StaticInvocationWriter.java | 4 +- .../asm/sc/StaticTypesCallSiteWriter.java | 9 +- ...cTypesMethodReferenceExpressionWriter.java | 22 ++++- .../classgen/asm/sc/bugs/Groovy7325Bug.groovy | 84 +++++++++++++++++++ 4 files changed, 114 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index e3c3923aba2..69943a615ff 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -393,7 +393,9 @@ && samePackageName(declaringClass, enclosingClass) } if (AsmClassGenerator.isMemberDirectlyAccessible(target.getModifiers(), declaringClass, enclosingClass) - && !(target.isProtected() && !inSamePackage(declaringClass, enclosingClass) && !isSubtype(receiverType, enclosingClass))) { // GROOVY-7325 + // GROOVY-7325, JVMS 4.10.1.8: outside its package, a protected instance method needs a receiver of the current class + && !(target.isProtected() && !target.isStatic() && !inSamePackage(declaringClass, enclosingClass) && receiver != null + && !isSuperExpression(receiver) && !isClassWithSuper(receiver) && !isSubtype(enclosingClass, receiverType))) { boolean isThisImplicit = implicitThis; Expression theReceiver = receiver; if (implicitThis && isThisExpression(receiver) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java index 27043977f6f..2e8b36a04bc 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java @@ -886,9 +886,12 @@ private boolean setField(final PropertyExpression expression, final Expression o FieldNode fn = AsmClassGenerator.getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(controller.getClassNode(), receiverType, name, false); if (fn == null) return false; - if (!fn.isStatic() && fn.isProtected() && !fn.getOwner().isPrimaryClassNode() - && samePackageName(controller.getClassNode(), fn.getOwner()) && !receiverType.isDerivedFrom(controller.getClassNode())) { - return false; // JVMS 4.10.1.8: the class may be in another run-time package, so the receiver must be of the current class + ClassNode currentClass = controller.getClassNode(); + if (!fn.isStatic() && fn.isProtected() && !currentClass.equals(fn.getOwner()) + // a class of another compilation unit may come from another class loader + && !(samePackageName(currentClass, fn.getOwner()) && fn.getOwner().isPrimaryClassNode()) + && !receiverType.isDerivedFrom(currentClass)) { + return false; // JVMS 4.10.1.8: in another run-time package, the receiver must be of the current class } OperandStack os = controller.getOperandStack(); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java index feb9f99a6c0..b0da72cf173 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java @@ -55,7 +55,9 @@ import static java.util.Comparator.comparingInt; import static java.util.stream.Collectors.joining; +import static org.apache.groovy.ast.tools.ClassNodeUtils.isSubtype; import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName; +import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression; import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; import static org.codehaus.groovy.ast.tools.GeneralUtils.classX; import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX; @@ -435,13 +437,31 @@ private void validate(final MethodReferenceExpression methodReference, final Cla addFatalError(error, methodReference); } else if (methodNode.isVoidMethod() && !ClassHelper.isPrimitiveVoid(samReturnType)) { addFatalError("Invalid return type: void is not convertible to " + samReturnType.getText(), methodReference); - } else if (!AsmClassGenerator.isMemberDirectlyAccessible(methodNode.getModifiers(), methodNode.getDeclaringClass(), controller.getClassNode())) { + } else if (!AsmClassGenerator.isMemberDirectlyAccessible(methodNode.getModifiers(), methodNode.getDeclaringClass(), controller.getClassNode()) + || isProtectedMethodOfAnotherPackageWithReceiverOfAnotherClass(methodReference.getExpression(), targetType, methodNode, samParameters)) { addFatalError("Cannot access method: " + methodName + " of class: " + methodNode.getDeclaringClass().getText(), methodReference); // GROOVY-11365 } else if (samParameters.length > 0 && isTypeReferringInstanceMethod(methodReference.getExpression(), methodNode) && !isAssignableTo(samParameters[0].getType(), targetType)) { throw new RuntimeParserException("Invalid receiver type: " + samParameters[0].getType().getText() + " is not compatible with " + targetType.getText(), methodReference.getExpression()); } } + /** + * Determines if the method is a protected instance method of another package + * and the receiver of the reference is not of the current class. Outside its + * package, a protected instance method can be called only through a receiver + * of the subclass (JVMS 4.10.1.8), so the JVM rejects such a reference. + */ + private boolean isProtectedMethodOfAnotherPackageWithReceiverOfAnotherClass(final Expression target, final ClassNode targetType, final MethodNode mn, final Parameter[] samParameters) { + if (!mn.isProtected() || mn.isStatic() || samePackageName(mn.getDeclaringClass(), controller.getClassNode()) || isSuperExpression(target)) { + return false; + } + ClassNode receiverType = targetType; + if (target instanceof ClassExpression && samParameters.length > 0) { + receiverType = samParameters[0].getType(); // type::instanceMethod + } + return !isSubtype(controller.getClassNode(), receiverType); + } + private MethodNode addSyntheticMethodForDGSM(final MethodNode mn) { Parameter[] parameters = removeFirstParameter(mn.getParameters()); ArgumentListExpression args = new ArgumentListExpression(parameters); diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy index 29e01a137ae..4cd7c48b71c 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy @@ -55,4 +55,88 @@ final class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCo ''', 'Cannot access method: clone() of class: java.lang.Object' } + + // GROOVY-12568 + @Test + void testProtectedMethodOfAnotherPackageThroughReceiverOfSuperclass() { + assertScript '''package p + class A { + protected String im() { 'im' } + protected String getQp() { 'qp' } + protected void setQs(String s) { } + } + true + ''' + shouldFailWithMessages '''package q + class B extends p.A { + static String test(p.A a) { + a.im() + } + } + ''', + 'Cannot access method: im() of class: p.A' + + shouldFailWithMessages '''package q + class B extends p.A { + static String test(p.A a) { + a.qp + } + } + ''', + 'Cannot access method: getQp() of class: p.A' + + shouldFailWithMessages '''package q + class B extends p.A { + static void test(p.A a) { + a.qs = 'v' + } + } + ''', + 'Cannot access method: setQs(java.lang.String) of class: p.A' + + shouldFailWithMessages '''package q + class B extends p.A { + static java.util.function.Supplier test(p.A a) { + a::im + } + } + ''', + 'Cannot access method: im of class: p.A' + + shouldFailWithMessages '''package q + class B extends p.A { + static java.util.function.Function test() { + p.A::im + } + } + ''', + 'Cannot access method: im of class: p.A' + } + + // GROOVY-12568 + @Test + void testProtectedMembersOfAnotherPackageThroughReceiverOfSubclass() { + assertScript '''package p + class A { + protected String im() { 'im' } + protected static String sm() { 'sm' } + protected String getQp() { 'qp' } + protected String f = 'f' + } + true + ''' + assertScript '''package q + import java.util.function.* + + class B extends p.A { + List test(B b, p.A a) { + Supplier bound = this::im + Function unbound = B::im + a.@f = 'w' // field of a receiver of the superclass: written through the meta-class + [im(), this.im(), super.im(), b.im(), sm(), p.A.sm(), qp, b.qp, bound.get(), unbound.apply(b), a.@f] + } + } + assert new B().test(new B(), new B()) == ['im', 'im', 'im', 'im', 'sm', 'sm', 'qp', 'qp', 'im', 'im', 'w'] + ''' + } } From d13e57abda38e34386fbeb2ca4edc2bd67f0b1c5 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 22:36:21 -0700 Subject: [PATCH 07/18] GROOVY-12568: create the object through the meta-class when invokedynamic cannot access the constructor A dynamic new of a protected constructor of another package, in a subclass, is accepted by Groovy and by the type checker. With invokedynamic, the constructor selector unreflected the selected constructor with the lookup of the calling class and turned an IllegalAccessException into GroovyBugError. For a constructor that the JVM refuses to the caller, such as the protected ObjectInputStream() of the encapsulated java.io package called by new in a subclass, the call failed with "BUG! UNCAUGHT EXCEPTION: member is protected". As the property selectors do since GROOVY-12314, the constructor selector now treats the refusal as an access-control outcome and leaves the call to the meta-class, which reports "could not access constructor", as the call does without invokedynamic. A constructor that reflection can reach, such as a protected constructor of a class of another package in the unnamed module, is still called. --- .../codehaus/groovy/vmplugin/v8/Selector.java | 6 +- src/test/groovy/bugs/Groovy12568.groovy | 86 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 src/test/groovy/bugs/Groovy12568.groovy diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java index 0279b947cb7..bfe54baab5d 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java @@ -699,7 +699,11 @@ public void setHandleForMetaMethod() { handle = this.callSite.getLookup().unreflectConstructor(con); if (LOG_ENABLED) LOG.info("successfully unreflected constructor"); } catch (IllegalAccessException e) { - throw new GroovyBugError(e); + // refusal is an access-control outcome, not an internal error, for example a protected + // constructor of an encapsulated JDK class called by new in a subclass: leave the handle + // unset so that the meta-class creates the object, as without invokedynamic + if (LOG_ENABLED) LOG.info("constructor refused, using meta class path"); + return; } } else { super.setHandleForMetaMethod(); diff --git a/src/test/groovy/bugs/Groovy12568.groovy b/src/test/groovy/bugs/Groovy12568.groovy new file mode 100644 index 00000000000..62a78d46904 --- /dev/null +++ b/src/test/groovy/bugs/Groovy12568.groovy @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package bugs + +import org.codehaus.groovy.control.CompilerConfiguration +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +import static groovy.test.GroovyAssert.shouldFail +import static org.junit.jupiter.api.Assumptions.assumeFalse + +/** + * A dynamic {@code new} of a protected constructor of another package, in a + * subclass, is selected through the meta-class. When the JVM refuses access to + * the constructor, as for an encapsulated JDK class, the call fails with the + * same Groovy exception with and without invokedynamic. + */ +final class Groovy12568 { + + private static GroovyShell shell(final boolean indy) { + def config = new CompilerConfiguration() + config.optimizationOptions[CompilerConfiguration.INVOKEDYNAMIC] = indy + new GroovyShell(Groovy12568.classLoader, new Binding(), config) + } + + // GROOVY-12568 + @ParameterizedTest(name = 'indy={0}') + @ValueSource(booleans = [true, false]) + void testProtectedConstructorOfEncapsulatedJdkClass(final boolean indy) { + assumeFalse(ObjectInputStream.module.isOpen('java.io', Groovy12568.module), + 'the JVM opens java.base/java.io, so the constructor is reflectively reachable') + + def err = shouldFail(GroovyRuntimeException) { + shell(indy).evaluate ''' + class C extends ObjectInputStream { + static ObjectInputStream make() { + new ObjectInputStream() + } + } + C.make() + ''' + } + // was, with invokedynamic: GroovyBugError "BUG! UNCAUGHT EXCEPTION: member is protected" + assert err.class == GroovyRuntimeException + assert err.message.startsWith('could not access constructor: protected java.io.ObjectInputStream()') + } + + // GROOVY-12568 + @ParameterizedTest(name = 'indy={0}') + @ValueSource(booleans = [true, false]) + void testProtectedConstructorOfClassOfAnotherPackage(final boolean indy) { + def shell = shell(indy) + shell.evaluate '''package p + class A { + final String value + protected A(String value) { this.value = value } + } + true + ''' + assert shell.evaluate('''package q + class C extends p.A { + C() { super('c') } + static p.A make() { + new p.A('x') + } + } + C.make().value + ''') == 'x' + } +} From 49f1ac45293e2ee8b17051130e35b4174a334261 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 22:41:02 -0700 Subject: [PATCH 08/18] GROOVY-12568: do not generate a super$ method for a package-private method of another run-time package For a dynamic super.m() call, the class gets a synthetic super$N$m method that calls the superclass method with INVOKESPECIAL. For a package-private method of a superclass with the same package name but of another compilation unit, the classes may be in different run-time packages, for example when the superclass comes from an earlier parse of the same GroovyShell, and the JVM refuses the call: super.m() failed with IllegalAccessError. Such a method, and a package-private method of a superclass of another package, no longer gets a super$ method, and the meta-class calls the superclass method reflectively, as it does for a statically compiled super.m() of such a method. --- .../groovy/classgen/asm/MopWriter.java | 5 ++++ src/test/groovy/bugs/Groovy12568.groovy | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java index 129c755d14c..c6e3659114b 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java @@ -34,6 +34,7 @@ import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toSet; +import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName; import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveDouble; import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveLong; import static org.objectweb.asm.Opcodes.ACC_ABSTRACT; @@ -153,6 +154,10 @@ private void visitMopMethodList(final Iterable methods, final boolea // super$ method for non-private isThis=false // --> results in XOR if (isThis ^ mn.isPrivate()) continue; + // a super$ method cannot call a package-private method of another run-time package; the class of + // another compilation unit may come from another class loader even with the same package name + if (!isThis && mn.isPackageScope() && !(mn.getDeclaringClass().isPrimaryClassNode() + && samePackageName(mn.getDeclaringClass(), controller.getClassNode()))) continue; String methodName = mn.getName(); Parameter[] parameters = mn.getParameters(); diff --git a/src/test/groovy/bugs/Groovy12568.groovy b/src/test/groovy/bugs/Groovy12568.groovy index 62a78d46904..778ed8f0ca8 100644 --- a/src/test/groovy/bugs/Groovy12568.groovy +++ b/src/test/groovy/bugs/Groovy12568.groovy @@ -83,4 +83,31 @@ final class Groovy12568 { C.make().value ''') == 'x' } + + // GROOVY-12568 + @ParameterizedTest(name = 'indy={0}') + @ValueSource(booleans = [true, false]) + void testSuperCallOfPackagePrivateMethodOfClassOfAnotherClassLoader(final boolean indy) { + def shell = shell(indy) + shell.evaluate '''package p + class M { + @groovy.transform.PackageScope String pm() { 'pm' } + } + true + ''' + assert shell.evaluate('''package p + class B extends M { // same package name as M, but another class loader + def test() { + super.pm() + } + } + @groovy.transform.CompileStatic + class C extends M { + def test() { + super.pm() + } + } + [new B().test(), new C().test()] + ''') == ['pm', 'pm'] + } } From 966ee1ee8b5e22e5b494380ffcc53822f60f132d Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 23:06:20 -0700 Subject: [PATCH 09/18] GROOVY-12568: use non-public members of another compilation unit through the meta-class only for classes defined in memory The previous commits accessed a protected or package-private member of a class with the same package name through the meta-class whenever that class was compiled by another compilation unit. That also hit class files: a Java class of a joint compilation, a main class used by a test, a class left over from an incremental compilation. Such classes are loaded by one class loader at run time, as javac assumes, so the direct access worked; the meta-class path was slower, followed meta-class semantics, needed reflection metadata in a native image, and made an incremental build give other bytecode than a full one. The run-time package of such a class only differs when the classes are defined in memory by a GroovyClassLoader, which defines each compilation (a GroovyShell script, parseClass, JSR-223) with an InnerLoader of its own. GroovyClassLoader now records that on the compilation unit, and AsmClassGenerator.isInSameRuntimePackage treats a class of another compilation unit as another run-time package only then. Constructors, map-style construction, methods, getters and setters, method references, fields, dynamic super and this.@ access and super$ methods all use this check. Compiling to class files keeps direct access, as before these changes and as javac does. (The rest of the change: new, map-style construction, methods, getters and setters, method references to methods, fields, dynamic super and this.@ access and super$ methods. The GroovyClassLoader marker, isInSameRuntimePackage and constructor references come first.) --- .../groovy/classgen/AsmClassGenerator.java | 11 +- .../groovy/classgen/asm/MopWriter.java | 8 +- .../asm/sc/StaticInvocationWriter.java | 8 +- .../asm/sc/StaticTypesCallSiteWriter.java | 22 ++- ...cTypesMethodReferenceExpressionWriter.java | 17 +- .../sc/StaticCompileRuntimePackageTest.groovy | 163 ++++++++++++++++-- 6 files changed, 182 insertions(+), 47 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index cd57d86925f..df24abf504f 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -1309,17 +1309,14 @@ private static String getStaticFieldName(final ClassNode type) { * (via {@code GETFIELD}, {@code GETSTATIC}, etc. bytecode instructions). *

* A protected or package-private field of a class with the same package - * name is accessible only within the same run-time package, which is the - * package name and the class loader. GroovyClassLoader defines all the - * classes of a compilation unit with one class loader, but a class of - * another compilation unit may come from another class loader, so its - * field is directly accessible only through the subclass rule for a - * protected field. + * name is accessible only within the same run-time package; see + * {@link #isInSameRuntimePackage}. Otherwise it is directly accessible + * only through the subclass rule for a protected field. */ public static boolean isFieldDirectlyAccessible(final FieldNode field, final ClassNode accessingClass) { if (field == null || !isMemberDirectlyAccessible(field.getModifiers(), field.getDeclaringClass(), accessingClass)) return false; ClassNode declaringClass = field.getDeclaringClass(); - return field.isPublic() || field.isPrivate() || declaringClass.isPrimaryClassNode() + return field.isPublic() || field.isPrivate() || isInSameRuntimePackage(declaringClass, accessingClass) || (field.isProtected() && accessingClass.isDerivedFrom(declaringClass)); } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java index c6e3659114b..d194e383d7c 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java @@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.tools.ParameterUtils; +import org.codehaus.groovy.classgen.AsmClassGenerator; import org.objectweb.asm.MethodVisitor; import java.util.Collections; @@ -34,7 +35,6 @@ import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toSet; -import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName; import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveDouble; import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveLong; import static org.objectweb.asm.Opcodes.ACC_ABSTRACT; @@ -154,10 +154,8 @@ private void visitMopMethodList(final Iterable methods, final boolea // super$ method for non-private isThis=false // --> results in XOR if (isThis ^ mn.isPrivate()) continue; - // a super$ method cannot call a package-private method of another run-time package; the class of - // another compilation unit may come from another class loader even with the same package name - if (!isThis && mn.isPackageScope() && !(mn.getDeclaringClass().isPrimaryClassNode() - && samePackageName(mn.getDeclaringClass(), controller.getClassNode()))) continue; + // a super$ method cannot call a package-private method of another run-time package + if (!isThis && mn.isPackageScope() && !AsmClassGenerator.isInSameRuntimePackage(mn.getDeclaringClass(), controller.getClassNode())) continue; String methodName = mn.getName(); Parameter[] parameters = mn.getParameters(); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index 69943a615ff..5495608f840 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -383,12 +383,12 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i receiverType = receiverType.getGenericsTypes()[0].getType(); // GROOVY-11694 } - if (!target.isPublic() && !target.isPrivate() && !declaringClass.isPrimaryClassNode() - && samePackageName(declaringClass, enclosingClass) + if (!target.isPublic() && !target.isPrivate() && samePackageName(declaringClass, enclosingClass) + && !AsmClassGenerator.isInSameRuntimePackage(declaringClass, enclosingClass) && !(target.isProtected() && enclosingClass.isDerivedFrom(declaringClass) && (target.isStatic() || isSuperExpression(receiver) || isSubtype(enclosingClass, receiverType)))) { - // a protected or package-private method of a class of another compilation unit may be in another run-time - // package, which is the package name and the class loader: select the method at run time + // a protected or package-private method of a class of another run-time package, which is the package + // name and the class loader, is selected at run time (except for the subclass rule of a protected method) return false; } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java index 2e8b36a04bc..f0a706f4a3d 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java @@ -234,7 +234,7 @@ && getField(receiverType, propertyName, FieldNode::isPublic) == null) { } } - if (isFieldOfAnotherCompilationUnit(receiverType, propertyName)) { + if (isFieldOfAnotherRuntimePackage(receiverType, propertyName)) { makeDynamicGetProperty(receiver, propertyName, safe); return; } @@ -246,15 +246,15 @@ && getField(receiverType, propertyName, FieldNode::isPublic) == null) { /** * Determines if the property is a protected or package-private field of a - * class with the same package name but of another compilation unit, which - * may be in another run-time package. Such a field is read through the - * meta-class when the current class cannot access it directly. + * class with the same package name but of another run-time package (see + * {@link AsmClassGenerator#isInSameRuntimePackage}). Such a field is read + * through the meta-class when the current class cannot access it directly. */ - private boolean isFieldOfAnotherCompilationUnit(final ClassNode receiverType, final String propertyName) { + private boolean isFieldOfAnotherRuntimePackage(final ClassNode receiverType, final String propertyName) { FieldNode field = getField(receiverType, propertyName); return field != null && !field.isPublic() && !field.isPrivate() - && !field.getDeclaringClass().isPrimaryClassNode() - && samePackageName(field.getDeclaringClass(), controller.getClassNode()); + && samePackageName(field.getDeclaringClass(), controller.getClassNode()) + && !AsmClassGenerator.isInSameRuntimePackage(field.getDeclaringClass(), controller.getClassNode()); } private void makeDynamicGetProperty(final Expression receiver, final String propertyName, final boolean safe) { @@ -424,7 +424,7 @@ && getField(receiverType, propertyName, FieldNode::isPublic) == null return; } - if (isFieldOfAnotherCompilationUnit(receiverType, propertyName)) { + if (isFieldOfAnotherRuntimePackage(receiverType, propertyName)) { makeDynamicGetProperty(receiver, propertyName, safe); return; } @@ -560,8 +560,7 @@ boolean makeGetField(final Expression receiver, final ClassNode receiverType, fi ClassNode currentClass = controller.getClassNode(); ClassNode castType = field.getOwner(); if (field.isProtected() && !currentClass.equals(field.getOwner()) - // a class of another compilation unit may come from another class loader - && !(samePackageName(currentClass, field.getOwner()) && field.getOwner().isPrimaryClassNode())) { + && !AsmClassGenerator.isInSameRuntimePackage(field.getOwner(), currentClass)) { if (receiverType.isDerivedFrom(currentClass)) castType = currentClass; else return false; } @@ -888,8 +887,7 @@ private boolean setField(final PropertyExpression expression, final Expression o if (fn == null) return false; ClassNode currentClass = controller.getClassNode(); if (!fn.isStatic() && fn.isProtected() && !currentClass.equals(fn.getOwner()) - // a class of another compilation unit may come from another class loader - && !(samePackageName(currentClass, fn.getOwner()) && fn.getOwner().isPrimaryClassNode()) + && !AsmClassGenerator.isInSameRuntimePackage(fn.getOwner(), currentClass) && !receiverType.isDerivedFrom(currentClass)) { return false; // JVMS 4.10.1.8: in another run-time package, the receiver must be of the current class } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java index b0da72cf173..75b98c0fabf 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java @@ -276,7 +276,7 @@ private ResolvedMethodReference adaptMethodReference(final FunctionalInterfaceCo if (methodRefMethod.isStatic() && !referenceTarget.targetIsArgument()) { referenceTarget = referenceTarget.asClassTarget(methodRefMethod.getDeclaringClass()); } - } else if (mayBeInAnotherRuntimePackage(methodRefMethod)) { + } else if (isInAnotherRuntimePackage(methodRefMethod)) { if (!referenceTarget.classExpression() && !methodRefMethod.isStatic()) { referenceTarget = referenceTarget.markTargetAsArgument(); } @@ -291,15 +291,14 @@ private ResolvedMethodReference adaptMethodReference(final FunctionalInterfaceCo /** * Determines if the method is a protected or package-private method of a - * class with the same package name but of another compilation unit. Such a - * class may come from another class loader, so it may be in another - * run-time package, and the JVM would then refuse a method handle for the - * method in the current class. + * class with the same package name but of another run-time package (see + * {@link AsmClassGenerator#isInSameRuntimePackage}). The JVM refuses a + * method handle for such a method in the current class. */ - private boolean mayBeInAnotherRuntimePackage(final MethodNode mn) { - ClassNode declaringClass = mn.getDeclaringClass(); - return !mn.isPublic() && !mn.isPrivate() && !declaringClass.isPrimaryClassNode() - && samePackageName(declaringClass, controller.getClassNode()); + private boolean isInAnotherRuntimePackage(final MethodNode mn) { + ClassNode declaringClass = mn.getDeclaringClass(), classNode = controller.getClassNode(); + return !mn.isPublic() && !mn.isPrivate() && samePackageName(declaringClass, classNode) + && !AsmClassGenerator.isInSameRuntimePackage(declaringClass, classNode); } /** diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index b9a155bf6e0..3f4c0dcd90b 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -23,17 +23,21 @@ import org.codehaus.groovy.classgen.asm.InstructionSequence import org.codehaus.groovy.control.CompilationUnit import org.codehaus.groovy.control.CompilerConfiguration import org.codehaus.groovy.control.Phases +import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit import org.junit.jupiter.api.Test +import java.security.CodeSource + /** * Unit tests for static compilation: protected and package-private members of * a class with the same package name, compiled by another compilation unit. *

* The JVM allows such access only within the same run-time package, which is * the package name and the class loader. GroovyClassLoader defines the classes - * of each compilation unit with its own class loader, so a class of an earlier - * parse of the same GroovyShell, or a precompiled class used by a script, is - * in another run-time package. + * of each compilation in memory with a class loader of their own, so a class of + * an earlier parse of the same GroovyShell, or a precompiled class used by a + * script, is in another run-time package. Class files are expected to be loaded + * by one class loader, as javac expects. */ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { @@ -474,20 +478,159 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { ''') == ['pkg:1', 'pm', 'spm', 'qp', 'pm', 'w', 'spf', 'DYNAMIC'] } + // GROOVY-12568 + @Test + void testProtectedAndPackagePrivateMembersOfClassCompiledBeforeToClassFilesAreAccessedDirectly() { + def dir = File.createTempDir() + try { + compileToDirectory(dir, 'Library.groovy': LIBRARY + MEMBERS.replace('package p', '') + FIELDS.replace('package p', '')) + compileToDirectory(dir, 'C.groovy': '''package p + @groovy.transform.CompileStatic + class C { // compiled by another compilation unit, to class files like groovyc + static List test(M m, F f) { + f.@pf = 'w' + [new A(1).value, m.pm(), m.qm(), M.spm(), f.pf, F.spf] + } + static java.util.function.Function ctorRef() { + A::new + } + static java.util.function.Supplier methodRef(M m) { + m::pm + } + } + ''') + + def bytecode = methodSequence(new File(dir, 'p/C.class').bytes, 'test') + assert bytecode.hasSequence(['INVOKESPECIAL p/A. (Ljava/lang/Integer;)V']) + assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.pm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.qm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKESTATIC p/M.spm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['PUTFIELD p/F.pf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETFIELD p/F.pf : Ljava/lang/String;']) + assert bytecode.hasSequence(['GETSTATIC p/F.spf : Ljava/lang/String;']) + assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invoke']) + assert !bytecode.hasSequence(['InvokerHelper.getProperty']) + assert methodSequence(new File(dir, 'p/C.class').bytes, 'ctorRef$ctorRef$0').hasSequence(['NEW p/A', 'DUP', 'ALOAD 0', 'INVOKESPECIAL p/A. (Ljava/lang/Integer;)V']) + assert methodSequence(new File(dir, 'p/C.class').bytes, 'methodRef').hasSequence(['// handle kind 0x5 : INVOKEVIRTUAL', 'p/M.pm()Ljava/lang/String;']) + + // one class loader for all the class files, as javac expects + def loader = new URLClassLoader([dir.toURI().toURL()] as URL[], this.class.classLoader) + def m = loader.loadClass('p.M').getDeclaredConstructor().newInstance() + def f = loader.loadClass('p.F').getDeclaredConstructor().newInstance() + def c = loader.loadClass('p.C') + assert c.test(m, f) == ['pkg:1', 'pm', 'qm', 'spm', 'w', 'spf'] + assert c.ctorRef().apply(2).value == 'pkg:2' + assert c.methodRef(m).get() == 'pm' + } finally { + dir.deleteDir() + } + } + + // GROOVY-12568 + @Test + void testFullAndIncrementalCompilationToClassFilesGiveTheSameBytecode() { + def caller = '''package p + @groovy.transform.CompileStatic + class C { + static List test(M m, F f) { + java.util.function.Supplier ref = m::pm + [new A(1).value, m.pm(), M.spm(), f.pf, F.spf, ref.get()] + } + } + ''' + def library = LIBRARY + MEMBERS.replace('package p', '') + FIELDS.replace('package p', '') + def full = File.createTempDir(), incremental = File.createTempDir() + try { + compileToDirectory(full, 'Library.groovy': library, 'C.groovy': caller) + compileToDirectory(incremental, 'Library.groovy': library) + compileToDirectory(incremental, 'C.groovy': caller) // C is recompiled alone + + assert new File(incremental, 'p/C.class').bytes == new File(full, 'p/C.class').bytes + } finally { + full.deleteDir() + incremental.deleteDir() + } + } + + // GROOVY-12568 + @Test + void testPackagePrivateMembersOfJavaClassOfJointCompilationAreAccessedDirectly() { + def dir = File.createTempDir(), src = File.createTempDir() + try { + new File(src, 'p').mkdirs() + def j = new File(src, 'p/J.java') + j.text = '''package p; + public class J { + String pf = "pf"; + J() { } + String pm() { return "pm"; } + protected String qm() { return "qm"; } + static String spm() { return "spm"; } + } + ''' + def g = new File(src, 'p/G.groovy') + g.text = '''package p + @groovy.transform.CompileStatic + class G { + static List test() { + J j = new J() + [j.pm(), j.qm(), J.spm(), j.pf] + } + } + ''' + def config = new CompilerConfiguration(targetDirectory: dir, jointCompilationOptions: [memStub: true]) + def unit = new JavaAwareCompilationUnit(config, new GroovyClassLoader(this.class.classLoader)) + unit.addSources(j, g) + unit.compile() + + def bytecode = methodSequence(new File(dir, 'p/G.class').bytes, 'test') + assert bytecode.hasSequence(['NEW p/J', 'DUP', 'INVOKESPECIAL p/J. ()V']) + assert bytecode.hasSequence(['INVOKEVIRTUAL p/J.pm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKEVIRTUAL p/J.qm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['INVOKESTATIC p/J.spm ()Ljava/lang/String;']) + assert bytecode.hasSequence(['GETFIELD p/J.pf : Ljava/lang/String;']) + assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invoke']) + + def loader = new URLClassLoader([dir.toURI().toURL()] as URL[], this.class.classLoader) + assert loader.loadClass('p.G').test() == ['pm', 'qm', 'spm', 'pf'] + } finally { + dir.deleteDir() + src.deleteDir() + } + } + + /** + * Compiles the sources to class files in {@code dir}, against the class + * files already there, as groovyc does. + */ + private static void compileToDirectory(final Map sources, final File dir) { + def config = new CompilerConfiguration(targetDirectory: dir, classpathList: [dir.path]) + def unit = new CompilationUnit(config, null, new GroovyClassLoader(StaticCompileRuntimePackageTest.classLoader, config)) + sources.each { name, text -> unit.addSource(name, text) } + unit.compile(Phases.OUTPUT) + } + /** * Compiles {@code script} against the classes of {@code library}, which are - * compiled before, by another compilation unit and class loader. + * compiled before, by another parse of the same GroovyClassLoader. */ private InstructionSequence compileAgainstLibrary(final String library, final String className, final String method, final String script) { - def loader = new GroovyClassLoader() + CompilationUnit unit = null + def loader = new GroovyClassLoader() { + @Override + protected CompilationUnit createCompilationUnit(final CompilerConfiguration config, final CodeSource source) { + unit = super.createCompilationUnit(config, source) + } + } loader.parseClass(library) - def unit = new CompilationUnit(null, null, loader) - unit.addSource('script', script) - unit.compile(Phases.CLASS_GENERATION) + loader.parseClass(script) def generated = unit.classes.find { it.name == className } assert generated != null : "class $className not found" - def sequence = extractSequence(generated.bytes, [method: method]) - List instructions = sequence.instructions + methodSequence(generated.bytes, method) + } + + private InstructionSequence methodSequence(final byte[] bytes, final String method) { + List instructions = extractSequence(bytes, [method: method]).instructions int begin = instructions.indexOf('--BEGIN--'), end = instructions.indexOf('--END--') assert begin >= 0 && end > begin : "method $method not found" new InstructionSequence(instructions: instructions[(begin + 1).. Date: Sat, 24 Oct 2026 23:18:34 -0700 Subject: [PATCH 10/18] GROOVY-12568: name the run-time package tests after the class loader The tests for protected and package-private members of a class of another compilation now compile it through a GroovyClassLoader, which defines it with another class loader; classes compiled to class files keep direct access. Name the tests accordingly. --- .../sc/StaticCompileRuntimePackageTest.groovy | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index 3f4c0dcd90b..5778f36b443 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -30,7 +30,8 @@ import java.security.CodeSource /** * Unit tests for static compilation: protected and package-private members of - * a class with the same package name, compiled by another compilation unit. + * a class with the same package name, compiled by another compilation and so + * maybe defined by another class loader. *

* The JVM allows such access only within the same run-time package, which is * the package name and the class loader. GroovyClassLoader defines the classes @@ -148,7 +149,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testProtectedAndPackagePrivateConstructorsOfClassOfAnotherCompilationUnitAreSelectedAtRuntime() { + void testProtectedAndPackagePrivateConstructorsOfClassOfAnotherClassLoaderAreSelectedAtRuntime() { def bytecode = compileAgainstLibrary(LIBRARY, 'p.C', 'make', '''package p @groovy.transform.CompileStatic class C { @@ -163,7 +164,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testPublicConstructorOfClassOfAnotherCompilationUnitIsCalledDirectly() { + void testPublicConstructorOfClassOfAnotherClassLoaderIsCalledDirectly() { def bytecode = compileAgainstLibrary('''package p class B { B(String s) { } @@ -237,7 +238,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testProtectedAndPackagePrivateMethodsOfClassOfAnotherCompilationUnitAreSelectedAtRuntime() { + void testProtectedAndPackagePrivateMethodsOfClassOfAnotherClassLoaderAreSelectedAtRuntime() { def bytecode = compileAgainstLibrary(MEMBERS, 'p.C', 'test', '''package p @groovy.transform.CompileStatic class C { @@ -257,7 +258,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testProtectedMethodsOfSuperclassOfAnotherCompilationUnitAreCalledDirectly() { + void testProtectedMethodsOfSuperclassOfAnotherClassLoaderAreCalledDirectly() { def bytecode = compileAgainstLibrary(MEMBERS, 'p.B', 'test', '''package p @groovy.transform.CompileStatic class B extends M { @@ -319,7 +320,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testMethodReferenceToPackagePrivateMethodOfClassOfAnotherCompilationUnitIsSelectedAtRuntime() { + void testMethodReferenceToPackagePrivateMethodOfClassOfAnotherClassLoaderIsSelectedAtRuntime() { def script = '''package p @groovy.transform.CompileStatic class C { @@ -416,7 +417,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testProtectedAndPackagePrivateFieldsOfClassOfAnotherCompilationUnitAreAccessedAtRuntime() { + void testProtectedAndPackagePrivateFieldsOfClassOfAnotherClassLoaderAreAccessedAtRuntime() { def bytecode = compileAgainstLibrary(FIELDS, 'p.C', 'test', '''package p @groovy.transform.CompileStatic class C { @@ -437,7 +438,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { // GROOVY-12568 @Test - void testProtectedFieldOfSuperclassOfAnotherCompilationUnitIsAccessedDirectly() { + void testProtectedFieldOfSuperclassOfAnotherClassLoaderIsAccessedDirectly() { def bytecode = compileAgainstLibrary(FIELDS, 'p.B', 'test', '''package p @groovy.transform.CompileStatic class B extends F { From 141f384fce754fa34ea328ae5e5f6108b7ceb890 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sun, 25 Oct 2026 09:16:36 -0700 Subject: [PATCH 11/18] GROOVY-12568: document the run-time package, subclass and meta-class rules of constructor references - isInSameRuntimePackage: isPrimaryClassNode() stands for "defined by the class loader of the accessing class", not for "compiled from source". - StaticInvocationWriter.isDirectlyAccessible: unlike isMemberDirectlyAccessible it has no subclass rule, since the verifier rejects NEW and INVOKESPECIAL of a protected constructor of another package in a subclass too. - findConstructorReferenceTarget: a selected constructor is called directly, so a meta-class constructor override is not used, as for a statically compiled new. The guard comment now says why enums are left to the meta-class: the type checker accepts an enum constructor reference whose arguments match the generated constructor, but Java code cannot instantiate an enum. - addSyntheticMethod: the ACC_FINAL rule of JVMS 4.6 applies to every helper method of a method reference in an interface. (The part on statically compiled new; the rest comes with constructor references.) --- .../groovy/classgen/asm/sc/StaticInvocationWriter.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index 5495608f840..fe91be83cce 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -202,8 +202,9 @@ public void writeInvokeConstructor(final ConstructorCallExpression call) { *

* Unlike {@link AsmClassGenerator#isMemberDirectlyAccessible}, there is no * subclass rule: the verifier rejects {@code NEW} and {@code INVOKESPECIAL} - * of a protected constructor of another package in a subclass too, so a - * constructor reference to it creates the object through the meta-class. + * of a protected constructor of another package in a subclass too, so + * {@code new} of it is reported and a constructor reference to it creates + * the object through the meta-class. */ static boolean isDirectlyAccessible(final ConstructorNode ctor, final ClassNode accessingClass) { if (ctor.isPublic()) return true; From dc1d4ea510634c53fb55ce6bab66017d631f3b4c Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sun, 25 Oct 2026 09:25:07 -0700 Subject: [PATCH 12/18] GROOVY-12568: test every kind of method-reference helper method in an interface All helper methods that a method reference adds to the current class are created by addSyntheticMethod, which leaves out ACC_FINAL in an interface. The interface tests covered the constructor-reference and variadic helpers; they now also cover the array constructor reference, the static extension method helper (Thread::startDaemon), the Class method helper (String::getName) and the helper that calls a package-private method of a class of another class loader (M::spm, M::pm), in static and default methods, and check that every helper is private and not final. With ACC_FINAL put back, both tests fail with ClassFormatError (illegal modifiers 0x101A). (The case of a method of another run-time package; the other helper methods are tested with constructor references.) --- .../sc/StaticCompileRuntimePackageTest.groovy | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index 5778f36b443..67ffc663565 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -303,6 +303,23 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { ''') == ['pm', 'qm', 'spm', 2, 3, 'pm', 'qm', 'sqm'] } + // GROOVY-12568 + @Test + void testMethodReferencesOfInterfaceToPackagePrivateMethodsOfClassOfAnotherClassLoader() { + shell.evaluate MEMBERS + assert shell.evaluate('''package p + import java.util.function.* + + @groovy.transform.CompileStatic + interface I { // same package name as M, but another class loader + static Supplier stat() { M::spm } + default Function unbound() { M::pm } + } + assert I.declaredMethods.findAll { it.name.startsWith('access$') }.size() == 2 + [I.stat().get(), new I() {}.unbound().apply(new M())] + ''') == ['spm', 'pm'] + } + // GROOVY-12568 @Test void testMethodReferenceToPackagePrivateMethodOfSameCompilationUnitIsDirect() { From 8afe728380068b2f98e7bcec6cd194129bcc45f9 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sun, 25 Oct 2026 09:27:51 -0700 Subject: [PATCH 13/18] GROOVY-12568: share the compile-against-a-library bytecode helper in AbstractBytecodeTestCase MethodReferenceTest and StaticCompileRuntimePackageTest each had a copy of the helper that compiles a script against the classes of an earlier parse of the same GroovyClassLoader, and of the code that keeps only the instructions of one method. AbstractBytecodeTestCase now provides them as compileAgainstLibrary, extractMethodSequence and selectMethod, and both tests use them. Test code only; the tests are unchanged. (Here StaticCompileRuntimePackageTest; MethodReferenceTest was moved to the helpers with constructor references.) --- .../sc/StaticCompileRuntimePackageTest.groovy | 37 ++----------------- 1 file changed, 4 insertions(+), 33 deletions(-) diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index 67ffc663565..7beaed226ab 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -19,15 +19,12 @@ package org.codehaus.groovy.classgen.asm.sc import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase -import org.codehaus.groovy.classgen.asm.InstructionSequence import org.codehaus.groovy.control.CompilationUnit import org.codehaus.groovy.control.CompilerConfiguration import org.codehaus.groovy.control.Phases import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit import org.junit.jupiter.api.Test -import java.security.CodeSource - /** * Unit tests for static compilation: protected and package-private members of * a class with the same package name, compiled by another compilation and so @@ -518,7 +515,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { } ''') - def bytecode = methodSequence(new File(dir, 'p/C.class').bytes, 'test') + def bytecode = extractMethodSequence(new File(dir, 'p/C.class').bytes, 'test') assert bytecode.hasSequence(['INVOKESPECIAL p/A. (Ljava/lang/Integer;)V']) assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.pm ()Ljava/lang/String;']) assert bytecode.hasSequence(['INVOKEVIRTUAL p/M.qm ()Ljava/lang/String;']) @@ -528,8 +525,8 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { assert bytecode.hasSequence(['GETSTATIC p/F.spf : Ljava/lang/String;']) assert !bytecode.hasSequence(['ScriptBytecodeAdapter.invoke']) assert !bytecode.hasSequence(['InvokerHelper.getProperty']) - assert methodSequence(new File(dir, 'p/C.class').bytes, 'ctorRef$ctorRef$0').hasSequence(['NEW p/A', 'DUP', 'ALOAD 0', 'INVOKESPECIAL p/A. (Ljava/lang/Integer;)V']) - assert methodSequence(new File(dir, 'p/C.class').bytes, 'methodRef').hasSequence(['// handle kind 0x5 : INVOKEVIRTUAL', 'p/M.pm()Ljava/lang/String;']) + assert extractMethodSequence(new File(dir, 'p/C.class').bytes, 'ctorRef$ctorRef$0').hasSequence(['NEW p/A', 'DUP', 'ALOAD 0', 'INVOKESPECIAL p/A. (Ljava/lang/Integer;)V']) + assert extractMethodSequence(new File(dir, 'p/C.class').bytes, 'methodRef').hasSequence(['// handle kind 0x5 : INVOKEVIRTUAL', 'p/M.pm()Ljava/lang/String;']) // one class loader for all the class files, as javac expects def loader = new URLClassLoader([dir.toURI().toURL()] as URL[], this.class.classLoader) @@ -601,7 +598,7 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { unit.addSources(j, g) unit.compile() - def bytecode = methodSequence(new File(dir, 'p/G.class').bytes, 'test') + def bytecode = extractMethodSequence(new File(dir, 'p/G.class').bytes, 'test') assert bytecode.hasSequence(['NEW p/J', 'DUP', 'INVOKESPECIAL p/J. ()V']) assert bytecode.hasSequence(['INVOKEVIRTUAL p/J.pm ()Ljava/lang/String;']) assert bytecode.hasSequence(['INVOKEVIRTUAL p/J.qm ()Ljava/lang/String;']) @@ -627,30 +624,4 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { sources.each { name, text -> unit.addSource(name, text) } unit.compile(Phases.OUTPUT) } - - /** - * Compiles {@code script} against the classes of {@code library}, which are - * compiled before, by another parse of the same GroovyClassLoader. - */ - private InstructionSequence compileAgainstLibrary(final String library, final String className, final String method, final String script) { - CompilationUnit unit = null - def loader = new GroovyClassLoader() { - @Override - protected CompilationUnit createCompilationUnit(final CompilerConfiguration config, final CodeSource source) { - unit = super.createCompilationUnit(config, source) - } - } - loader.parseClass(library) - loader.parseClass(script) - def generated = unit.classes.find { it.name == className } - assert generated != null : "class $className not found" - methodSequence(generated.bytes, method) - } - - private InstructionSequence methodSequence(final byte[] bytes, final String method) { - List instructions = extractSequence(bytes, [method: method]).instructions - int begin = instructions.indexOf('--BEGIN--'), end = instructions.indexOf('--END--') - assert begin >= 0 && end > begin : "method $method not found" - new InstructionSequence(instructions: instructions[(begin + 1).. Date: Thu, 29 Oct 2026 11:32:01 -0700 Subject: [PATCH 14/18] GROOVY-12568: document the run-time package rule and the new compile errors COMPATIBILITY.md describes how statically compiled code of a compilation that a GroovyClassLoader defines in memory reaches protected and package-private members of classes of other compilations, and the new compile errors for new of a protected constructor of another package and for a protected method of another package called through a superclass receiver. StaticInvocationWriter notes that the constructor error is reported during class generation, so @TypeChecked code is not checked for it and works dynamically. --- COMPATIBILITY.md | 27 +++++++++++++++++++ .../asm/sc/StaticInvocationWriter.java | 3 ++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index e13bbcd970e..9705855d837 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -774,6 +774,33 @@ package-private constructor of a class that a `GroovyClassLoader` defined in another compilation, such as a class of an earlier `GroovyShell` parse. Dynamic and `@TypeChecked` code select the constructor at run time as before. +### Groovy 7 — statically compiled code respects the run-time package of a class defined by another `GroovyClassLoader` compilation (GROOVY-12568) + +The JVM allows access to a protected or package-private member only within the +same run-time package, which is the package name and the class loader. A +`GroovyClassLoader` defines each compilation in memory with a class loader of +its own: a `GroovyShell` script, `parseClass`, a script run with `groovy`, +JSR-223. Under `@CompileStatic`, such a compilation now reaches a protected or +package-private constructor, method or field of a class of an earlier +compilation, or of a precompiled class, with the same package name through the +meta-class, as dynamic code does, where it failed with `IllegalAccessError`. +Code compiled to class files (groovyc, Gradle, Maven, Ant, joint compilation +with Java) keeps direct access, as javac does. + +**Who is affected.** Statically compiled code that reaches such members of a +class defined by another compilation now follows meta-class semantics for them, +like dynamic code; in a GraalVM native image those accesses need reflection +metadata. + +**New compile errors.** Under `@CompileStatic`, `new A(...)` of a protected +constructor of another package in a subclass is now reported ("Cannot call +protected constructor"); as in Java, only `super(...)` and `new A(...) {}` can +call it. A protected method of another package called through a receiver of +the superclass type (`a.m()`, `a.name`, `a::m`, `A::m`) is reported as "Cannot +access method". Both used to compile and then fail verification, so no working +code is affected. Dynamic and `@TypeChecked` code, which call them through the +meta-class, are unchanged. + ## The binary-compatibility check The [`subprojects/binary-compatibility/`](subprojects/binary-compatibility) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index fe91be83cce..bc1806fa51b 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -158,7 +158,8 @@ public void writeInvokeConstructor(final ConstructorCallExpression call) { writeDynamicConstructorCall(call, declaringClass); return; } - // JLS 6.6.2.2: outside its package, only super(...) or an anonymous inner class can call a protected constructor + // JLS 6.6.2.2: outside its package, only super(...) or an anonymous inner class can call a protected constructor; + // reported here, while the class is generated, so @TypeChecked code is not checked for it and works dynamically controller.getSourceUnit().addError(new SyntaxException( "Cannot call " + (cn.isProtected() ? "protected" : "package-private") + " constructor for " + declaringClass.toString(false) + " from class " + enclosingClass.toString(false), From fc76de7ed9a0ff7da0adaaec2026ff80df6ef9b1 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 29 Oct 2026 11:44:05 -0700 Subject: [PATCH 15/18] GROOVY-12568: test map-style construction from a Map variable of a class of another class loader Since GROOVY-12454, new A(map) with a Map variable, under static compilation, calls the no-argument constructor and then sets the properties of the map at run time. For a class of an earlier parse of the same GroovyShell whose no-argument constructor is package-private, the constructor is called through the meta-class, as for a map literal. --- .../asm/sc/StaticCompileRuntimePackageTest.groovy | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy index 7beaed226ab..771b44b9a3d 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileRuntimePackageTest.groovy @@ -119,9 +119,11 @@ final class StaticCompileRuntimePackageTest extends AbstractBytecodeTestCase { assert shell.evaluate('''package p @groovy.transform.CompileStatic A make() { new A(name: 'n') } - def a = make() - a.value + ':' + a.name - ''') == 'none:n' + @groovy.transform.CompileStatic + A make(Map map) { new A(map) } // a map that is not a literal + def a = make(), b = make(name: 'm') + [a.value + ':' + a.name, b.value + ':' + b.name] + ''') == ['none:n', 'none:m'] } // GROOVY-12568 From 771af39cb1ed2749cbe25fd21d80bc26bc9237af Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 24 Oct 2026 23:12:13 -0700 Subject: [PATCH 16/18] GROOVY-12570: let super.m() reach a package-private method of a superclass of another class loader A class that declares m() and calls super.m(), where m() is package-private in a superclass with the same package name but defined by another class loader, for example a class of an earlier parse of the same GroovyShell, failed with MissingMethodException. The JVM does not let the subclass method override a package-private method of another run-time package, so no super$ method can call it; without one, the meta-class removed the subclass method from the methods for super calls and kept nothing, as the superclass method had already been replaced by the subclass method in the index. The meta-class now keeps the superclass method for super calls in that case. Invoked reflectively it is the superclass method, because the JVM does not consider it overridden. Super calls that a super$ method serves, and package-private methods of another package, are unchanged. --- src/main/java/groovy/lang/MetaClassImpl.java | 31 ++++++++++++++++++-- src/test/groovy/bugs/Groovy12568.groovy | 29 ++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index 55647b6276f..7b4612dd600 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -577,7 +577,12 @@ public void methodNameAction(final Class c, final MetaMethodIndex.Cache e) { if (matchedMethod >= 0) { methods.set(i, mopMethods[matchedMethod]); } else if (!useThis && notSuperUseful(method, c)) { - methods.remove(i--); // not fit for super usage + MetaMethod superMethod = superMethodOfAnotherRuntimePackage(method, c); + if (superMethod != null) { + methods.set(i, superMethod); + } else { + methods.remove(i--); // not fit for super usage + } } } if (!useThis) { @@ -592,7 +597,7 @@ public void methodNameAction(final Class c, final MetaMethodIndex.Cache e) { if (useThis) e.methods = mopMethods[matchedMethod]; else e.methodsForSuper = mopMethods[matchedMethod]; } else if (!useThis && notSuperUseful(method, c)) { - e.methodsForSuper = null; // not fit for super usage + e.methodsForSuper = superMethodOfAnotherRuntimePackage(method, c); // or null: not fit for super usage } } } @@ -635,6 +640,28 @@ private int mopArrayIndex(final MetaMethod method, final String mopName) { return -1; } + /** + * Finds the superclass method that a method declared by class c + * replaced in the index, if it is a package-private method of a + * class with the same package name but another class loader, so of + * another run-time package. The JVM does not let the method of c + * override it, and no super$ method can call it, but invoked + * reflectively it is the method a super call should reach. + */ + private MetaMethod superMethodOfAnotherRuntimePackage(final MetaMethod method, final Class c) { + if (isDGM(method) || c != method.getDeclaringClass().getTheClass()) return null; + for (Class sc = c.getSuperclass(); sc != null; sc = sc.getSuperclass()) { + for (CachedMethod cm : ReflectionCache.getCachedClass(sc).getMethods()) { + if (cm.getName().equals(method.getName()) && !cm.isStatic() + && MetaMethod.equal(cm.getParameterTypes(), method.getParameterTypes())) { + return cm.isPackagePrivate() && sc.getClassLoader() != c.getClassLoader() + && sc.getPackageName().equals(c.getPackageName()) ? cm : null; + } + } + } + return null; + } + private boolean notSuperUseful(final MetaMethod method, final Class c) { return !isDGM(method) && (isBridge(method) || method.isAbstract() // GROOVY-11929 diff --git a/src/test/groovy/bugs/Groovy12568.groovy b/src/test/groovy/bugs/Groovy12568.groovy index 778ed8f0ca8..3b625302300 100644 --- a/src/test/groovy/bugs/Groovy12568.groovy +++ b/src/test/groovy/bugs/Groovy12568.groovy @@ -110,4 +110,33 @@ final class Groovy12568 { [new B().test(), new C().test()] ''') == ['pm', 'pm'] } + + // GROOVY-12570 + @ParameterizedTest(name = 'indy={0}') + @ValueSource(booleans = [true, false]) + void testSuperCallOfPackagePrivateMethodOfClassOfAnotherClassLoaderWithMethodOfSameName(final boolean indy) { + def shell = shell(indy) + shell.evaluate '''package p + class M { + @groovy.transform.PackageScope String pm() { 'M' } + } + true + ''' + assert shell.evaluate('''package p + class B extends M { // same package name as M, but another class loader + String pm() { 'B>' + super.pm() } + def test() { + [super.pm(), pm()] + } + } + @groovy.transform.CompileStatic + class C extends M { + String pm() { 'C>' + super.pm() } + def test() { + [super.pm(), pm()] + } + } + new B().test() + new C().test() + ''') == ['M', 'B>M', 'M', 'C>M'] + } } From ef2eebada2872305d5b253d8db0c11af8d54500e Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Fri, 30 Oct 2026 08:58:25 -0700 Subject: [PATCH 17/18] GROOVY-12570: test the search order and the super index entries of the meta-class --- src/main/java/groovy/lang/MetaClassImpl.java | 3 +- src/test/groovy/bugs/Groovy12568.groovy | 113 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index 46b4c8be92f..4382d4638d7 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -649,11 +649,12 @@ private int mopArrayIndex(final MetaMethod method, final String mopName) { * reflectively it is the method a super call should reach. */ private MetaMethod superMethodOfAnotherRuntimePackage(final MetaMethod method, final Class c) { - if (isDGM(method) || c != method.getDeclaringClass().getTheClass()) return null; + if (c != method.getDeclaringClass().getTheClass()) return null; for (Class sc = c.getSuperclass(); sc != null; sc = sc.getSuperclass()) { for (CachedMethod cm : ReflectionCache.getCachedClass(sc).getMethods()) { if (cm.getName().equals(method.getName()) && !cm.isStatic() && MetaMethod.equal(cm.getParameterTypes(), method.getParameterTypes())) { + // the nearest method decides; if it is not of another run-time package, it hides any deeper one return cm.isPackagePrivate() && sc.getClassLoader() != c.getClassLoader() && sc.getPackageName().equals(c.getPackageName()) ? cm : null; } diff --git a/src/test/groovy/bugs/Groovy12568.groovy b/src/test/groovy/bugs/Groovy12568.groovy index 3b625302300..ef87901d1b2 100644 --- a/src/test/groovy/bugs/Groovy12568.groovy +++ b/src/test/groovy/bugs/Groovy12568.groovy @@ -19,6 +19,7 @@ package bugs import org.codehaus.groovy.control.CompilerConfiguration +import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource @@ -139,4 +140,116 @@ final class Groovy12568 { new B().test() + new C().test() ''') == ['M', 'B>M', 'M', 'C>M'] } + + // GROOVY-12570 + @ParameterizedTest(name = 'indy={0}') + @ValueSource(booleans = [true, false]) + void testSuperCallReachesNearestSuperclassMethodOfSameNameAndParameters(final boolean indy) { + def shell = shell(indy) + shell.evaluate '''package p + class A { + @groovy.transform.PackageScope String pm() { 'A' } + } + true + ''' + shell.evaluate '''package p + class MidOfOtherLoader extends A { // its pm() does not override A's + @groovy.transform.PackageScope String pm() { 'MidOfOtherLoader' } + } + class PublicMid extends A { + String pm() { 'PublicMid' } + } + class MidWithoutPm extends A { + } + true + ''' + def mids = ['MidOfOtherLoader', 'PublicMid', 'MidWithoutPm', 'MidOfSameLoader'] + List classes = shell.evaluate('''package p + class MidOfSameLoader extends A { + @groovy.transform.PackageScope String pm() { 'MidOfSameLoader' } + } + ''' + mids.collect { mid -> """ + class D$mid extends $mid { + String pm() { 'D>' + super.pm() } + def test() { [super.pm(), pm()] } + } + @groovy.transform.CompileStatic + class S$mid extends $mid { + String pm() { 'S>' + super.pm() } + def test() { [super.pm(), pm()] } + } + """ }.join('') + ''' + [DMidOfOtherLoader, SMidOfOtherLoader, DPublicMid, SPublicMid, DMidWithoutPm, SMidWithoutPm, DMidOfSameLoader, SMidOfSameLoader] + ''') + // the nearest method of the superclasses decides, so a deeper A.pm() is not reached + def nearest = { Class c -> c.superclass.simpleName == 'MidWithoutPm' ? 'A' : c.superclass.simpleName } + assert classes.collect { it.getDeclaredConstructor().newInstance().test() } == classes.collect { + [nearest(it), it.simpleName[0] + '>' + nearest(it)] + } + // the super method of the meta-class: a super$ method of D, if any, or the method of the superclass; + // S calls PublicMid.pm() and MidOfSameLoader.pm() directly, so it has no super$ method for them, and + // the nearest method hides A.pm() all the same + assert classes.collect { c -> + def metaClass = (MetaClassImpl) GroovySystem.metaClassRegistry.getMetaClass(c) + metaClass.getMethodWithoutCaching(c, 'pm', [] as Class[], true)?.invoke(c.getDeclaredConstructor().newInstance(), [] as Object[]) + } == classes.collect { c -> + c.simpleName in ['SPublicMid', 'SMidOfSameLoader'] ? null : nearest(c) + } + } + + // GROOVY-12570 + @ParameterizedTest(name = 'indy={0}') + @ValueSource(booleans = [true, false]) + void testSuperCallOfPackagePrivateMethodOfClassOfAnotherPackage(final boolean indy) { + def shell = shell(indy) + shell.evaluate '''package q + class M { + @groovy.transform.PackageScope String pm() { 'M' } + } + true + ''' + def err = shouldFail(MissingMethodException) { + shell.evaluate '''package p + class B extends q.M { // another package: M.pm() is not accessible + String pm() { 'B' } + def test() { + super.pm() + } + } + new B().test() + ''' + } + assert err.method == 'pm' + } + + // GROOVY-12570 + @Test + void testSuperMethodsOfMetaClassForPackagePrivateMethodsOfClassOfAnotherClassLoader() { + def shell = shell(true) + shell.evaluate '''package p + class M { + @groovy.transform.PackageScope String pm() { 'M' } + @groovy.transform.PackageScope String pm(String s) { 'M' + s } + @groovy.transform.PackageScope String qm() { 'M' } + } + true + ''' + Class b = shell.evaluate '''package p + class B extends M { + String pm() { 'B' } + String pm(String s) { 'B' + s } + String qm() { 'B' } + } + B + ''' + def metaClass = (MetaClassImpl) GroovySystem.metaClassRegistry.getMetaClass(b) + def object = b.getDeclaredConstructor().newInstance() + // the method of M, not B: for the overloads of pm and for qm, which has none + [pm: [], pm1: [String], qm: []].each { name, types -> + MetaMethod method = metaClass.getMethodWithoutCaching(b, name - '1', types as Class[], true) + assert method.declaringClass.theClass == b.superclass + assert method.invoke(object, types ? ['x'] as Object[] : [] as Object[]) == (types ? 'Mx' : 'M') + assert metaClass.getMethodWithoutCaching(b, name - '1', types as Class[], false).declaringClass.theClass == b + } + } } From e9c86b5dfcd1263388525eff11f9a24d1aa8960c Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Fri, 30 Oct 2026 17:41:17 -0700 Subject: [PATCH 18/18] GROOVY-12568: state the JVM rule that isFieldDirectlyAccessible adds to the language rule For a public or private field the language rule of isMemberDirectlyAccessible decides. For a protected or package-private field, JVMS 5.4.4 allows access from the run-time package of the declaring class, or to a protected field from a subclass. That condition implies isMemberDirectlyAccessible, so it replaces it there instead of being checked after it. --- .../groovy/classgen/AsmClassGenerator.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index 7f50ee2b9b9..e0cacd1550e 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -1308,15 +1308,20 @@ private static String getStaticFieldName(final ClassNode type) { * Determines if the given field can be directly accessed by the given class * (via {@code GETFIELD}, {@code GETSTATIC}, etc. bytecode instructions). *

- * A protected or package-private field of a class with the same package - * name is accessible only within the same run-time package; see - * {@link #isInSameRuntimePackage}. Otherwise it is directly accessible - * only through the subclass rule for a protected field. + * Unlike {@link #isMemberDirectlyAccessible}, the package name is not + * enough for a protected or package-private field; it needs the same + * run-time package (see {@link #isInSameRuntimePackage}). */ public static boolean isFieldDirectlyAccessible(final FieldNode field, final ClassNode accessingClass) { - if (field == null || !isMemberDirectlyAccessible(field.getModifiers(), field.getDeclaringClass(), accessingClass)) return false; + if (field == null) return false; ClassNode declaringClass = field.getDeclaringClass(); - return field.isPublic() || field.isPrivate() || isInSameRuntimePackage(declaringClass, accessingClass) + if (field.isPublic() || field.isPrivate()) { // nestmates can access a private field + return isMemberDirectlyAccessible(field.getModifiers(), declaringClass, accessingClass); + } + // JVMS 5.4.4: a protected or package-private field is accessible from the run-time package of its declaring + // class, which includes the declaring class, and a protected field also from a subclass; access from a class + // of the same package name but another run-time package fails with IllegalAccessError + return isInSameRuntimePackage(declaringClass, accessingClass) || (field.isProtected() && accessingClass.isDerivedFrom(declaringClass)); }