[
https://issues.apache.org/jira/browse/GROOVY-12440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18125074#comment-18125074
]
ASF GitHub Bot commented on GROOVY-12440:
-----------------------------------------
paulk-asert opened a new pull request, #2959:
URL: https://github.com/apache/groovy/pull/2959
… inside a closure corrupts NestMembers causing IllegalAccessError
> closure on the right-hand side of a property assignment inside a closure
> corrupts NestMembers causing IllegalAccessError
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12440
> URL: https://issues.apache.org/jira/browse/GROOVY-12440
> Project: Groovy
> Issue Type: Bug
> Components: class generator, Static compilation
> Affects Versions: 5.0.0
> Reporter: James Daugherty
> Assignee: Paul King
> Priority: Major
>
> *Affects:* every 5.x release tested, 5.0.0 through 5.1.3.
> *Works in:* 4.0.33 (no NestMembers emitted) and 6.0.0.
> *Related:* GROOVY-10687, GROOVY-11780.
> h3. Description
> Under {{{}@CompileStatic{}}}, a closure on the right-hand side of a property
> assignment made inside another closure is recorded twice in the enclosing
> class's {{NestMembers}} attribute. Two examples are shown below. One entry
> names the real class; the other names a class that is never written. Every
> nested closure compiled after it in the same class is then listed under the
> wrong name, one higher than the class actually written.
> {code:groovy}
> { -> holder.value = list.collect { it } }
> holder.with { value = list.collect { it } }
> {code}
> Those closures name the outer class as their {{{}NestHost{}}}, but the outer
> class doesn't list them, so the JVM rejects their direct calls to its private
> methods:
> {noformat}
> java.lang.IllegalAccessError: class Repro$_callPrivate_closure2$_closure4
> tried to access private method
> 'int Repro.secret(java.lang.Integer)' ... (Type
> Repro$_callPrivate_closure2$_closure4 (loader: 'app') is not a
> nest member of type Repro (loader: 'app'): current type is not listed as a
> nest member)
> {noformat}
> The failing closure can be in any later method of the class, and the method
> containing the assignment never has to run. We hit this in a Grails 8
> application (Groovy 5.1.3): a statically compiled service failed at startup
> because an unrelated method assigned a property inside a {{with}} block, with
> a {{collect}} closure on the right-hand side.
> h3. Reproducer
> Standalone, no build tool:
> [jdaugherty/groovy5-issue-nested-closures|https://github.com/jdaugherty/groovy5-issue-nested-closures]
> {code:bash}
> ./reproduce.sh 5.1.3 # fails
> ./reproduce.sh 6.0.0 # passes
> {code}
> {code:groovy}
> import groovy.transform.CompileStatic
> @CompileStatic
> class Repro {
> static class Holder {
> String value
> }
> // Never called. Compiling it is enough to corrupt the NestMembers
> attribute of Repro.
> void assign(Holder holder) {
> Closure assignment = { -> holder.value = ['a'].collect { it
> }.join(',') }
> assignment.call()
> }
> List<Integer> callPrivate() {
> Closure<List<Integer>> outer = { -> [1, 2].collect { Integer i ->
> secret(i) } }
> outer.call()
> }
> private int secret(Integer i) {
> i * 10
> }
> static void main(String[] args) {
> println new Repro().callPrivate()
> }
> }
> {code}
> *Expected:* prints {{{}[10, 20]{}}}, and {{NestMembers}} lists exactly the
> closure classes written.
> *Actual (5.1.3):* the {{IllegalAccessError}} above. Classes written versus
> the {{NestMembers}} attribute:
> {noformat}
> Classes written NestMembers of Repro
> Repro$_assign_closure1 Repro$_assign_closure1
> Repro$_assign_closure1$_closure3 Repro$_assign_closure1$_closure3
> Repro$_assign_closure1$_closure4
> <- never written
> Repro$_callPrivate_closure2 Repro$_callPrivate_closure2
> Repro$_callPrivate_closure2$_closure4
> Repro$_callPrivate_closure2$_closure5 <- written as ...$_closure4
> {noformat}
> h3. Scope
> Tested on 5.1.3, with the same {{callPrivate()}} method following each
> variant:
> {code:groovy}
> // Fails: whether or not the assignment is the closure's last statement.
> // Each assignment adds one phantom entry; two assignments shift later names
> by two.
> { -> holder.value = list.collect { it } }
> holder.with { value = list.collect { it } }
> holder.with { it.value = list.collect { it } }
> // Passes
> { -> holder.setValue(list.collect { it }) } // explicit setter call
> { -> localVariable = list.collect { it } } // local variable
> { -> fieldOfThisClass = list.collect { it } } // field of the enclosing
> class
> holder.value = list.collect { it } // same assignment directly
> in the method
> { -> holder.value = 'x' } // no closure on the
> right-hand side
> {code}
> The failing source compiled without {{@CompileStatic}} passes and produces a
> correct {{NestMembers}} attribute. Reproduced on JDK 21.0.12 and 25.0.4.
> h3. Analysis
> {{AsmClassGenerator.moreNestmateEntries}} doesn't record the closure classes
> that are actually generated. It predicts the names of closures nested in
> closures by walking each closure's {{doCall}} body with a
> {{CodeVisitorSupport}} and incrementing a counter for each
> {{ClosureExpression}} it meets. The prediction holds only if the walk visits
> each {{ClosureExpression}} exactly once.
> For an assignment whose value is kept, static compilation rewrites the
> property assignment into the following setter form:
> {code:groovy}
> def tmp = val; a.setX(tmp); tmp
> {code}
> {{StaticPropertyAccessHelper.transformToSetterCall}} builds that as
> {{{}PoppingListOfExpressionsExpression(tmp, setX(tmp)){}}}, so the same
> {{TemporaryVariableExpression}} appears twice in the tree.
> {{TemporaryVariableExpression.visit}} evaluates the wrapped expression only
> once when the visitor is an {{{}AsmClassGenerator{}}}. For any other visitor
> it delegates straight to the wrapped expression. The nest-member walker
> therefore counts the right-hand-side closure twice, while code generation
> writes it once.
> GROOVY-11780 (backported to 5.0.x) doesn't cover this case.
> {{moreNestmateEntries}} and {{TemporaryVariableExpression}} look unchanged on
> master, so we haven't identified the change that makes 6.0.0 pass. Possible
> fixes:
> * Emit {{NestMembers}} from the closure classes actually generated rather
> than predicted names.
> * Have the walker skip {{ClosureExpression}} nodes it has already counted
> (identity-based).
> h3. Workaround
> Compute the value outside the closure, or call the setter explicitly:
> {code:groovy}
> String joined = ['a'].collect { it }.join(',')
> holder.with { value = joined }
> holder.with { setValue(['a'].collect { it }.join(',')) }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
