Skip to content

Commit c8e9075

Browse files
author
vsilaev
committed
Fixing severe bug in FastClassVerifier (all asm versions) - the cause of the invalid generated code
1 parent 60f55b7 commit c8e9075

File tree

16 files changed

+269
-219
lines changed

16 files changed

+269
-219
lines changed

‎net.tascalate.javaflow.providers.asm3/src/main/java/org/apache/commons/javaflow/providers/asm3/Asm3ClassTransformer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ final class Asm3ClassTransformer implements ResourceTransformer {
4343
}
4444

4545
public byte[] transform(final byte[] original) {
46-
final ClassWriter cw = new ComputeClassWriter(ClassWriter.COMPUTE_FRAMES, cciResolver.resourceLoader());
46+
final ComputeClassWriter cw = new ComputeClassWriter(ClassWriter.COMPUTE_FRAMES, cciResolver.resourceLoader());
4747
final ContinuableClassVisitor visitor = new ContinuableClassVisitor(
4848
cw /* BytecodeDebugUtils.decorateClassVisitor(cw, true, * System.err) -- DUMP*/,
4949
cciResolver,

‎net.tascalate.javaflow.providers.asm3/src/main/java/org/apache/commons/javaflow/providers/asm3/ComputeClassWriter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.objectweb.asm.ClassReader;
3939
import org.objectweb.asm.ClassWriter;
4040
import org.objectweb.asm.Opcodes;
41+
import org.objectweb.asm.Type;
4142

4243
/**
4344
* A ClassWriter that computes the common super class of two classes without
@@ -53,6 +54,10 @@ public ComputeClassWriter(final int flags, final ResourceLoader l) {
5354
super(flags);
5455
this.l = l;
5556
}
57+
58+
protected Type getCommonSuperType(final Type type1, final Type type2) {
59+
return Type.getObjectType(getCommonSuperClass(type1.getInternalName(), type2.getInternalName()));
60+
}
5661

5762
@Override
5863
protected String getCommonSuperClass(final String type1, final String type2) {

‎net.tascalate.javaflow.providers.asm3/src/main/java/org/apache/commons/javaflow/providers/asm3/ContinuableClassVisitor.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.apache.commons.javaflow.spi.ContinuableClassInfoResolver;
2828
import org.apache.commons.javaflow.spi.StopException;
2929
import org.objectweb.asm.ClassAdapter;
30-
import org.objectweb.asm.ClassVisitor;
3130
import org.objectweb.asm.FieldVisitor;
3231
import org.objectweb.asm.MethodVisitor;
3332
import org.objectweb.asm.Opcodes;
@@ -47,7 +46,7 @@ public class ContinuableClassVisitor extends ClassAdapter {
4746
private boolean skipEnchancing = false;
4847
private boolean isInterface = false;
4948

50-
public ContinuableClassVisitor(final ClassVisitor cv, final ContinuableClassInfoResolver cciResolver, final byte[] originalBytes) {
49+
public ContinuableClassVisitor(ComputeClassWriter cv, ContinuableClassInfoResolver cciResolver, byte[] originalBytes) {
5150
super(cv);
5251
this.cciResolver = cciResolver;
5352
this.originalBytes = originalBytes;
@@ -103,7 +102,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
103102
if (skip) {
104103
return mv;
105104
} else {
106-
return new ContinuableMethodNode(access, name, desc, signature, exceptions, className, cciResolver, mv);
105+
return new ContinuableMethodNode(access, name, desc, signature, exceptions, className, (ComputeClassWriter)cv, cciResolver, mv);
107106
}
108107
}
109108
}

‎net.tascalate.javaflow.providers.asm3/src/main/java/org/apache/commons/javaflow/providers/asm3/ContinuableMethodNode.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.objectweb.asm.tree.analysis.SourceValue;
5353

5454
public class ContinuableMethodNode extends MethodNode implements Opcodes {
55+
private final ComputeClassWriter verifierHelper;
5556
private final ContinuableClassInfoResolver cciResolver;
5657
private final String className;
5758

@@ -64,9 +65,14 @@ public class ContinuableMethodNode extends MethodNode implements Opcodes {
6465
protected Analyzer analyzer;
6566
public int stackRecorderVar;
6667

67-
public ContinuableMethodNode(int access, String name, String desc, String signature, String[] exceptions, String className, ContinuableClassInfoResolver cciResolver, MethodVisitor mv) {
68+
public ContinuableMethodNode(int access, String name, String desc, String signature, String[] exceptions,
69+
String className,
70+
ComputeClassWriter verifierHelper,
71+
ContinuableClassInfoResolver cciResolver,
72+
MethodVisitor mv) {
6873
super(access, name, desc, signature, exceptions);
6974
this.className = className;
75+
this.verifierHelper = verifierHelper;
7076
this.cciResolver = cciResolver;
7177
this.mv = mv;
7278
}
@@ -109,7 +115,7 @@ public void visitEnd() {
109115
try {
110116
moveNew();
111117

112-
analyzer = new Analyzer(new FastClassVerifier()) {
118+
analyzer = new Analyzer(new FastClassVerifier(verifierHelper)) {
113119
@Override
114120
protected Frame newFrame(final int nLocals, final int nStack) {
115121
return new MonitoringFrame(nLocals, nStack);

‎net.tascalate.javaflow.providers.asm3/src/main/java/org/apache/commons/javaflow/providers/asm3/FastClassVerifier.java‎

Lines changed: 73 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -23,91 +23,94 @@
2323
*/
2424
package org.apache.commons.javaflow.providers.asm3;
2525

26-
import org.objectweb.asm.Opcodes;
2726
import org.objectweb.asm.Type;
28-
import org.objectweb.asm.tree.AbstractInsnNode;
29-
import org.objectweb.asm.tree.analysis.AnalyzerException;
3027
import org.objectweb.asm.tree.analysis.BasicValue;
31-
import org.objectweb.asm.tree.analysis.BasicVerifier;
28+
import org.objectweb.asm.tree.analysis.SimpleVerifier;
3229
import org.objectweb.asm.tree.analysis.Value;
3330

34-
public class FastClassVerifier extends BasicVerifier {
35-
36-
/**
37-
* Constructs a new {@link FastClassVerifier} to verify a specific class. This
38-
* class will not be loaded into the JVM since it may be incorrect.
39-
*
40-
*/
41-
public FastClassVerifier() {
42-
super();
31+
public class FastClassVerifier extends SimpleVerifier {
32+
private ComputeClassWriter verifierHelper;
33+
34+
public FastClassVerifier(ComputeClassWriter verifierHelper) {
35+
this.verifierHelper = verifierHelper;
4336
}
44-
37+
4538
@Override
46-
public Value copyOperation(final AbstractInsnNode insn, Value value) throws AnalyzerException {
47-
// Fix error with analyzer for try-with-resources (it sees uninitialized values)
48-
if (insn.getOpcode() == Opcodes.ALOAD && (value instanceof BasicValue && !((BasicValue)value).isReference())) {
49-
value = newValue(Type.getType("Lnull;"));
39+
protected boolean isAssignableFrom(Type t, Type u) {
40+
if (t.equals(u)) {
41+
return true;
5042
}
51-
return super.copyOperation(insn, value);
52-
}
53-
54-
@Override
55-
public Value unaryOperation(final AbstractInsnNode insn, Value value) throws AnalyzerException {
56-
// Fix error with analyzer for try-with-resources (it sees uninitialized values)
57-
if (insn.getOpcode() == Opcodes.ARETURN && (value instanceof BasicValue && !((BasicValue)value).isReference())) {
58-
value = newValue(Type.getType("Lnull;"));
43+
// Null is assignable to any reference type
44+
if ("Lnull;".equals(u.getDescriptor()) && t.getSort() >= Type.ARRAY) {
45+
return true;
5946
}
60-
return super.unaryOperation(insn, value);
61-
}
62-
63-
@Override
64-
public Value newValue(final Type type) {
65-
if (type == null) {
66-
return BasicValue.UNINITIALIZED_VALUE;
47+
Type et, eu;
48+
if (t.getSort() == Type.ARRAY) {
49+
// u must be an array of bigger or equals dimension
50+
if (u.getSort() != Type.ARRAY ) {
51+
return false;
52+
}
53+
et = t.getElementType();
54+
eu = u.getElementType();
55+
int dt = t.getDimensions();
56+
int du = u.getDimensions();
57+
boolean isObject = et.equals(((BasicValue)BasicValue.REFERENCE_VALUE).getType());
58+
if (dt == du || dt < du && isObject) {
59+
// Ok
60+
} else {
61+
return false;
62+
}
63+
} else {
64+
et = t;
65+
eu = u;
6766
}
67+
Type commonType = verifierHelper.getCommonSuperType(et, eu);
68+
return commonType.equals(et);
6869

69-
final boolean isArray = type.getSort() == Type.ARRAY;
70-
Value v = super.newValue(type);
71-
if (BasicValue.REFERENCE_VALUE.equals(v)) {
72-
if (isArray) {
73-
v = newValue(type.getElementType());
74-
String desc = ((BasicValue)v).getType().getDescriptor();
75-
for (int i = 0; i < type.getDimensions(); ++i) {
76-
desc = '[' + desc;
70+
}
71+
72+
@Override
73+
public Value merge(final Value v, final Value w) {
74+
if (!v.equals(w)) {
75+
Type t = ((BasicValue)v).getType();
76+
Type u = ((BasicValue)w).getType();
77+
if (t != null
78+
&& (t.getSort() == Type.OBJECT || t.getSort() == Type.ARRAY)) {
79+
if (u != null
80+
&& (u.getSort() == Type.OBJECT || u.getSort() == Type.ARRAY)) {
81+
if ("Lnull;".equals(t.getDescriptor())) {
82+
return w;
83+
}
84+
if ("Lnull;".equals(u.getDescriptor())) {
85+
return v;
86+
}
87+
if (isAssignableFrom(t, u)) {
88+
return v;
89+
}
90+
if (isAssignableFrom(u, t)) {
91+
return w;
92+
}
93+
return new BasicValue(verifierHelper.getCommonSuperType(t, u));
7794
}
78-
v = new BasicValue(Type.getType(desc));
79-
} else {
80-
v = new BasicValue(type);
8195
}
96+
return BasicValue.UNINITIALIZED_VALUE;
8297
}
8398
return v;
8499
}
85100

86101
@Override
87-
protected boolean isSubTypeOf(final Value value, final Value expected) {
88-
if (!(value instanceof BasicValue))
89-
{
90-
return value.equals(expected);
91-
}
92-
Type expectedType = ((BasicValue)expected).getType();
93-
Type type = ((BasicValue)value).getType();
94-
switch (expectedType.getSort()) {
95-
case Type.BOOLEAN:
96-
case Type.CHAR:
97-
case Type.BYTE:
98-
case Type.SHORT:
99-
case Type.INT:
100-
case Type.FLOAT:
101-
case Type.LONG:
102-
case Type.DOUBLE:
103-
return type.equals(expectedType);
104-
case Type.ARRAY:
105-
case Type.OBJECT:
106-
// We are transforming valid bytecode to (hopefully) valid bytecode
107-
// hence pairs of "value" and "expected" must be compatible
108-
return true;//isAssignableFrom(expectedType, type);
109-
default:
110-
throw new Error("Internal error");
111-
}
102+
protected Class<?> getClass(final Type t) {
103+
throw new UnsupportedOperationException();
112104
}
113-
}
105+
106+
@Override
107+
protected boolean isInterface(final Type t) {
108+
throw new UnsupportedOperationException();
109+
}
110+
111+
@Override
112+
protected Type getSuperClass(final Type t) {
113+
throw new UnsupportedOperationException();
114+
}
115+
116+
}

‎net.tascalate.javaflow.providers.asm4/src/main/java/org/apache/commons/javaflow/providers/asm4/Asm4ClassTransformer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ final class Asm4ClassTransformer implements ResourceTransformer {
4343
}
4444

4545
public byte[] transform(final byte[] original) {
46-
final ClassWriter cw = new ComputeClassWriter(ClassWriter.COMPUTE_FRAMES, cciResolver.resourceLoader());
46+
final ComputeClassWriter cw = new ComputeClassWriter(ClassWriter.COMPUTE_FRAMES, cciResolver.resourceLoader());
4747
final ContinuableClassVisitor visitor = new ContinuableClassVisitor(
4848
cw /* BytecodeDebugUtils.decorateClassVisitor(cw, true, * System.err) -- DUMP*/,
4949
cciResolver,

‎net.tascalate.javaflow.providers.asm4/src/main/java/org/apache/commons/javaflow/providers/asm4/ComputeClassWriter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.objectweb.asm.ClassReader;
3939
import org.objectweb.asm.ClassWriter;
4040
import org.objectweb.asm.Opcodes;
41+
import org.objectweb.asm.Type;
4142

4243
/**
4344
* A ClassWriter that computes the common super class of two classes without
@@ -54,6 +55,10 @@ public ComputeClassWriter(final int flags, final ResourceLoader l) {
5455
this.l = l;
5556
}
5657

58+
protected Type getCommonSuperType(final Type type1, final Type type2) {
59+
return Type.getObjectType(getCommonSuperClass(type1.getInternalName(), type2.getInternalName()));
60+
}
61+
5762
@Override
5863
protected String getCommonSuperClass(final String type1, final String type2) {
5964
try {

‎net.tascalate.javaflow.providers.asm4/src/main/java/org/apache/commons/javaflow/providers/asm4/ContinuableClassVisitor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class ContinuableClassVisitor extends ClassVisitor {
4646
private boolean skipEnchancing = false;
4747
private boolean isInterface = false;
4848

49-
public ContinuableClassVisitor(final ClassVisitor cv, final ContinuableClassInfoResolver cciResolver, final byte[] originalBytes) {
49+
public ContinuableClassVisitor(final ComputeClassWriter cv, final ContinuableClassInfoResolver cciResolver, final byte[] originalBytes) {
5050
super(Opcodes.ASM4, cv);
5151
this.cciResolver = cciResolver;
5252
this.originalBytes = originalBytes;
@@ -102,7 +102,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
102102
if (skip) {
103103
return mv;
104104
} else {
105-
return new ContinuableMethodNode(access, name, desc, signature, exceptions, className, cciResolver, mv);
105+
return new ContinuableMethodNode(access, name, desc, signature, exceptions, className, (ComputeClassWriter)cv, cciResolver, mv);
106106
}
107107
}
108108
}

‎net.tascalate.javaflow.providers.asm4/src/main/java/org/apache/commons/javaflow/providers/asm4/ContinuableMethodNode.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.objectweb.asm.tree.analysis.SourceValue;
5555

5656
public class ContinuableMethodNode extends MethodNode implements Opcodes {
57+
private final ComputeClassWriter verifierHelper;
5758
private final ContinuableClassInfoResolver cciResolver;
5859
private final String className;
5960

@@ -66,9 +67,14 @@ public class ContinuableMethodNode extends MethodNode implements Opcodes {
6667
protected Analyzer analyzer;
6768
public int stackRecorderVar;
6869

69-
public ContinuableMethodNode(int access, String name, String desc, String signature, String[] exceptions, String className, ContinuableClassInfoResolver cciResolver, MethodVisitor mv) {
70+
public ContinuableMethodNode(int access, String name, String desc, String signature, String[] exceptions,
71+
String className,
72+
ComputeClassWriter verifierHelper,
73+
ContinuableClassInfoResolver cciResolver,
74+
MethodVisitor mv) {
7075
super(Opcodes.ASM4, access, name, desc, signature, exceptions);
7176
this.className = className;
77+
this.verifierHelper = verifierHelper;
7278
this.cciResolver = cciResolver;
7379
this.mv = mv;
7480
}
@@ -122,7 +128,7 @@ public void visitEnd() {
122128
try {
123129
moveNew();
124130

125-
analyzer = new Analyzer(new FastClassVerifier()) {
131+
analyzer = new Analyzer(new FastClassVerifier(verifierHelper)) {
126132
@Override
127133
protected Frame newFrame(final int nLocals, final int nStack) {
128134
return new MonitoringFrame(nLocals, nStack);

0 commit comments

Comments
 (0)