From 1cd5a49d80a0bdc85ef0932e2f2a7bf94be77b4e Mon Sep 17 00:00:00 2001 From: emeroad Date: Thu, 26 May 2016 17:58:42 +0900 Subject: [PATCH] #1807 Javassist bug : Incorrect StackMap Frame generation - add bug regression testcase --- java8-test/pom.xml | 16 +++ .../javassist/CustomURLClassLoader.java | 32 ++++++ .../javassist/InvalidStackMapFrame.java | 25 +++++ .../javassist/JavassistVerifyErrorTest.java | 104 ++++++++++++++++++ .../asm/AddIntVariableMethodAdapter.java | 36 ++++++ .../asm/BytecodeVerifyTestClassVisitor.java | 45 ++++++++ java8-test/src/test/resources/log4j.xml | 39 +++++++ .../pinpoint/test/util/BytecodeUtils.java | 72 +++++++++--- 8 files changed, 351 insertions(+), 18 deletions(-) create mode 100644 java8-test/src/test/java/bug_regression_jdk7/javassist/CustomURLClassLoader.java create mode 100644 java8-test/src/test/java/bug_regression_jdk7/javassist/InvalidStackMapFrame.java create mode 100644 java8-test/src/test/java/bug_regression_jdk7/javassist/JavassistVerifyErrorTest.java create mode 100644 java8-test/src/test/java/bug_regression_jdk7/javassist/asm/AddIntVariableMethodAdapter.java create mode 100644 java8-test/src/test/java/bug_regression_jdk7/javassist/asm/BytecodeVerifyTestClassVisitor.java create mode 100644 java8-test/src/test/resources/log4j.xml diff --git a/java8-test/pom.xml b/java8-test/pom.xml index b46a0a624..537c20ba2 100644 --- a/java8-test/pom.xml +++ b/java8-test/pom.xml @@ -36,6 +36,22 @@ test + + com.navercorp.pinpoint + pinpoint-profiler + test + + + org.javassist + javassist + test + + + org.ow2.asm + asm-debug-all + test + + diff --git a/java8-test/src/test/java/bug_regression_jdk7/javassist/CustomURLClassLoader.java b/java8-test/src/test/java/bug_regression_jdk7/javassist/CustomURLClassLoader.java new file mode 100644 index 000000000..458f37150 --- /dev/null +++ b/java8-test/src/test/java/bug_regression_jdk7/javassist/CustomURLClassLoader.java @@ -0,0 +1,32 @@ +package bug_regression_jdk7.javassist; + +import java.net.URL; +import java.net.URLClassLoader; + +/** + * @author Woonduk Kang(emeroad) + */ +public class CustomURLClassLoader extends URLClassLoader { + + public CustomURLClassLoader(URL[] urls, ClassLoader parent) { + super(urls, parent); + } + + public CustomURLClassLoader(URL[] urls) { + super(urls); + } + + public Class defineClass0(String name, byte[] bytes) { + return this.defineClass(name, bytes, 0, bytes.length); + } + + public Class defineClass0(String name, byte[] bytes, int off, int length) { + return super.defineClass(name, bytes, off, length); + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + return super.loadClass(name); + } +} + diff --git a/java8-test/src/test/java/bug_regression_jdk7/javassist/InvalidStackMapFrame.java b/java8-test/src/test/java/bug_regression_jdk7/javassist/InvalidStackMapFrame.java new file mode 100644 index 000000000..fe741b2ce --- /dev/null +++ b/java8-test/src/test/java/bug_regression_jdk7/javassist/InvalidStackMapFrame.java @@ -0,0 +1,25 @@ +package bug_regression_jdk7.javassist; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author Woonduk Kang(emeroad) + */ +public class InvalidStackMapFrame { + + public void bytecodeVerifyError() { + // javassist bug : invalid stack map frame + List test = new ArrayList(); + String[] newLine = new String[10]; + for (Integer idx : test) { + String address = newLine[1]; + int tabPos = -1; + if (tabPos != -1) { + address = address.substring(tabPos + 1); + } + newLine[4] = address; + } + + } +} diff --git a/java8-test/src/test/java/bug_regression_jdk7/javassist/JavassistVerifyErrorTest.java b/java8-test/src/test/java/bug_regression_jdk7/javassist/JavassistVerifyErrorTest.java new file mode 100644 index 000000000..4b70fa5f9 --- /dev/null +++ b/java8-test/src/test/java/bug_regression_jdk7/javassist/JavassistVerifyErrorTest.java @@ -0,0 +1,104 @@ +package bug_regression_jdk7.javassist; + +import bug_regression_jdk7.javassist.asm.BytecodeVerifyTestClassVisitor; +import com.navercorp.pinpoint.profiler.instrument.ASMBytecodeDisassembler; +import com.navercorp.pinpoint.profiler.util.JavaAssistUtils; +import javassist.ClassPool; +import javassist.CtClass; +import javassist.CtMethod; +import javassist.LoaderClassPath; +import org.junit.Assert; +import org.junit.Test; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.InputStream; +import java.net.URL; + +/** + * @author Woonduk Kang(emeroad) + */ +public class JavassistVerifyErrorTest { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private static final String INVALID_STACK_MAP_FRAME = "bug_regression_jdk7.javassist.InvalidStackMapFrame"; + + /** + * bug id + * https://github.com/naver/pinpoint/issues/1807 + * @throws Exception + */ + @Test + public void bug_regression_BytecodeVerifyError_Invalid_StackMapFrame() throws Exception { + + CustomURLClassLoader classLoader = new CustomURLClassLoader(new URL[]{}, Thread.currentThread().getContextClassLoader()); + + ClassPool classPool = new ClassPool(true); + classPool.appendClassPath(new LoaderClassPath(classLoader)); + + final CtClass ctClass = classPool.get(INVALID_STACK_MAP_FRAME); + final CtMethod method = ctClass.getDeclaredMethod("bytecodeVerifyError"); + method.addLocalVariable("test_localVariable", CtClass.intType); + method.insertBefore("{ test_localVariable = 1; }"); + + final byte[] bytecode = ctClass.toBytecode(); + classLoader.defineClass0(INVALID_STACK_MAP_FRAME, bytecode); + try { + Class.forName(INVALID_STACK_MAP_FRAME, true, classLoader); + Assert.fail("VerifyError"); + } catch (java.lang.VerifyError e) { + logger.debug("verifyError:{}", e.getMessage(), e); + } + + + final ASMBytecodeDisassembler bytecodeDisassembler = new ASMBytecodeDisassembler(); + + final String dumpBytecode = bytecodeDisassembler.dumpBytecode(bytecode); + logger.debug("dumpBytecode:{}", dumpBytecode); + +// javassist bug : invalid stack map frame +// 00013 InvalidStackMapFrame ArrayList String Iterator I : : FRAME FULL [bug_regression_jdk7/javassist/InvalidStackMapFrame java/util/ArrayList [[[java/lang/Object->[Ljava/lang/String;]]] java/util/Iterator T T T I] [] + final String verify = bytecodeDisassembler.dumpVerify(bytecode, classLoader); + logger.debug("dumpVerify:{}", verify); + + final String dumpAsm = bytecodeDisassembler.dumpASM(bytecode); + logger.debug("dumpAsm :{}", dumpAsm); + + } + + + @Test + public void asm_stackmapframe_check() throws Exception { + + CustomURLClassLoader classLoader = new CustomURLClassLoader(new URL[]{}, Thread.currentThread().getContextClassLoader()); + final InputStream stream = classLoader.getResourceAsStream(JavaAssistUtils.javaNameToJvmName(INVALID_STACK_MAP_FRAME) + ".class"); + + ClassReader cr = new ClassReader(stream); + ClassWriter cw = new ClassWriter(cr, ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); + ClassVisitor cv = new BytecodeVerifyTestClassVisitor(cw); + cr.accept(cv, ClassReader.EXPAND_FRAMES | ClassReader.SKIP_DEBUG); + + byte[] bytecode = cw.toByteArray(); + classLoader.defineClass0(INVALID_STACK_MAP_FRAME, bytecode); + + final Class aClass = Class.forName(INVALID_STACK_MAP_FRAME, true, classLoader); + Assert.assertSame(aClass.getClassLoader(), classLoader); + + + final ASMBytecodeDisassembler bytecodeDisassembler = new ASMBytecodeDisassembler(); + + final String dumpBytecode = bytecodeDisassembler.dumpBytecode(bytecode); + logger.debug("dumpBytecode:{}", dumpBytecode); + + final String verify = bytecodeDisassembler.dumpVerify(bytecode, classLoader); + logger.debug("dumpVerify:{}", verify); + +// final String dumpAsm = bytecodeDisassembler.dumpASM(bytecode); +// logger.debug("dumpAsm :{}", dumpAsm); + + } +} diff --git a/java8-test/src/test/java/bug_regression_jdk7/javassist/asm/AddIntVariableMethodAdapter.java b/java8-test/src/test/java/bug_regression_jdk7/javassist/asm/AddIntVariableMethodAdapter.java new file mode 100644 index 000000000..799cce859 --- /dev/null +++ b/java8-test/src/test/java/bug_regression_jdk7/javassist/asm/AddIntVariableMethodAdapter.java @@ -0,0 +1,36 @@ +package bug_regression_jdk7.javassist.asm; + +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.commons.AdviceAdapter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * @author Woonduk Kang(emeroad) + */ +public class AddIntVariableMethodAdapter extends AdviceAdapter { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private String name; + + public AddIntVariableMethodAdapter(MethodVisitor mv, int acc, String name, String desc) { + super(Opcodes.ASM5, mv, acc, name, desc); + this.name = name; + + } + + @Override + public void visitMaxs(int maxStack, int maxLocals) { + super.visitMaxs(maxStack + 1, maxLocals); + } + + @Override + protected void onMethodEnter() { + logger.debug("onMethodEnter() add int local variable inst:ISTORE_7 {}", name); + mv.visitInsn(Opcodes.ICONST_0); + mv.visitVarInsn(Opcodes.ISTORE, 7); + } + +} diff --git a/java8-test/src/test/java/bug_regression_jdk7/javassist/asm/BytecodeVerifyTestClassVisitor.java b/java8-test/src/test/java/bug_regression_jdk7/javassist/asm/BytecodeVerifyTestClassVisitor.java new file mode 100644 index 000000000..7cb07783a --- /dev/null +++ b/java8-test/src/test/java/bug_regression_jdk7/javassist/asm/BytecodeVerifyTestClassVisitor.java @@ -0,0 +1,45 @@ +package bug_regression_jdk7.javassist.asm; + +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.objectweb.asm.Opcodes.ACC_PUBLIC; + +/** + * @author Woonduk Kang(emeroad) + */ +public class BytecodeVerifyTestClassVisitor extends ClassVisitor { + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private String name; + + public BytecodeVerifyTestClassVisitor(final ClassVisitor cv) { + super(org.objectweb.asm.Opcodes.ASM5, cv); + } + + @Override + public void visit(final int version, final int access, final String name, + final String signature, final String superName, + final String[] interfaces) { + this.name = name; + super.visit(version, access, name, signature, superName, interfaces); + } + + @Override + public MethodVisitor visitMethod(final int access, final String name, + final String desc, final String signature, final String[] exceptions) { + + MethodVisitor mv = cv.visitMethod(access, name, desc, signature, exceptions); + logger.debug("mv:{}", mv); + logger.debug("name:{}", name); + logger.debug("desc:{}", desc); + logger.debug("signature:{}", signature); + if (name.contains("bytecodeVerifyError")) { + return new AddIntVariableMethodAdapter(mv, Opcodes.ACC_PUBLIC, name, "()V"); + } + return mv; + } +} diff --git a/java8-test/src/test/resources/log4j.xml b/java8-test/src/test/resources/log4j.xml new file mode 100644 index 000000000..9f622e9eb --- /dev/null +++ b/java8-test/src/test/resources/log4j.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/profiler/src/main/java/com/navercorp/pinpoint/test/util/BytecodeUtils.java b/profiler/src/main/java/com/navercorp/pinpoint/test/util/BytecodeUtils.java index 9a6870be4..3bf42d87c 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/test/util/BytecodeUtils.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/test/util/BytecodeUtils.java @@ -16,13 +16,12 @@ package com.navercorp.pinpoint.test.util; +import com.navercorp.pinpoint.profiler.util.JavaAssistUtils; + import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Method; -import java.nio.ByteBuffer; -import java.nio.channels.Channels; -import java.nio.channels.ReadableByteChannel; // TODO move package public final class BytecodeUtils { @@ -63,32 +62,69 @@ public final class BytecodeUtils { throw new NullPointerException("className must not be null"); } - final InputStream is = classLoader.getResourceAsStream(className.replace('.', '/') + ".class"); + final String jvmClassName = JavaAssistUtils.javaNameToJvmName(className); + final InputStream is = classLoader.getResourceAsStream(jvmClassName + ".class"); if (is == null) { throw new RuntimeException("No such class file: " + className); } - ReadableByteChannel channel = Channels.newChannel(is); - ByteBuffer buffer; - try { - buffer = ByteBuffer.allocate(is.available()); - - while (channel.read(buffer) >= 0) { - if (buffer.remaining() == 0) { - buffer.flip(); - ByteBuffer newBuffer = ByteBuffer.allocate(buffer.capacity() * 2); - newBuffer.put(buffer); - buffer = newBuffer; - } - } + return readClass(is, false); } catch (IOException e) { throw new RuntimeException(e); } finally { close(is); } + } - return buffer.array(); + /** + * COPY ASM method. reference : org.objectweb.asm.ClassReader + * + * Reads the bytecode of a class. + * + * @param is + * an input stream from which to read the class. + * @param close + * true to close the input stream after reading. + * @return the bytecode read from the given input stream. + * @throws IOException + * if a problem occurs during reading. + */ + public static byte[] readClass(final InputStream is, boolean close) + throws IOException { + if (is == null) { + throw new IOException("Class not found"); + } + try { + byte[] b = new byte[is.available()]; + int len = 0; + while (true) { + int n = is.read(b, len, b.length - len); + if (n == -1) { + if (len < b.length) { + byte[] c = new byte[len]; + System.arraycopy(b, 0, c, 0, len); + b = c; + } + return b; + } + len += n; + if (len == b.length) { + int last = is.read(); + if (last < 0) { + return b; + } + byte[] c = new byte[b.length + 1000]; + System.arraycopy(b, 0, c, 0, len); + c[len++] = (byte) last; + b = c; + } + } + } finally { + if (close) { + is.close(); + } + } } private static void close(Closeable closeable) {