8335935: Chained builders not sending transformed models to next transforms

Reviewed-by: asotona
This commit is contained in:
Chen Liang 2024-07-10 21:06:39 +00:00
parent 242f1133f8
commit cad68e06ec
10 changed files with 196 additions and 124 deletions

@ -86,7 +86,7 @@ import java.lang.classfile.instruction.TypeCheckInstruction;
import static java.util.Objects.requireNonNull;
import static jdk.internal.classfile.impl.BytecodeHelpers.handleDescToHandleInfo;
import jdk.internal.classfile.impl.TransformingCodeBuilder;
import jdk.internal.javac.PreviewFeature;
/**
@ -192,7 +192,7 @@ public sealed interface CodeBuilder
default CodeBuilder transforming(CodeTransform transform, Consumer<CodeBuilder> handler) {
var resolved = transform.resolve(this);
resolved.startHandler().run();
handler.accept(new TransformingCodeBuilder(this, resolved.consumer()));
handler.accept(new ChainedCodeBuilder(this, resolved.consumer()));
resolved.endHandler().run();
return this;
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -51,13 +51,13 @@ public final class BlockCodeBuilderImpl
public void start() {
topLocal = topLocal(parent);
terminalMaxLocals = topLocal(terminal);
terminal.with((LabelTarget) startLabel);
terminalMaxLocals = terminal.curTopLocal();
parent.with((LabelTarget) startLabel);
}
public void end() {
terminal.with((LabelTarget) endLabel);
if (terminalMaxLocals != topLocal(terminal)) {
parent.with((LabelTarget) endLabel);
if (terminalMaxLocals != terminal.curTopLocal()) {
throw new IllegalStateException("Interference in local variable slot management");
}
}
@ -73,10 +73,8 @@ public final class BlockCodeBuilderImpl
private int topLocal(CodeBuilder parent) {
return switch (parent) {
case BlockCodeBuilderImpl b -> b.topLocal;
case ChainedCodeBuilder b -> topLocal(b.terminal);
case DirectCodeBuilder b -> b.curTopLocal();
case BufferedCodeBuilder b -> b.curTopLocal();
case TransformingCodeBuilder b -> topLocal(b.delegate);
case ChainedCodeBuilder b -> b.terminal.curTopLocal();
case TerminalCodeBuilder b -> b.curTopLocal();
};
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -43,7 +43,7 @@ import java.util.Optional;
import java.util.function.Consumer;
public final class BufferedCodeBuilder
implements TerminalCodeBuilder, LabelContext {
implements TerminalCodeBuilder {
private final SplitConstantPool constantPool;
private final ClassFileImpl context;
private final List<CodeElement> elements = new ArrayList<>();

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -33,13 +33,11 @@ import java.lang.classfile.constantpool.Utf8Entry;
public final class ChainedClassBuilder
implements ClassBuilder, Consumer<ClassElement> {
private final ClassBuilder downstream;
private final DirectClassBuilder terminal;
private final Consumer<ClassElement> consumer;
public ChainedClassBuilder(ClassBuilder downstream,
Consumer<ClassElement> consumer) {
this.downstream = downstream;
this.consumer = consumer;
this.terminal = switch (downstream) {
case ChainedClassBuilder cb -> cb.terminal;
@ -60,10 +58,11 @@ public final class ChainedClassBuilder
@Override
public ClassBuilder withField(Utf8Entry name, Utf8Entry descriptor, Consumer<? super FieldBuilder> handler) {
return downstream.with(new BufferedFieldBuilder(terminal.constantPool, terminal.context,
consumer.accept(new BufferedFieldBuilder(terminal.constantPool, terminal.context,
name, descriptor, null)
.run(handler)
.toModel());
return this;
}
@Override
@ -72,16 +71,18 @@ public final class ChainedClassBuilder
field.fieldName(), field.fieldType(),
field);
builder.transform(field, transform);
return downstream.with(builder.toModel());
consumer.accept(builder.toModel());
return this;
}
@Override
public ClassBuilder withMethod(Utf8Entry name, Utf8Entry descriptor, int flags,
Consumer<? super MethodBuilder> handler) {
return downstream.with(new BufferedMethodBuilder(terminal.constantPool, terminal.context,
consumer.accept(new BufferedMethodBuilder(terminal.constantPool, terminal.context,
name, descriptor, null)
.run(handler)
.toModel());
return this;
}
@Override
@ -89,7 +90,8 @@ public final class ChainedClassBuilder
BufferedMethodBuilder builder = new BufferedMethodBuilder(terminal.constantPool, terminal.context,
method.methodName(), method.methodType(), method);
builder.transform(method, transform);
return downstream.with(builder.toModel());
consumer.accept(builder.toModel());
return this;
}
@Override

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -36,13 +36,11 @@ import java.lang.classfile.MethodModel;
import java.lang.classfile.constantpool.ConstantPoolBuilder;
public final class ChainedMethodBuilder implements MethodBuilder {
final MethodBuilder downstream;
final TerminalMethodBuilder terminal;
final Consumer<MethodElement> consumer;
public ChainedMethodBuilder(MethodBuilder downstream,
Consumer<MethodElement> consumer) {
this.downstream = downstream;
this.consumer = consumer;
this.terminal = switch (downstream) {
case ChainedMethodBuilder cb -> cb.terminal;
@ -58,16 +56,18 @@ public final class ChainedMethodBuilder implements MethodBuilder {
@Override
public MethodBuilder withCode(Consumer<? super CodeBuilder> handler) {
return downstream.with(terminal.bufferedCodeBuilder(null)
consumer.accept(terminal.bufferedCodeBuilder(null)
.run(handler)
.toModel());
return this;
}
@Override
public MethodBuilder transformCode(CodeModel code, CodeTransform transform) {
BufferedCodeBuilder builder = terminal.bufferedCodeBuilder(code);
builder.transform(code, transform);
return downstream.with(builder.toModel());
consumer.accept(builder.toModel());
return this;
}
@Override

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -75,7 +75,7 @@ import static java.lang.classfile.Opcode.LDC_W;
public final class DirectCodeBuilder
extends AbstractDirectBuilder<CodeModel>
implements TerminalCodeBuilder, LabelContext {
implements TerminalCodeBuilder {
private final List<CharacterRange> characterRanges = new ArrayList<>();
final List<AbstractPseudoInstruction.ExceptionCatchImpl> handlers = new ArrayList<>();
private final List<LocalVariable> localVariables = new ArrayList<>();

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -27,7 +27,7 @@ package jdk.internal.classfile.impl;
import java.lang.classfile.Label;
public sealed interface LabelContext
permits BufferedCodeBuilder, CodeImpl, DirectCodeBuilder {
permits TerminalCodeBuilder, CodeImpl {
Label newLabel();
Label getLabel(int bci);
void setLabelTarget(Label label, int bci);

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -26,7 +26,7 @@ package jdk.internal.classfile.impl;
import java.lang.classfile.CodeBuilder;
public sealed interface TerminalCodeBuilder extends CodeBuilder
permits DirectCodeBuilder, BufferedCodeBuilder, TransformingCodeBuilder {
public sealed interface TerminalCodeBuilder extends CodeBuilder, LabelContext
permits DirectCodeBuilder, BufferedCodeBuilder {
int curTopLocal();
}

@ -1,91 +0,0 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package jdk.internal.classfile.impl;
import java.lang.classfile.CodeBuilder;
import java.lang.classfile.CodeModel;
import java.util.Optional;
import java.util.function.Consumer;
import java.lang.classfile.CodeElement;
import java.lang.classfile.Label;
import java.lang.classfile.TypeKind;
import java.lang.classfile.constantpool.ConstantPoolBuilder;
public final class TransformingCodeBuilder implements TerminalCodeBuilder {
final CodeBuilder delegate;
final Consumer<CodeElement> consumer;
public TransformingCodeBuilder(CodeBuilder delegate, Consumer<CodeElement> consumer) {
this.delegate = delegate;
this.consumer = consumer;
}
@Override
public CodeBuilder with(CodeElement e) {
consumer.accept(e);
return this;
}
@Override
public Optional<CodeModel> original() {
return delegate.original();
}
@Override
public Label newLabel() {
return delegate.newLabel();
}
@Override
public Label startLabel() {
return delegate.startLabel();
}
@Override
public Label endLabel() {
return delegate.endLabel();
}
@Override
public int receiverSlot() {
return delegate.receiverSlot();
}
@Override
public int parameterSlot(int paramNo) {
return delegate.parameterSlot(paramNo);
}
@Override
public int allocateLocal(TypeKind typeKind) {
return delegate.allocateLocal(typeKind);
}
@Override
public ConstantPoolBuilder constantPool() {
return delegate.constantPool();
}
}

@ -23,9 +23,22 @@
/*
* @test
* @bug 8336010
* @summary Testing ClassFile transformations.
* @run junit TransformTests
*/
import java.lang.classfile.ClassBuilder;
import java.lang.classfile.CodeBuilder;
import java.lang.classfile.CodeElement;
import java.lang.classfile.FieldModel;
import java.lang.classfile.FieldTransform;
import java.lang.classfile.Label;
import java.lang.classfile.MethodTransform;
import java.lang.classfile.instruction.BranchInstruction;
import java.lang.classfile.instruction.LabelTarget;
import java.lang.constant.ClassDesc;
import java.lang.constant.MethodTypeDesc;
import java.lang.reflect.AccessFlag;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
@ -39,8 +52,13 @@ import java.lang.classfile.CodeModel;
import java.lang.classfile.CodeTransform;
import java.lang.classfile.MethodModel;
import java.lang.classfile.instruction.ConstantInstruction;
import java.util.HashSet;
import java.util.Set;
import org.junit.jupiter.api.Test;
import static java.lang.classfile.ClassFile.*;
import static java.lang.constant.ConstantDescs.*;
import static org.junit.jupiter.api.Assertions.*;
/**
@ -126,6 +144,151 @@ class TransformTests {
assertEquals(invoke(cc.transformClass(cm, transformCode(foo2foo.andThen(foo2bar).andThen(bar2baz)))), "baz");
}
/**
* Test to ensure class elements, such as field and
* methods defined with transform/with, are visible
* to next transforms.
*/
@Test
void testClassChaining() throws Exception {
var bytes = Files.readAllBytes(testClassPath);
var cf = ClassFile.of();
var cm = cf.parse(bytes);
var otherCm = cf.parse(cf.build(ClassDesc.of("Temp"), clb -> clb
.withMethodBody("baz", MTD_void, ACC_STATIC, CodeBuilder::return_)
.withField("baz", CD_long, ACC_STATIC)));
var methodBaz = otherCm.methods().getFirst();
var fieldBaz = otherCm.fields().getFirst();
ClassTransform transform1 = ClassTransform.endHandler(cb -> {
ClassBuilder ret;
ret = cb.withMethodBody("bar", MTD_void, ACC_STATIC, CodeBuilder::return_);
assertSame(cb, ret);
ret = cb.transformMethod(methodBaz, MethodTransform.ACCEPT_ALL);
assertSame(cb, ret);
ret = cb.withField("bar", CD_int, ACC_STATIC);
assertSame(cb, ret);
ret = cb.transformField(fieldBaz, FieldTransform.ACCEPT_ALL);
assertSame(cb, ret);
});
Set<String> methodNames = new HashSet<>();
Set<String> fieldNames = new HashSet<>();
ClassTransform transform2 = (cb, ce) -> {
if (ce instanceof MethodModel mm) {
methodNames.add(mm.methodName().stringValue());
}
if (ce instanceof FieldModel fm) {
fieldNames.add(fm.fieldName().stringValue());
}
cb.with(ce);
};
cf.transformClass(cm, transform1.andThen(transform2));
assertEquals(Set.of(INIT_NAME, "foo", "bar", "baz"), methodNames);
assertEquals(Set.of("bar", "baz"), fieldNames);
}
/**
* Test to ensure method elements, such as generated
* or transformed code, are visible to transforms.
*/
@Test
void testMethodChaining() throws Exception {
var mtd = MethodTypeDesc.of(CD_String);
var cf = ClassFile.of();
// withCode
var cm = cf.parse(cf.build(ClassDesc.of("Temp"), clb -> clb
.withMethod("baz", mtd, ACC_STATIC | ACC_NATIVE, _ -> {})));
MethodTransform transform1 = MethodTransform.endHandler(mb -> {
var ret = mb.withCode(cob -> cob.loadConstant("foo").areturn());
assertSame(mb, ret);
});
boolean[] sawWithCode = { false };
MethodTransform transform2 = (mb, me) -> {
if (me instanceof CodeModel) {
sawWithCode[0] = true;
}
mb.with(me);
};
cf.transformClass(cm, ClassTransform.transformingMethods(transform1.andThen(transform2)));
assertTrue(sawWithCode[0], "Code attribute generated not visible");
// transformCode
var outerCm = cf.parse(testClassPath);
var foo = outerCm.methods().stream()
.filter(m -> m.flags().has(AccessFlag.STATIC))
.findFirst().orElseThrow();
MethodTransform transform3 = MethodTransform.endHandler(mb -> {
var ret = mb.transformCode(foo.code().orElseThrow(), CodeTransform.ACCEPT_ALL);
assertSame(mb, ret);
});
boolean[] sawTransformCode = { false };
MethodTransform transform4 = (mb, me) -> {
if (me instanceof CodeModel) {
sawTransformCode[0] = true;
}
mb.with(me);
};
cf.transformClass(cm, ClassTransform.transformingMethods(transform3.andThen(transform4)));
assertTrue(sawTransformCode[0], "Code attribute transformed not visible");
}
/**
* Test to ensure code elements, such as code block
* begin and end labels, are visible to transforms.
*/
@Test
void testCodeChaining() throws Exception {
var bytes = Files.readAllBytes(testClassPath);
var cf = ClassFile.of();
var cm = cf.parse(bytes);
CodeTransform transform1 = new CodeTransform() {
@Override
public void atStart(CodeBuilder builder) {
builder.block(bcb -> {
bcb.loadConstant(9876L);
bcb.goto_(bcb.endLabel());
});
}
@Override
public void accept(CodeBuilder builder, CodeElement element) {
builder.with(element);
}
};
Set<Label> leaveLabels = new HashSet<>();
Set<Label> targetedLabels = new HashSet<>();
CodeTransform transform2 = (cb, ce) -> {
if (ce instanceof BranchInstruction bi) {
leaveLabels.add(bi.target());
}
if (ce instanceof LabelTarget lt) {
targetedLabels.add(lt.label());
}
cb.with(ce);
};
cf.transformClass(cm, ClassTransform.transformingMethods(MethodTransform
.transformingCode(transform1.andThen(transform2))));
leaveLabels.removeIf(targetedLabels::contains);
assertTrue(leaveLabels.isEmpty(), () -> "Some labels are not bounded: " + leaveLabels);
}
public static class TestClass {
static public String foo() {
return "foo";