Skip to content

Commit 7b11bd1

Browse files
liachliach
committed
8372047: ClassTransform.transformingMethodBodies andThen composes incorrectly
Reviewed-by: asotona
1 parent c419dda commit 7b11bd1

File tree

2 files changed

+67
-25
lines changed

2 files changed

+67
-25
lines changed

‎src/java.base/share/classes/jdk/internal/classfile/impl/TransformImpl.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -120,9 +120,9 @@ public ResolvedTransform<ClassElement> resolve(ClassBuilder builder) {
120120

121121
@Override
122122
public ClassTransform andThen(ClassTransform next) {
123-
if (next instanceof ClassMethodTransform cmt)
124-
return new ClassMethodTransform(transform.andThen(cmt.transform),
125-
mm -> filter.test(mm) && cmt.filter.test(mm));
123+
// Optimized for shared _ -> true filter in ClassTransform.transformingMethods(MethodTransform)
124+
if (next instanceof ClassMethodTransform(var nextTransform, var nextFilter) && filter == nextFilter)
125+
return new ClassMethodTransform(transform.andThen(nextTransform), filter);
126126
else
127127
return UnresolvedClassTransform.super.andThen(next);
128128
}
@@ -143,9 +143,9 @@ public ResolvedTransform<ClassElement> resolve(ClassBuilder builder) {
143143

144144
@Override
145145
public ClassTransform andThen(ClassTransform next) {
146-
if (next instanceof ClassFieldTransform cft)
147-
return new ClassFieldTransform(transform.andThen(cft.transform),
148-
mm -> filter.test(mm) && cft.filter.test(mm));
146+
// Optimized for shared _ -> true filter in ClassTransform.transformingFields(FieldTransform)
147+
if (next instanceof ClassFieldTransform(var nextTransform, var nextFilter) && filter == nextFilter)
148+
return new ClassFieldTransform(transform.andThen(nextTransform), filter);
149149
else
150150
return UnresolvedClassTransform.super.andThen(next);
151151
}
@@ -208,8 +208,8 @@ public ResolvedTransform<MethodElement> resolve(MethodBuilder builder) {
208208

209209
@Override
210210
public MethodTransform andThen(MethodTransform next) {
211-
return (next instanceof TransformImpl.MethodCodeTransform mct)
212-
? new TransformImpl.MethodCodeTransform(xform.andThen(mct.xform))
211+
return (next instanceof MethodCodeTransform(var nextXform))
212+
? new TransformImpl.MethodCodeTransform(xform.andThen(nextXform))
213213
: UnresolvedMethodTransform.super.andThen(next);
214214

215215
}

‎test/jdk/jdk/classfile/TransformTests.java‎

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -23,25 +23,15 @@
2323

2424
/*
2525
* @test
26-
* @bug 8335935 8336588
26+
* @bug 8335935 8336588 8372047
2727
* @summary Testing ClassFile transformations.
2828
* @run junit TransformTests
2929
*/
3030

31-
import java.lang.classfile.ClassBuilder;
32-
import java.lang.classfile.ClassElement;
33-
import java.lang.classfile.ClassFile;
34-
import java.lang.classfile.ClassModel;
35-
import java.lang.classfile.ClassTransform;
36-
import java.lang.classfile.CodeBuilder;
37-
import java.lang.classfile.CodeElement;
38-
import java.lang.classfile.CodeModel;
39-
import java.lang.classfile.CodeTransform;
40-
import java.lang.classfile.FieldModel;
41-
import java.lang.classfile.FieldTransform;
42-
import java.lang.classfile.Label;
43-
import java.lang.classfile.MethodModel;
44-
import java.lang.classfile.MethodTransform;
31+
import java.lang.classfile.*;
32+
import java.lang.classfile.attribute.AnnotationDefaultAttribute;
33+
import java.lang.classfile.attribute.ConstantValueAttribute;
34+
import java.lang.classfile.attribute.SourceDebugExtensionAttribute;
4535
import java.lang.classfile.instruction.BranchInstruction;
4636
import java.lang.classfile.instruction.ConstantInstruction;
4737
import java.lang.classfile.instruction.LabelTarget;
@@ -54,8 +44,10 @@
5444
import java.nio.file.Paths;
5545
import java.util.HashSet;
5646
import java.util.Set;
47+
import java.util.concurrent.atomic.AtomicBoolean;
5748

5849
import helpers.ByteArrayClassLoader;
50+
import jdk.internal.classfile.impl.TransformImpl;
5951
import org.junit.jupiter.api.Test;
6052

6153
import static java.lang.classfile.ClassFile.*;
@@ -334,4 +326,54 @@ static public String foo() {
334326
return "foo";
335327
}
336328
}
329+
330+
@Test
331+
void testFilteringTransformChaining() {
332+
var cf = ClassFile.of();
333+
var clazz = cf.parse(cf.build(ClassDesc.of("Test"), clb -> clb
334+
.withField("one", CD_int, fb -> fb.with(ConstantValueAttribute.of(1)))
335+
.withField("two", CD_int, fb -> fb.with(ConstantValueAttribute.of(2)))
336+
.withMethod("one", MTD_void, 0, mb -> mb.with(AnnotationDefaultAttribute.of(AnnotationValue.ofInt(1))).withCode(CodeBuilder::return_))
337+
.withMethod("two", MTD_void, 0, mb -> mb.with(AnnotationDefaultAttribute.of(AnnotationValue.ofInt(2))).withCode(CodeBuilder::return_))));
338+
339+
AtomicBoolean oneFieldCalled = new AtomicBoolean(false);
340+
var oneFieldTransform = new TransformImpl.ClassFieldTransform((fb, fe) -> {
341+
if (fe instanceof ConstantValueAttribute cv) {
342+
assertEquals(1, ((Integer) cv.constant().constantValue()), "Should only transform one");
343+
}
344+
oneFieldCalled.set(true);
345+
fb.with(fe);
346+
}, fm -> fm.fieldName().equalsString("one"));
347+
AtomicBoolean twoFieldCalled = new AtomicBoolean(false);
348+
var twoFieldTransform = new TransformImpl.ClassFieldTransform((fb, fe) -> {
349+
if (fe instanceof ConstantValueAttribute cv) {
350+
assertEquals(2, ((Integer) cv.constant().constantValue()), "Should only transform two");
351+
}
352+
twoFieldCalled.set(true);
353+
fb.with(fe);
354+
}, fm -> fm.fieldName().equalsString("two"));
355+
cf.transformClass(clazz, oneFieldTransform.andThen(twoFieldTransform));
356+
assertTrue(oneFieldCalled.get(), "Field one not transformed");
357+
assertTrue(twoFieldCalled.get(), "Field two not transformed");
358+
359+
AtomicBoolean oneMethodCalled = new AtomicBoolean(false);
360+
var oneMethodTransform = ClassTransform.transformingMethods(mm -> mm.methodName().equalsString("one"), (mb, me) -> {
361+
if (me instanceof AnnotationDefaultAttribute ada) {
362+
assertEquals(1, ((AnnotationValue.OfInt) ada.defaultValue()).intValue(), "Should only transform one");
363+
}
364+
oneMethodCalled.set(true);
365+
mb.with(me);
366+
});
367+
AtomicBoolean twoMethodCalled = new AtomicBoolean(false);
368+
var twoMethodTransform = ClassTransform.transformingMethods(mm -> mm.methodName().equalsString("two"), (mb, me) -> {
369+
if (me instanceof AnnotationDefaultAttribute ada) {
370+
assertEquals(2, ((AnnotationValue.OfInt) ada.defaultValue()).intValue(), "Should only transform two");
371+
}
372+
twoMethodCalled.set(true);
373+
mb.with(me);
374+
});
375+
cf.transformClass(clazz, oneMethodTransform.andThen(twoMethodTransform));
376+
assertTrue(oneMethodCalled.get(), "Method one not transformed");
377+
assertTrue(twoMethodCalled.get(), "Method two not transformed");
378+
}
337379
}

0 commit comments

Comments
 (0)