8262002: java/lang/instrument/VerifyLocalVariableTableOnRetransformTest.sh failed with "TestCaseScaffoldException: DummyClassWithLVT did not match .class file"

Reviewed-by: coleenp, sspitsyn

src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2021, 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
@@ -789,18 +789,12 @@ void JvmtiClassFileReconstituter::write_class_attributes() {
   if (ik()->source_debug_extension() != NULL) {
     write_source_debug_extension_attribute();
   }
-  if (inner_classes_length > 0) {
-    write_inner_classes_attribute(inner_classes_length);
-  }
   if (anno != NULL) {
     write_annotations_attribute("RuntimeVisibleAnnotations", anno);
   }
   if (type_anno != NULL) {
     write_annotations_attribute("RuntimeVisibleTypeAnnotations", type_anno);
   }
-  if (cpool()->operands() != NULL) {
-    write_bootstrapmethod_attribute();
-  }
   if (ik()->nest_host_index() != 0) {
     write_nest_host_attribute();
   }
@@ -813,6 +807,12 @@ void JvmtiClassFileReconstituter::write_class_attributes() {
   if (ik()->record_components() != NULL) {
     write_record_attribute();
   }
+  if (cpool()->operands() != NULL) {
+    write_bootstrapmethod_attribute();
+  }
+  if (inner_classes_length > 0) {
+    write_inner_classes_attribute(inner_classes_length);
+  }
 }
 
 // Write the method information portion of ClassFile structure

test/jdk/java/lang/instrument/ATransformerManagementTestCase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2021, 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
@@ -117,9 +117,11 @@ ATransformerManagementTestCase
         {
             fTransformers.add(transformer);
         }
+        // workaroud for JDK-8264667: create log message before addTransformer()
+        String msg = "Added transformer " + transformer
+            + " with canRetransform=" + canRetransform;
         manager.addTransformer(transformer, canRetransform);
-        verbosePrint("Added transformer " + transformer
+        verbosePrint(msg);
-            + " with canRetransform=" + canRetransform);
     }
 
     /**

test/jdk/java/lang/instrument/VerifyLocalVariableTableOnRetransformTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, 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
@@ -21,11 +21,30 @@
  * questions.
  */
 
+/*
+ * @test
+ * @bug 7064927
+ * @summary Verify LocalVariableTable (LVT) exists when passed to transform() on a retransform operation.
+ * @author Daniel D. Daugherty
+ *
+ * @library /test/lib
+ * @run build VerifyLocalVariableTableOnRetransformTest
+ * @run compile -g DummyClassWithLVT.java
+ * @run shell MakeJAR.sh retransformAgent
+ * @run main/othervm -javaagent:retransformAgent.jar VerifyLocalVariableTableOnRetransformTest VerifyLocalVariableTableOnRetransformTest
+ */
+
 import java.io.*;
 import java.lang.instrument.Instrumentation;
 import java.lang.instrument.ClassFileTransformer;
 import java.net.*;
 import java.security.ProtectionDomain;
+import java.util.List;
+import java.util.regex.Pattern;
+
+import jdk.test.lib.JDKToolLauncher;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
 
 public class
 VerifyLocalVariableTableOnRetransformTest
@@ -34,6 +53,7 @@ VerifyLocalVariableTableOnRetransformTest
     private byte[]  fTargetClassBytes;
     private boolean fTargetClassMatches;
     private String  fTargetClassName = "DummyClassWithLVT";
