Javassist #328 workaround: repair stack map using ASM -> no more '-no…

…verify' TODO: remove after fix for #328 is released, see jboss-javassist/javassist#328
SarekTest · Jul 14, 2020 · 0f5083d · 0f5083d
1 parent 50cc603
commit 0f5083d
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 4 deletions.
diff --git a/sarek-constructor-mock-javassist/pom.xml b/sarek-constructor-mock-javassist/pom.xml
@@ -16,10 +16,6 @@
 
       <plugin>
         <artifactId>maven-failsafe-plugin</artifactId>
-        <configuration>
-          <!-- TODO: remove '-noverify' after https://github.com/jboss-javassist/javassist/issues/328 is fixed -->
-          <argLine>-noverify</argLine>
-        </configuration>
         <executions>
           <execution>
             <id>reuse-jvm</id>
@@ -45,6 +41,14 @@
       <groupId>dev.sarek</groupId>
       <artifactId>sarek-agent-common</artifactId>
     </dependency>
+    <!--
+      Temporary dependency for repairing stack map frames
+      TODO: remove after fix for https://github.com/jboss-javassist/javassist/issues/328 is released
+    -->
+    <dependency>
+      <groupId>net.bytebuddy</groupId>
+      <artifactId>byte-buddy</artifactId>
+    </dependency>
 
     <!-- Test dependencies -->
 

diff --git a/...t/src/main/java/dev/sarek/agent/constructor_mock/ConstructorMockJavassistTransformer.java b/...t/src/main/java/dev/sarek/agent/constructor_mock/ConstructorMockJavassistTransformer.java
@@ -2,6 +2,7 @@
 
 import dev.sarek.agent.Transformer;
 import javassist.*;
+import net.bytebuddy.jar.asm.*;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -128,6 +129,11 @@ public byte[] transform(
       return null;
     }
 
+    // TODO: remove after fix for https://github.com/jboss-javassist/javassist/issues/328 is released
+    final boolean REPAIR = true;
+    if (REPAIR)
+      transformedBytecode = repairStackMapUsingASM(className, transformedBytecode);
+
     if (DUMP_CLASS_FILES) {
       Path path = new File(DUMP_CLASS_BASE_DIR + "/" + className + ".class").toPath();
       try {
@@ -144,6 +150,47 @@ public byte[] transform(
     return transformedBytecode;
   }
 
+  private byte[] repairStackMapUsingASM(String className, byte[] transformedBytecode) {
+    if (DUMP_CLASS_FILES) {
+      Path path = new File(DUMP_CLASS_BASE_DIR + "/" + className + ".unrepaired.class").toPath();
+      try {
+        Files.createDirectories(path.getParent());
+        log("Dumping (unrepaired) transformed class file " + path.toAbsolutePath());
+        Files.write(path, transformedBytecode);
+      }
+      catch (Exception e) {
+        log("ERROR: Cannot write (unrepaired) class file to " + path.toAbsolutePath());
+        e.printStackTrace();
+      }
+    }
+
+    // Repair stack map frames via ASM
+    ClassReader classReader = new ClassReader(transformedBytecode);
+
+    // Directly passing the writer to the reader leads to re-ordering of the constant pool table. This is not a
+    // problem with regard to functionality as such, but more difficult to diff when comparing the 'javap' output with
+    // the corresponding result created directly via ASM.
+    //
+    //   ClassWriter classWriter = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
+    //   classReader.accept(classWriter, ClassReader.SKIP_FRAMES);
+    //
+    // So we use this slightly more complicated method which copies the original constant pool, new entries only being
+    // appended to it as needed. Solution taken from https://stackoverflow.com/a/46644677/1082681.
+    ClassWriter classWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_FRAMES);
+    classReader.accept(
+      new ClassVisitor(Opcodes.ASM5, classWriter) {
+        @Override
+        public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
+          MethodVisitor writer = super.visitMethod(access, name, desc, signature, exceptions);
+          return new MethodVisitor(Opcodes.ASM5, writer) {};
+        }
+      },
+      ClassReader.SKIP_FRAMES
+    );
+
+    return classWriter.toByteArray();
+  }
+
   // TODO: This only works if all classes are being transformed via class-loading. Implement recursive manual mode which
   //       does not require the user to retransform parent classes by himself. For that purpose but also generally, it
   //       would be good to also have a registry of already transformed classes (per classloader?) so as to avoid