From e98ad40f6dbc1ebf52ba8e3ff6f6b259df157c87 Mon Sep 17 00:00:00 2001 From: Paul King Date: Mon, 27 Apr 2026 16:28:25 +1000 Subject: [PATCH] GROOVY-11968: SC: trait static field access generates invalid bytecode under indy=false (GROOVY-11907 follow-up) --- .../asm/sc/StaticTypesCallSiteWriter.java | 9 ++ .../asm/sc/StaticTypesWriterController.java | 69 +++++++++++++++ .../transform/traitx/Groovy11968.groovy | 83 +++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 src/test/groovy/org/codehaus/groovy/transform/traitx/Groovy11968.groovy 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 1eea09ad4bc..1f37b8def23 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 @@ -507,6 +507,15 @@ boolean makeGetField(final Expression receiver, final ClassNode receiverType, fi @Override public void makeSiteEntry() { + // GROOVY-11968: when the statically compiled method body contains any + // sub-expression marked DYNAMIC_RESOLUTION, codegen will delegate that + // sub-expression to the regular CallSiteWriter via getCallSiteWriterFor. + // The regular writer's per-method state (callSiteArrayVarIndex and the + // $getCallSiteArray() prologue) must be set up at method entry; otherwise + // its prepareCallSite() emits ALOAD against an unallocated local slot. + if (controller.methodHasDynamicResolution()) { + controller.getRegularCallSiteWriter().makeSiteEntry(); + } } @Override diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java index 78ceed91967..6a08d53cfd6 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java @@ -19,9 +19,15 @@ package org.codehaus.groovy.classgen.asm.sc; import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.CodeVisitorSupport; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.AttributeExpression; import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.MethodCallExpression; +import org.codehaus.groovy.ast.expr.PropertyExpression; +import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.classgen.GeneratorContext; import org.codehaus.groovy.classgen.asm.BinaryExpressionHelper; @@ -51,6 +57,7 @@ public class StaticTypesWriterController extends DelegatingController { protected boolean isInStaticallyCheckedMethod; + private boolean methodHasDynamicResolution; // GROOVY-11968 private LambdaWriter lambdaWriter; private ClosureWriter closureWriter; @@ -87,15 +94,77 @@ public void init(final AsmClassGenerator asmClassGenerator, final GeneratorConte @Override public void setMethodNode(final MethodNode mn) { isInStaticallyCheckedMethod = isStaticallyCompiled(mn); + methodHasDynamicResolution = isInStaticallyCheckedMethod && hasDynamicResolution(mn); super.setMethodNode(mn); } @Override public void setConstructorNode(final ConstructorNode cn) { isInStaticallyCheckedMethod = isStaticallyCompiled(cn); + methodHasDynamicResolution = isInStaticallyCheckedMethod && hasDynamicResolution(cn); super.setConstructorNode(cn); } + /** + * GROOVY-11968: returns {@code true} when the current statically compiled method + * contains one or more sub-expressions that will be routed through the regular + * (non-static) call site writer via {@link #getCallSiteWriterFor}. The regular + * writer's per-method state must then be initialized at method entry. + */ + public boolean methodHasDynamicResolution() { + return methodHasDynamicResolution; + } + + private static boolean hasDynamicResolution(final MethodNode mn) { + if (mn == null) return false; + if (mn.getNodeMetaData(DYNAMIC_RESOLUTION) != null) return true; + if (mn.getCode() == null) return false; + var scanner = new DynamicResolutionScanner(); + mn.getCode().visit(scanner); + return scanner.found; + } + + private static class DynamicResolutionScanner extends CodeVisitorSupport { + boolean found; + + @Override + public void visitMethodCallExpression(final MethodCallExpression call) { + if (isMarked(call)) return; + super.visitMethodCallExpression(call); + } + + @Override + public void visitStaticMethodCallExpression(final StaticMethodCallExpression call) { + if (isMarked(call)) return; + super.visitStaticMethodCallExpression(call); + } + + @Override + public void visitPropertyExpression(final PropertyExpression expression) { + if (isMarked(expression)) return; + super.visitPropertyExpression(expression); + } + + @Override + public void visitAttributeExpression(final AttributeExpression expression) { + if (isMarked(expression)) return; + super.visitAttributeExpression(expression); + } + + @Override + public void visitVariableExpression(final VariableExpression expression) { + if (isMarked(expression)) return; + super.visitVariableExpression(expression); + } + + private boolean isMarked(final Expression e) { + if (!found && e.getNodeMetaData(DYNAMIC_RESOLUTION) != null) { + found = true; + } + return found; + } + } + @Override public boolean isFastPath() { if (isInStaticallyCheckedMethod) return true; diff --git a/src/test/groovy/org/codehaus/groovy/transform/traitx/Groovy11968.groovy b/src/test/groovy/org/codehaus/groovy/transform/traitx/Groovy11968.groovy new file mode 100644 index 00000000000..2841038954f --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/transform/traitx/Groovy11968.groovy @@ -0,0 +1,83 @@ +/* + * 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.transform.traitx + +import org.codehaus.groovy.control.CompilerConfiguration +import org.junit.jupiter.api.Test + +/** + * GROOVY-11968 (follow-up to GROOVY-11817 / GROOVY-11907): a sub-expression marked + * DYNAMIC_RESOLUTION inside a @CompileStatic method delegates mid-method to the + * regular CallSiteWriter via getCallSiteWriterFor. The regular writer's per-method + * state must be initialized at method entry; otherwise its prepareCallSite() + * emits ALOAD against an unallocated local slot, producing methods whose first + * instruction references a local beyond max_locals (VerifyError at class load). + * + * Trait static-field access is the most accessible reproducer: $static$self.T__d$set + * is marked DO_DYNAMIC by TraitReceiverTransformer and translated to DYNAMIC_RESOLUTION + * by TraitTypeCheckingExtension. With invokedynamic the cached call-site array is not + * used and the bug is masked; with indy=false the verifier rejects the helper. + */ +final class Groovy11968 { + + private static final String SCRIPT = ''' + @groovy.transform.CompileStatic + trait T { + static double d = 1.0d + static long l = 1L + static String s = 'hello' + + static double bumpD() { d = d + 1.0d; d } + static long bumpL() { l = l + 1L; l } + static String wrap(String x) { "[$s/$d/$l] $x" } + } + + @groovy.transform.CompileStatic + class C implements T { + String run() { + d = 41.0d + l = 41L + s = 'world' + bumpD() + bumpL() + wrap('ok') + } + } + + assert new C().run() == '[world/42.0/42] ok' + ''' + + @Test + void testTraitStaticFieldHelperLoadsAndRunsUnderIndy() { + runWithIndy(true) + } + + @Test + void testTraitStaticFieldHelperLoadsAndRunsWithoutIndy() { + runWithIndy(false) + } + + private static void runWithIndy(boolean indy) { + def config = new CompilerConfiguration() + config.optimizationOptions[CompilerConfiguration.INVOKEDYNAMIC] = indy + // Loading the helper class is what triggers the verifier; an assertion + // inside the script proves runtime semantics are also correct. + new GroovyShell(this.class.classLoader, new Binding(), config).evaluate(SCRIPT) + } +}