+    private String  classFileName = fTargetClassName + ".class";
     private boolean fTargetClassSeen;
 
     /**
@@ -44,8 +64,7 @@ VerifyLocalVariableTableOnRetransformTest
     {
         super(name);
 
-        String resourceName = fTargetClassName + ".class";
+        File f = originalClassFile();
-        File f = new File(System.getProperty("test.classes", "."), resourceName);
         System.out.println("Reading test class from " + f);
         try
         {
@@ -55,7 +74,7 @@ VerifyLocalVariableTableOnRetransformTest
         }
         catch (IOException e)
         {
-            fail("Could not load the class: "+resourceName);
+            fail("Could not load the class: " + f.getName());
         }
     }
 
@@ -105,8 +124,7 @@ VerifyLocalVariableTableOnRetransformTest
         // behavior by the HotSpot VM. If this behavior is intentionally
         // changed in the future, then this test will need to be
         // updated.
-        assertTrue(fTargetClassName + " did not match .class file",
+        compareClassFileBytes(true);
-            fTargetClassMatches);
 
         fTargetClassSeen = false;
         fTargetClassMatches = false;
@@ -120,8 +138,88 @@ VerifyLocalVariableTableOnRetransformTest
         // Table (LVT) attribute so the class file bytes seen by the
         // retransformClasses() call will not match the class file bytes
         // seen at initial class load time.
-        assertTrue(fTargetClassName + " did not match .class file",
+        compareClassFileBytes(false);
-            fTargetClassMatches);
+    }
+
+    private File originalClassFile() {
+        return new File(System.getProperty("test.classes", "."), classFileName);
+    }
+
+    private File transformedClassFile() {
+        // This file will get created in the test execution
+        // directory so there is no conflict with the file
+        // in the test classes directory.
+        return new File(classFileName);
+    }
+
+    private static final String[] expectedDifferentStrings = {
+            "^Classfile .+$",
+            "^[\\s]+SHA-256 checksum .[^\\s]+$"
+    };
+
+    private boolean expectedDifferent(String line) {
+        for (String s: expectedDifferentStrings) {
+            Pattern p = Pattern.compile(s);
+            if (p.matcher(line).find()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private void compareClassFileBytes(boolean initialLoad) throws Throwable {
+        if (fTargetClassMatches) {
+            return;
+        }
+        File f1 = originalClassFile();
+        File f2 = transformedClassFile();
+        System.out.println(fTargetClassName + " did not match .class file");
+        System.out.println("Disassembly difference (" + f1 + " vs " + f2 +"):");
+        // compare 'javap -v' output for the class files
+        List<String> out1 = disassembleClassFile(f1);
+        List<String> out2 = disassembleClassFile(f2);
+        boolean different = false;
+        boolean orderChanged = false;
+        int lineNum = 0;
+        for (String line: out1) {
+            if (!expectedDifferent(line)) {
+                if (!out2.contains(line)) {
+                    different = true;
+                    System.out.println("< (" + (lineNum + 1) + ") " + line);
+                } else {
+                    if (lineNum < out2.size() && out1.get(lineNum) != out2.get(lineNum)) {
+                        // out2 contains line, but at different position
+                        orderChanged = true;
+                    }
+                }
+            }
+            lineNum++;
+        }
+        lineNum = 0;
+        for (String line: out2) {
+            if (!expectedDifferent(line)) {
+                if (!out1.contains(line)) {
+                    different = true;
+                    System.out.println("> (" + (lineNum + 1) + ") " + line);
+                }
+            }
+            lineNum++;
+        }
+        // accordingly the spec orderChanged is fine, but we consider it as error
+        // (see comments in verifyClassFileBuffer())
+        if (different || orderChanged) {
+            fail(fTargetClassName + " (" + (initialLoad ? "load" : "retransform") + ") did not match .class file"
+                    + (different ? "" : " (only order changed)"));
+        }
+    }
+
+    private List<String> disassembleClassFile(File file) throws Throwable {
+        JDKToolLauncher javap = JDKToolLauncher.create("javap")
+                                               .addToolArg("-v")
+                                               .addToolArg(file.toString());
+        ProcessBuilder pb = new ProcessBuilder(javap.getCommand());
+        OutputAnalyzer out = ProcessTools.executeProcess(pb);
+        return out.asLines();
     }
 
     public class MyObserver implements ClassFileTransformer {
@@ -134,12 +232,7 @@ VerifyLocalVariableTableOnRetransformTest
 
         private void saveMismatchedBytes(byte[] classfileBuffer) {
             try {
-                FileOutputStream fos = null;
+                FileOutputStream fos = new FileOutputStream(transformedClassFile());
-                // This file will get created in the test execution
-                // directory so there is no conflict with the file
-                // in the test classes directory.
-                String resourceName = fTargetClassName + ".class";
-                fos = new FileOutputStream(resourceName);
                 fos.write(classfileBuffer);
                 fos.close();
             } catch (IOException ex) {

test/jdk/java/lang/instrument/VerifyLocalVariableTableOnRetransformTest.sh

@@ -1,83 +0,0 @@
-#
-# Copyright (c) 2012, 2015, 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.
-#
-# 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.
-#
-
-#
-
-# @test
-# @bug 7064927
-# @summary Verify LocalVariableTable (LVT) exists when passed to
-# transform() on a retransform operation.
-# @author Daniel D. Daugherty
-#
-# @run build VerifyLocalVariableTableOnRetransformTest
-# @run compile -g DummyClassWithLVT.java
-# @run shell MakeJAR.sh retransformAgent
-# @run shell VerifyLocalVariableTableOnRetransformTest.sh
-
-
-if [ "${TESTJAVA}" = "" ]
-then
-  echo "TESTJAVA not set.  Test cannot execute.  Failed."
-  exit 1
-fi
-
-if [ "${TESTSRC}" = "" ]
-then
-  echo "TESTSRC not set.  Test cannot execute.  Failed."
-  exit 1
-fi
-
-if [ "${TESTCLASSES}" = "" ]
-then
-  echo "TESTCLASSES not set.  Test cannot execute.  Failed."
-  exit 1
-fi
-
-JAVA="${TESTJAVA}"/bin/java
-
-"${JAVA}" ${TESTVMOPTS} -Dtest.classes="${TESTCLASSES}" \
-    -javaagent:retransformAgent.jar \
-    -classpath "${TESTCLASSES}" \
-    VerifyLocalVariableTableOnRetransformTest \
-    VerifyLocalVariableTableOnRetransformTest \
-    > output.log 2>&1
-cat output.log
-
-MESG="did not match .class file"
-grep "$MESG" output.log
-result=$?
-if [ "$result" = 0 ]; then
-    echo "FAIL: found '$MESG' in the test output"
-
-    echo "INFO: 'javap -v' comparison between the .class files:"
-    ${JAVA}p -v -classpath "${TESTCLASSES}" DummyClassWithLVT > orig.javap
-    ${JAVA}p -v DummyClassWithLVT > mismatched.javap
-    diff orig.javap mismatched.javap
-
-    result=1
-else
-    echo "PASS: did NOT find '$MESG' in the test output"
-    result=0
-fi
-
-exit $result