[
https://issues.apache.org/jira/browse/GROOVY-12159?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12159:
-------------------------------
Description:
TLDR this is a reproducibility (bytecode ordering) issue not a correctness
issue.
h2. Description
{{AsmClassGenerator}} collects the class literals that cannot be emitted as an
{{ldc}} into an unordered map, and iterates it to generate the synthetic helper
members:
{code:java}
// AsmClassGenerator.java:284
private final Map<String,ClassNode> referencedClasses = new HashMap<>();
// AsmClassGenerator.java:1857 (visitClassExpression)
String staticFieldName = getStaticFieldName(type);
referencedClasses.put(staticFieldName, type);
// AsmClassGenerator.java:1774 (createSyntheticStaticFields)
for (Map.Entry<String, ClassNode> entry : referencedClasses.entrySet()) {
... // <-- emits the $class$... field and its $get$$class$... accessor
}
{code}
The keys are the synthetic field names, so *the emitted {{$class$...}} fields
and their {{$get$$class$...}} accessors appear in the hash order of those
names* rather than in the order the class literals were encountered. Both
iteration sites are affected: {{createSyntheticStaticFields}} ({{:1774}}) for
classes and {{createInterfaceSyntheticStaticFields}} ({{:1758}}) for interfaces.
h3. Reachability
This path is narrow. {{visitClassExpression}} emits an {{ldc}} — and never
touches the map — unless {{BytecodeHelper.isClassLiteralPossible(type)}} is
false and the type is not in the same compilation unit. That predicate only
checks for {{public}}:
{code:java}
// BytecodeHelper.java:660
public static boolean isClassLiteralPossible(ClassNode classNode) {
return Modifier.isPublic(classNode.getModifiers());
}
{code}
So the map is populated only when a class literal names a *non-public type from
another compilation unit*. Ordinary code never reaches it. This is the least
impactful of the related ordering issues below, and worth fixing mainly for
consistency and because the fix is a single word.
h3. Demonstration
Compilation unit 1:
{code:java}
package p
import groovy.transform.PackageScope
@PackageScope class Ha {}
@PackageScope class Hb {}
@PackageScope class Hc {}
@PackageScope class Hd {}
@PackageScope class He {}
{code}
Compilation unit 2, compiled against the first:
{code:java}
class User {
def f() { [p.Ha, p.Hb, p.Hc, p.Hd, p.He] }
}
{code}
Reading the emitted {{User.class}} with an ASM {{ClassReader}}, the synthetic
members appear as:
{noformat}
fields : $class$p$Ha, $class$p$Hc, $class$p$Hb, $class$p$He, $class$p$Hd
accessors: $get$$class$p$Ha, $get$$class$p$Hc, $get$$class$p$Hb,
$get$$class$p$He, $get$$class$p$Hd
{noformat}
rather than the encounter order {{Ha, Hb, Hc, Hd, He}}.
h3. Impact
The keys are {{String}}s and {{String.hashCode}} is stable, so for a given key
set the iteration order is reproducible across JVM runs — this is not by itself
a live reproducible-builds failure. It still matters because the emitted order
is arbitrary rather than meaningful, and because within a colliding bucket
{{HashMap}} chain order follows insertion order, which is the order the
literals are visited.
h3. Proposed fix
Use an insertion-ordered map:
{code:java}
private final Map<String,ClassNode> referencedClasses = new LinkedHashMap<>();
{code}
The synthetic fields and accessors are then emitted in the order the class
literals were encountered. The member *set* is unchanged — only the order. The
map is already {{clear()}}ed per class in {{visitClass}} ({{:355}}), so
ordering remains scoped to each class.
h3. Test
{{src/test/groovy/org/codehaus/groovy/classgen/SyntheticClassLiteralFieldOrderTest.groovy}}
compiles the {{@PackageScope}} types into a temp directory as one compilation
unit, compiles the referencing class as a second unit with that directory on
the classpath, then reads the emitted bytes with an ASM {{ClassReader}} and
asserts both the {{$class$...}} fields and the {{$get$$class$...}} accessors
appear in encounter order. It fails on the current code with {{['Ha', 'Hc',
'Hb', 'He', 'Hd']}} and passes with the fix.
h3. Related
GROOVY-12149 (nondeterministic reflection order flows into generated bytecode).
Same class of defect: {{Verifier.addCovariantMethods}} (covariant bridge
methods emitted in hash order),
{{AutoImplementASTTransformation.getAllCorrectedMethodsMap}}
({{@AutoImplement}} methods emitted in hash order),
{{AnnotationCollectorTransform.makeListOfAnnotations}} (annotation member order
for precompiled collectors), and GROOVY-12156 ({{chooseBestMethod}} candidates
in identity-hash order).
was:
h2. Description
{{AsmClassGenerator}} collects the class literals that cannot be emitted as an
{{ldc}} into an unordered map, and iterates it to generate the synthetic helper
members:
{code:java}
// AsmClassGenerator.java:284
private final Map<String,ClassNode> referencedClasses = new HashMap<>();
// AsmClassGenerator.java:1857 (visitClassExpression)
String staticFieldName = getStaticFieldName(type);
referencedClasses.put(staticFieldName, type);
// AsmClassGenerator.java:1774 (createSyntheticStaticFields)
for (Map.Entry<String, ClassNode> entry : referencedClasses.entrySet()) {
... // <-- emits the $class$... field and its $get$$class$... accessor
}
{code}
The keys are the synthetic field names, so *the emitted {{$class$...}} fields
and their {{$get$$class$...}} accessors appear in the hash order of those
names* rather than in the order the class literals were encountered. Both
iteration sites are affected: {{createSyntheticStaticFields}} ({{:1774}}) for
classes and {{createInterfaceSyntheticStaticFields}} ({{:1758}}) for interfaces.
h3. Reachability
This path is narrow. {{visitClassExpression}} emits an {{ldc}} — and never
touches the map — unless {{BytecodeHelper.isClassLiteralPossible(type)}} is
false and the type is not in the same compilation unit. That predicate only
checks for {{public}}:
{code:java}
// BytecodeHelper.java:660
public static boolean isClassLiteralPossible(ClassNode classNode) {
return Modifier.isPublic(classNode.getModifiers());
}
{code}
So the map is populated only when a class literal names a *non-public type from
another compilation unit*. Ordinary code never reaches it. This is the least
impactful of the related ordering issues below, and worth fixing mainly for
consistency and because the fix is a single word.
h3. Demonstration
Compilation unit 1:
{code:java}
package p
import groovy.transform.PackageScope
@PackageScope class Ha {}
@PackageScope class Hb {}
@PackageScope class Hc {}
@PackageScope class Hd {}
@PackageScope class He {}
{code}
Compilation unit 2, compiled against the first:
{code:java}
class User {
def f() { [p.Ha, p.Hb, p.Hc, p.Hd, p.He] }
}
{code}
Reading the emitted {{User.class}} with an ASM {{ClassReader}}, the synthetic
members appear as:
{noformat}
fields : $class$p$Ha, $class$p$Hc, $class$p$Hb, $class$p$He, $class$p$Hd
accessors: $get$$class$p$Ha, $get$$class$p$Hc, $get$$class$p$Hb,
$get$$class$p$He, $get$$class$p$Hd
{noformat}
rather than the encounter order {{Ha, Hb, Hc, Hd, He}}.
h3. Impact
The keys are {{String}}s and {{String.hashCode}} is stable, so for a given key
set the iteration order is reproducible across JVM runs — this is not by itself
a live reproducible-builds failure. It still matters because the emitted order
is arbitrary rather than meaningful, and because within a colliding bucket
{{HashMap}} chain order follows insertion order, which is the order the
literals are visited.
h3. Proposed fix
Use an insertion-ordered map:
{code:java}
private final Map<String,ClassNode> referencedClasses = new LinkedHashMap<>();
{code}
The synthetic fields and accessors are then emitted in the order the class
literals were encountered. The member *set* is unchanged — only the order. The
map is already {{clear()}}ed per class in {{visitClass}} ({{:355}}), so
ordering remains scoped to each class.
h3. Test
{{src/test/groovy/org/codehaus/groovy/classgen/SyntheticClassLiteralFieldOrderTest.groovy}}
compiles the {{@PackageScope}} types into a temp directory as one compilation
unit, compiles the referencing class as a second unit with that directory on
the classpath, then reads the emitted bytes with an ASM {{ClassReader}} and
asserts both the {{$class$...}} fields and the {{$get$$class$...}} accessors
appear in encounter order. It fails on the current code with {{['Ha', 'Hc',
'Hb', 'He', 'Hd']}} and passes with the fix.
h3. Related
GROOVY-12149 (nondeterministic reflection order flows into generated bytecode).
Same class of defect: {{Verifier.addCovariantMethods}} (covariant bridge
methods emitted in hash order),
{{AutoImplementASTTransformation.getAllCorrectedMethodsMap}}
({{@AutoImplement}} methods emitted in hash order),
{{AnnotationCollectorTransform.makeListOfAnnotations}} (annotation member order
for precompiled collectors), and GROOVY-12156 ({{chooseBestMethod}} candidates
in identity-hash order).
> Synthetic class-literal fields and accessors are emitted in hash order, not
> the order the literals were encountered
> -------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12159
> URL: https://issues.apache.org/jira/browse/GROOVY-12159
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
> Fix For: 5.0.8, 6.0.0-beta-1
>
>
> TLDR this is a reproducibility (bytecode ordering) issue not a correctness
> issue.
> h2. Description
> {{AsmClassGenerator}} collects the class literals that cannot be emitted as
> an {{ldc}} into an unordered map, and iterates it to generate the synthetic
> helper members:
> {code:java}
> // AsmClassGenerator.java:284
> private final Map<String,ClassNode> referencedClasses = new HashMap<>();
> // AsmClassGenerator.java:1857 (visitClassExpression)
> String staticFieldName = getStaticFieldName(type);
> referencedClasses.put(staticFieldName, type);
> // AsmClassGenerator.java:1774 (createSyntheticStaticFields)
> for (Map.Entry<String, ClassNode> entry : referencedClasses.entrySet()) {
> ... // <-- emits the $class$... field and its $get$$class$... accessor
> }
> {code}
> The keys are the synthetic field names, so *the emitted {{$class$...}} fields
> and their {{$get$$class$...}} accessors appear in the hash order of those
> names* rather than in the order the class literals were encountered. Both
> iteration sites are affected: {{createSyntheticStaticFields}} ({{:1774}}) for
> classes and {{createInterfaceSyntheticStaticFields}} ({{:1758}}) for
> interfaces.
> h3. Reachability
> This path is narrow. {{visitClassExpression}} emits an {{ldc}} — and never
> touches the map — unless {{BytecodeHelper.isClassLiteralPossible(type)}} is
> false and the type is not in the same compilation unit. That predicate only
> checks for {{public}}:
> {code:java}
> // BytecodeHelper.java:660
> public static boolean isClassLiteralPossible(ClassNode classNode) {
> return Modifier.isPublic(classNode.getModifiers());
> }
> {code}
> So the map is populated only when a class literal names a *non-public type
> from another compilation unit*. Ordinary code never reaches it. This is the
> least impactful of the related ordering issues below, and worth fixing mainly
> for consistency and because the fix is a single word.
> h3. Demonstration
> Compilation unit 1:
> {code:java}
> package p
> import groovy.transform.PackageScope
> @PackageScope class Ha {}
> @PackageScope class Hb {}
> @PackageScope class Hc {}
> @PackageScope class Hd {}
> @PackageScope class He {}
> {code}
> Compilation unit 2, compiled against the first:
> {code:java}
> class User {
> def f() { [p.Ha, p.Hb, p.Hc, p.Hd, p.He] }
> }
> {code}
> Reading the emitted {{User.class}} with an ASM {{ClassReader}}, the synthetic
> members appear as:
> {noformat}
> fields : $class$p$Ha, $class$p$Hc, $class$p$Hb, $class$p$He, $class$p$Hd
> accessors: $get$$class$p$Ha, $get$$class$p$Hc, $get$$class$p$Hb,
> $get$$class$p$He, $get$$class$p$Hd
> {noformat}
> rather than the encounter order {{Ha, Hb, Hc, Hd, He}}.
> h3. Impact
> The keys are {{String}}s and {{String.hashCode}} is stable, so for a given
> key set the iteration order is reproducible across JVM runs — this is not by
> itself a live reproducible-builds failure. It still matters because the
> emitted order is arbitrary rather than meaningful, and because within a
> colliding bucket {{HashMap}} chain order follows insertion order, which is
> the order the literals are visited.
> h3. Proposed fix
> Use an insertion-ordered map:
> {code:java}
> private final Map<String,ClassNode> referencedClasses = new LinkedHashMap<>();
> {code}
> The synthetic fields and accessors are then emitted in the order the class
> literals were encountered. The member *set* is unchanged — only the order.
> The map is already {{clear()}}ed per class in {{visitClass}} ({{:355}}), so
> ordering remains scoped to each class.
> h3. Test
> {{src/test/groovy/org/codehaus/groovy/classgen/SyntheticClassLiteralFieldOrderTest.groovy}}
> compiles the {{@PackageScope}} types into a temp directory as one
> compilation unit, compiles the referencing class as a second unit with that
> directory on the classpath, then reads the emitted bytes with an ASM
> {{ClassReader}} and asserts both the {{$class$...}} fields and the
> {{$get$$class$...}} accessors appear in encounter order. It fails on the
> current code with {{['Ha', 'Hc', 'Hb', 'He', 'Hd']}} and passes with the fix.
> h3. Related
> GROOVY-12149 (nondeterministic reflection order flows into generated
> bytecode). Same class of defect: {{Verifier.addCovariantMethods}} (covariant
> bridge methods emitted in hash order),
> {{AutoImplementASTTransformation.getAllCorrectedMethodsMap}}
> ({{@AutoImplement}} methods emitted in hash order),
> {{AnnotationCollectorTransform.makeListOfAnnotations}} (annotation member
> order for precompiled collectors), and GROOVY-12156 ({{chooseBestMethod}}
> candidates in identity-hash order).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
