[
https://issues.apache.org/jira/browse/GROOVY-11968?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18076457#comment-18076457
]
ASF GitHub Bot commented on GROOVY-11968:
-----------------------------------------
Copilot commented on code in PR #2495:
URL: https://github.com/apache/groovy/pull/2495#discussion_r3145300018
##########
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)
Review Comment:
`runWithIndy` is declared `static`, but it uses `this.class.classLoader`.
Using `this` in a static context is a compile-time error in Groovy/Java. Use
`Groovy11968.class.classLoader` (or make `runWithIndy` an instance method) when
constructing the `GroovyShell`.
```suggestion
new GroovyShell(Groovy11968.class.classLoader, new Binding(),
config).evaluate(SCRIPT)
```
> SC: trait static field access generates invalid bytecode under indy=false
> (GROOVY-11907 follow-up)
> --------------------------------------------------------------------------------------------------
>
> Key: GROOVY-11968
> URL: https://issues.apache.org/jira/browse/GROOVY-11968
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> GROOVY-11817 introduced per-expression delegation between
> StaticTypesCallSiteWriter
> and the regular CallSiteWriter: a sub-expression marked DYNAMIC_RESOLUTION
> inside a
> @CompileStatic method is routed through the regular writer while the rest of
> the
> method stays on the static path. However,
> StaticTypesCallSiteWriter.makeSiteEntry()
> is a no-op, so the regular writer's per-method state (callSiteArrayVarIndex,
> the
> $getCallSiteArray() prologue, and the cached CallSite[] local slot) is never
> initialized. With invokedynamic this is harmless, since indy doesn't use the
> cached array. With indy=false, the regular writer's prepareCallSite() emits
> ALOAD against an unallocated slot, producing methods whose first instruction
> references a local beyond max_locals.
> This surfaces as VerifyError at class load. The most accessible reproducer is
> a
> @CompileStatic trait with a static field, compiled with
> groovy.target.indy=false:
> {code:groovy}
> @CompileStatic
> trait T {
> static double d = 1.0d
> static double bump() { d = d + 1.0d; d }
> }
> {code}
> T$Trait$Helper.bump and the static-field accessors are emitted with
> max_locals=1
> and a body that begins with dload_3, failing the verifier with
> "get long/double overflows locals". The same shape is broken for any
> wide-typed
> static trait field (long/double), and more generally for any
> DYNAMIC_RESOLUTION
> sub-expression in a statically compiled method when indy is disabled. Groovy 4
> is unaffected because pre-GROOVY-11817 the whole method ran on the regular
> writer and makeSiteEntry() always executed.
> The restructure for the original GROOVY-11907 (commit 19f38997a4) addresses
> the receiver-expression typing for the indy=true / global-AST-transform case,
> but does not address this handoff and is complementary to it.
> Proposed fix: ensure the regular CallSiteWriter is initialized for the current
> method whenever the method body contains any DYNAMIC_RESOLUTION-marked
> expressions. Concretely, in StaticTypesWriterController.setMethodNode (or
> StaticTypesCallSiteWriter.makeSiteEntry), detect the presence of
> DYNAMIC_RESOLUTION markers and call super.makeSiteEntry() so that the
> $getCallSiteArray() prologue is emitted and the cached-array local slot is
> allocated up front. No change to the trait transform, no change to opcode
> emission elsewhere; statically compiled methods with no DYNAMIC_RESOLUTION
> sub-expressions are unaffected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
