Merge pull request #3 from jboss-javassist/master

Sync with base/master.

20 files changed, 334 insertions, 17 deletions

diff --git a/Readme.html b/Readme.html
index b89b5b95..68f49e1e 100644
--- a/Readme.html
+++ b/Readme.html
@@ -281,9 +281,20 @@ see javassist.Dump.
 
 <h2>Changes</h2>
 
+<p>-version 3.25
+<ul>
+    <li>GitHub Issue #72 (PR #231), #241, #242 (PR #243), PR #244.
+</ul>
+
+<p>-version 3.24.1 on December 9, 2018
+<ul>
+    <li>GitHub Issue #228, #229</li>
+<ul>
+</p>
+
 <p>-version 3.24 on November 1, 2018
 <ul>
-	<li>Java 11 supports.</li>
+    <li>Java 11 supports.</li>
     <li>JIRA JASSIST-267.</li>
     <li>Github PR #218.</li>
 </ul>
diff --git a/build.xml b/build.xml
index 1c3f8b0f..c0178eea 100644
--- a/build.xml
+++ b/build.xml
@@ -6,7 +6,7 @@
 
 <project name="javassist" default="jar" basedir=".">
 
-  <property name="dist-version" value="javassist-3.24-GA"/>
+  <property name="dist-version" value="javassist-3.24.1-GA"/>
 
   <property environment="env"/>
   <property name="target.jar" value="javassist.jar"/>
diff --git a/javassist.jar b/javassist.jar
index 3099d18a..f9e84222 100644
--- a/javassist.jar
+++ b/javassist.jar
diff --git a/pom.xml b/pom.xml
index 6d42cf18..eaef95f1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -7,7 +7,7 @@
   	Javassist (JAVA programming ASSISTant) makes Java bytecode manipulation
     simple.  It is a class library for editing bytecodes in Java.
   </description>
-  <version>3.24.0-GA</version>
+  <version>3.24.1-GA</version>
   <name>Javassist</name>
   <url>http://www.javassist.org/</url>
 
@@ -210,6 +210,7 @@
 Copyright (C) 1999- Shigeru Chiba. All Rights Reserved.</i>]]></bottom>
           <show>public</show>
           <nohelp>true</nohelp>
+          <doclint>none</doclint>
 	</configuration>
       </plugin>
       <plugin>
diff --git a/src/main/META-INF/MANIFEST.MF b/src/main/META-INF/MANIFEST.MF
index 09bb026c..72421eb8 100644
--- a/src/main/META-INF/MANIFEST.MF
+++ b/src/main/META-INF/MANIFEST.MF
@@ -1,5 +1,5 @@
 Specification-Title: Javassist
 Specification-Vendor: Shigeru Chiba, www.javassist.org
-Specification-Version: 3.24.0-GA
+Specification-Version: 3.24.1-GA
 Main-Class: javassist.CtClass
 Automatic-Module-Name: org.javassist
diff --git a/src/main/javassist/CodeConverter.java b/src/main/javassist/CodeConverter.java
index 6df3622c..10d7ddad 100644
--- a/src/main/javassist/CodeConverter.java
+++ b/src/main/javassist/CodeConverter.java
@@ -25,6 +25,7 @@ import javassist.convert.TransformAccessArrayField;
 import javassist.convert.TransformAfter;
 import javassist.convert.TransformBefore;
 import javassist.convert.TransformCall;
+import javassist.convert.TransformCallToStatic;
 import javassist.convert.TransformFieldAccess;
 import javassist.convert.TransformNew;
 import javassist.convert.TransformNewClass;
@@ -407,6 +408,42 @@ public class CodeConverter {
     }
 
     /**
+     * Redirect non-static method invocations in a method body to a static
+     * method. The return type must be same with the originally invoked method.
+     * As parameters, the static method receives
+     * the target object and all the parameters to the originally invoked
+     * method.  For example, if the originally invoked method is
+     * <code>move()</code>:
+     *
+     * <pre>class Point {
+     *     Point move(int x, int y) { ... }
+     * }</pre>
+     *
+     * <p>Then the static method must be something like this:
+     *
+     * <pre>class Verbose {
+     *     static Point print(Point target, int x, int y) { ... }
+     * }</pre>
+     *
+     * <p>The <code>CodeConverter</code> would translate bytecode
+     * equivalent to:
+     *
+     * <pre>Point p2 = p.move(x + y, 0);</pre>
+     *
+     * <p>into the bytecode equivalent to:
+     *
+     * <pre>Point p2 = Verbose.print(p, x + y, 0);</pre>
+     *
+     * @param origMethod   original method
+     * @param staticMethod static method
+     */
+    public void redirectMethodCallToStatic(CtMethod origMethod,
+                                           CtMethod staticMethod) {
+        transformers = new TransformCallToStatic(transformers, origMethod,
+                staticMethod);
+    }
+
+    /**
      * Insert a call to another method before an existing method call.
      * That "before" method must be static.  The return type must be
      * <code>void</code>.  As parameters, the before method receives
diff --git a/src/main/javassist/CtBehavior.java b/src/main/javassist/CtBehavior.java
index 1b9dbf0b..a0738ec7 100644
--- a/src/main/javassist/CtBehavior.java
+++ b/src/main/javassist/CtBehavior.java
@@ -782,7 +782,7 @@ public abstract class CtBehavior extends CtMember {
                                         Modifier.isStatic(getModifiers()));
             jv.recordParamNames(ca, nvars);
             jv.recordLocalVariables(ca, 0);
-            jv.recordType(getReturnType0());
+            jv.recordReturnType(getReturnType0(), false);
             jv.compileStmnt(src);
             Bytecode b = jv.getBytecode();
             int stack = b.getMaxStack();
diff --git a/src/main/javassist/CtClass.java b/src/main/javassist/CtClass.java
index 847d9c7b..5c9ca722 100644
--- a/src/main/javassist/CtClass.java
+++ b/src/main/javassist/CtClass.java
@@ -69,7 +69,7 @@ public abstract class CtClass {
     /**
      * The version number of this release.
      */
-    public static final String version = "3.24.0-GA";
+    public static final String version = "3.24.1-GA";
 
     /**
      * Prints the version number and the copyright notice.
diff --git a/src/main/javassist/compiler/CodeGen.java b/src/main/javassist/compiler/CodeGen.java
index d4c748f8..3cb11719 100644
--- a/src/main/javassist/compiler/CodeGen.java
+++ b/src/main/javassist/compiler/CodeGen.java
@@ -542,7 +542,23 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
     }
 
     private void atSwitchStmnt(Stmnt st) throws CompileError {
+        boolean isString = false;
+        if (typeChecker != null) {
+            doTypeCheck(st.head());
+            isString = typeChecker.exprType == TypeChecker.CLASS
+                       && typeChecker.arrayDim == 0
+                       && TypeChecker.jvmJavaLangString.equals(typeChecker.className);
+        }
+
         compileExpr(st.head());
+        int tmpVar = -1;
+        if (isString) {
+            tmpVar = getMaxLocals();
+            incMaxLocals(1);
+            bytecode.addAstore(tmpVar);
+            bytecode.addAload(tmpVar);
+            bytecode.addInvokevirtual(TypeChecker.jvmJavaLangString, "hashCode", "()I");
+        }
 
         List<Integer>  prevBreakList = breakList;
         breakList = new ArrayList<Integer>();
@@ -565,6 +581,7 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         bytecode.addGap(npairs * 8);
 
         long[] pairs = new long[npairs];
+        ArrayList<Integer> gotoDefaults = new ArrayList<Integer>();
         int ipairs = 0;
         int defaultPc = -1;
         for (ASTList list = body; list != null; list = list.tail()) {
@@ -575,9 +592,18 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
             else if (op != CASE)
                 fatal();
             else {
+                int curPos = bytecode.currentPc();
+                long caseLabel;
+                if (isString) {
+                    // computeStringLabel() also adds bytecode as its side-effects.
+                    caseLabel = (long)computeStringLabel(label.head(), tmpVar, gotoDefaults);
+                }
+                else
+                    caseLabel = (long)computeLabel(label.head());
+
                 pairs[ipairs++]
-                    = ((long)computeLabel(label.head()) << 32) + 
-                      ((long)(bytecode.currentPc() - opcodePc) & 0xffffffff);
+                    = (caseLabel << 32) + 
+                      ((long)(curPos - opcodePc) & 0xffffffff);
             }
 
             hasReturned = false;
@@ -600,6 +626,8 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
             defaultPc = endPc;
 
         bytecode.write32bit(opcodePc2, defaultPc - opcodePc);
+        for (int addr: gotoDefaults)
+            bytecode.write16bit(addr, defaultPc - addr + 1);
 
         patchGoto(breakList, endPc);
         breakList = prevBreakList;
@@ -613,6 +641,26 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         throw new CompileError("bad case label");
     }
 
+    private int computeStringLabel(ASTree expr, int tmpVar, List<Integer> gotoDefaults)
+        throws CompileError
+    {
+        doTypeCheck(expr);
+        expr = TypeChecker.stripPlusExpr(expr);
+        if (expr instanceof StringL) {
+            String label = ((StringL)expr).get();
+            bytecode.addAload(tmpVar);
+            bytecode.addLdc(label);
+            bytecode.addInvokevirtual(TypeChecker.jvmJavaLangString, "equals",
+                                      "(Ljava/lang/Object;)Z");
+            bytecode.addOpcode(IFEQ);
+            Integer pc = Integer.valueOf(bytecode.currentPc());
+            bytecode.addIndex(0);
+            gotoDefaults.add(pc);
+            return (int)label.hashCode();
+        }
+        throw new CompileError("bad case label");
+    }
+
     private void atBreakStmnt(Stmnt st, boolean notCont)
         throws CompileError
     {
diff --git a/src/main/javassist/convert/TransformCallToStatic.java b/src/main/javassist/convert/TransformCallToStatic.java
new file mode 100644
index 00000000..87181edf
--- /dev/null
+++ b/src/main/javassist/convert/TransformCallToStatic.java
@@ -0,0 +1,29 @@
+package javassist.convert;
+
+import javassist.CtMethod;
+import javassist.bytecode.BadBytecode;
+import javassist.bytecode.CodeIterator;
+import javassist.bytecode.ConstPool;
+import javassist.bytecode.Descriptor;
+import javassist.bytecode.Opcode;
+
+public class TransformCallToStatic extends TransformCall {
+    public TransformCallToStatic(Transformer next, CtMethod origMethod, CtMethod substMethod) {
+        super(next, origMethod, substMethod);
+        methodDescriptor = origMethod.getMethodInfo2().getDescriptor();
+    }
+
+    @Override
+    protected int match(int c, int pos, CodeIterator iterator, int typedesc, ConstPool cp) {
+        if (newIndex == 0) {
+            String desc = Descriptor.insertParameter(classname, methodDescriptor);
+            int nt = cp.addNameAndTypeInfo(newMethodname, desc);
+            int ci = cp.addClassInfo(newClassname);
+            newIndex = cp.addMethodrefInfo(ci, nt);
+            constPool = cp;
+        }
+        iterator.writeByte(Opcode.INVOKESTATIC, pos);
+        iterator.write16bit(newIndex, pos + 1);
+        return pos;
+    }
+}
diff --git a/src/main/javassist/util/proxy/DefineClassHelper.java b/src/main/javassist/util/proxy/DefineClassHelper.java
index 96ade4aa..106e9932 100644
--- a/src/main/javassist/util/proxy/DefineClassHelper.java
+++ b/src/main/javassist/util/proxy/DefineClassHelper.java
@@ -219,9 +219,6 @@ public class DefineClassHelper {
                 if (e instanceof RuntimeException) throw (RuntimeException) e;
                 throw new CannotCompileException(e);
             }
-            finally {
-                SecurityActions.setAccessible(defineClass, false);
-            }
         }
     }
 
diff --git a/src/main/javassist/util/proxy/DefinePackageHelper.java b/src/main/javassist/util/proxy/DefinePackageHelper.java
index 8a91eb28..7ddffd9e 100644
--- a/src/main/javassist/util/proxy/DefinePackageHelper.java
+++ b/src/main/javassist/util/proxy/DefinePackageHelper.java
@@ -128,9 +128,6 @@ public class DefinePackageHelper
                 }
                 if (e instanceof RuntimeException) throw (RuntimeException) e;
             }
-            finally {
-                definePackage.setAccessible(false);
-            }
             return null;
         }
     };
diff --git a/src/main/javassist/util/proxy/ProxyFactory.java b/src/main/javassist/util/proxy/ProxyFactory.java
index 55f13059..98e352ca 100644
--- a/src/main/javassist/util/proxy/ProxyFactory.java
+++ b/src/main/javassist/util/proxy/ProxyFactory.java
@@ -472,6 +472,10 @@ public class ProxyFactory {
 
     /**
      * Generates a proxy class using the current filter.
+     * It loads a class file by the given
+     * {@code java.lang.invoke.MethodHandles.Lookup} object,
+     * which can be obtained by {@code MethodHandles.lookup()} called from
+     * somewhere in the package that the loaded class belongs to.
      *
      * @param lookup    used for loading the proxy class.
      *                  It needs an appropriate right to invoke {@code defineClass}
@@ -492,6 +496,7 @@ public class ProxyFactory {
      *                  It needs an appropriate right to invoke {@code defineClass}
      *                  for the proxy class.
      * @param filter    the filter.
+     * @see #createClass(Lookup)
      * @since 3.24
      */
     public Class<?> createClass(Lookup lookup, MethodFilter filter) {
@@ -622,7 +627,7 @@ public class ProxyFactory {
      * {@code java.lang.invoke.MethodHandles.Lookup}.
      */
     private Class<?> getClassInTheSamePackage() {
-        if (basename.startsWith("javassist.util.proxy."))       // maybe the super class is java.*
+        if (basename.startsWith(packageForJavaBase))       // maybe the super class is java.*
             return this.getClass();
         else if (superClass != null && superClass != OBJECT_TYPE)
             return superClass;
@@ -921,10 +926,15 @@ public class ProxyFactory {
         if (Modifier.isFinal(superClass.getModifiers()))
             throw new RuntimeException(superName + " is final");
 
-        if (basename.startsWith("java.") || onlyPublicMethods)
-            basename = "javassist.util.proxy." + basename.replace('.', '_');
+        // Since java.base module is not opened, its proxy class should be
+        // in a different (open) module.  Otherwise, it could not be created
+        // by reflection.
+        if (basename.startsWith("java.") || basename.startsWith("jdk.") || onlyPublicMethods)
+            basename = packageForJavaBase + basename.replace('.', '_');
     }
 
+    private static final String packageForJavaBase = "javassist.util.proxy.";
+
     private void allocateClassName() {
         classname = makeProxyName(basename);
     }
diff --git a/src/test/javassist/ConcurrentClassDefinitionTest.java b/src/test/javassist/ConcurrentClassDefinitionTest.java
new file mode 100644
index 00000000..c26360c4
--- /dev/null
+++ b/src/test/javassist/ConcurrentClassDefinitionTest.java
@@ -0,0 +1,75 @@
+package javassist;
+
+import javassist.bytecode.ClassFile;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ConcurrentClassDefinitionTest {
+
+  @Parameterized.Parameter
+  public int N;
+
+  @Parameterized.Parameters
+  public static Object[] data() {
+    return new Object[] {
+            1, // single threaded - should pass
+            Runtime.getRuntime().availableProcessors() * 2
+    };
+  }
+
+  @Test
+  public void showDefineClassRaceCondition() throws Exception{
+    Worker[] workers = new Worker[N];
+    for (int i = 0; i < N; i++) {
+      workers[i] = new Worker(N + "_ " + i, 100);
+      workers[i].start();
+    }
+    for (Worker w : workers) {
+      w.join();
+    }
+    for (Worker w : workers) {
+      if (w.e != null) {
+        throw w.e;
+      }
+    }
+  }
+
+  private static class Worker extends Thread {
+    String id;
+    int count;
+    Exception e;
+
+    Worker(String id, int count) {
+      this.id = id;
+      this.count = count;
+    }
+
+    @Override
+    public void run() {
+      try {
+        for (int i = 0; i < count; i++) {
+          Class c = makeClass(id + "_" + i);
+          assert c != null;
+        }
+      } catch (Exception e) {
+        this.e = e;
+      }
+    }
+
+    @Override
+    public void interrupt() {
+      super.interrupt();
+    }
+  }
+
+  private static Class makeClass(String id) throws Exception {
+    ClassFile cf = new ClassFile(
+            false, "com.example.JavassistGeneratedClass_" + id, null);
+    ClassPool classPool = ClassPool.getDefault();
+    return classPool.makeClass(cf).toClass();
+  }
+
+
+}
diff --git a/src/test/javassist/JvstTest3.java b/src/test/javassist/JvstTest3.java
index 46f06b16..c065170c 100644
--- a/src/test/javassist/JvstTest3.java
+++ b/src/test/javassist/JvstTest3.java
@@ -586,6 +586,20 @@ public class JvstTest3 extends JvstTestRoot {
         assertEquals(524, invoke(obj, "test"));
     }
 
+    public void testMethodRedirectToStatic() throws Exception {
+        CtClass targetClass = sloader.get("test3.MethodRedirectToStatic");
+        CtClass staticClass = sloader.get("test3.MethodRedirectToStatic2");
+        CtMethod targetMethod = targetClass.getDeclaredMethod("add");
+        CtMethod staticMethod = staticClass.getDeclaredMethod("add2");
+        CodeConverter conv = new CodeConverter();
+
+        conv.redirectMethodCallToStatic(targetMethod, staticMethod);
+        targetClass.instrument(conv);
+        targetClass.writeFile();
+        Object obj = make(targetClass.getName());
+        assertEquals(30, invoke(obj, "test"));
+    }
+
     public void testClassMap() throws Exception {
         ClassMap map = new ClassMap();
         map.put("aa", "AA");
diff --git a/src/test/javassist/JvstTest5.java b/src/test/javassist/JvstTest5.java
index c5eff4d1..915e1e1a 100644
--- a/src/test/javassist/JvstTest5.java
+++ b/src/test/javassist/JvstTest5.java
@@ -453,4 +453,49 @@ public class JvstTest5 extends JvstTestRoot {
         cc.getClassFile().compact();
         cc.toClass(test5.DefineClassCapability.class);
     }
+
+    public void testSwitchCaseWithStringConstant() throws Exception {
+        CtClass cc = sloader.get("test5.SwitchCase");
+        cc.addMethod(CtNewMethod.make(
+                "public int run() {" +
+                "    String s = \"foobar\";\n" +
+                "    switch (s) {\n" +
+                "    case STR1: return 1;\n" +
+                "    case \"foobar\": return 2;\n" +
+                "    default: return 3; }\n" +
+                "}\n", cc));
+        cc.writeFile();
+        Object obj = make(cc.getName());
+        assertEquals(2, invoke(obj, "run"));   
+    }
+
+    public void testSwitchCaseWithStringConstant2() throws Exception {
+        CtClass cc = sloader.makeClass("test5.SwitchCase2");
+        cc.addMethod(CtNewMethod.make(
+                "public int run() {" +
+                "    String s = \"foo\";\n" +
+                "    switch (s) {\n" +
+                "    case test5.SwitchCase.STR1: return 1;\n" +
+                "    case \"foobar\": return 2;\n" +
+                "    }\n" +
+                "    return 3;\n" +
+                "}\n", cc));
+        cc.writeFile();
+        Object obj = make(cc.getName());
+        assertEquals(1, invoke(obj, "run"));   
+    }
+
+    // Issue #241
+    public void testInsertBeforeAndDollarR() throws Exception {
+        CtClass cc = sloader.get(test5.InsertBeforeDollarR.class.getName());
+        CtMethod m = cc.getDeclaredMethod("foo");
+        m.insertBefore("{ if ($1 == 1) return ($r)$2; }");
+        try {
+            m.insertBefore("{ $_ = \"bar\"; }");
+            assertTrue(false);
+        } catch (CannotCompileException e) {}
+        cc.writeFile();
+        Object obj = make(cc.getName());
+        assertEquals(1, invoke(obj, "run"));
+    }
 }
diff --git a/src/test/javassist/proxyfactory/ProxyFactoryTest.java b/src/test/javassist/proxyfactory/ProxyFactoryTest.java
index 43a96bb6..c69acc9d 100644
--- a/src/test/javassist/proxyfactory/ProxyFactoryTest.java
+++ b/src/test/javassist/proxyfactory/ProxyFactoryTest.java
@@ -140,4 +140,16 @@ public class ProxyFactoryTest extends TestCase {
             }
         });
     }
+
+    public void testJava11jdk() throws Exception {
+        ProxyFactory factory = new ProxyFactory();
+        factory.setSuperclass(jdk.javadoc.doclet.StandardDoclet.class);
+        jdk.javadoc.doclet.StandardDoclet e = (jdk.javadoc.doclet.StandardDoclet)factory.create(null, null, new MethodHandler() {
+            @Override
+            public Object invoke(Object self, Method thisMethod,
+                    Method proceed, Object[] args) throws Throwable {
+                return proceed.invoke(self, args);
+            }
+        });
+    }
 }
diff --git a/src/test/test3/MethodRedirectToStatic.java b/src/test/test3/MethodRedirectToStatic.java
new file mode 100644
index 00000000..f1d68e35
--- /dev/null
+++ b/src/test/test3/MethodRedirectToStatic.java
@@ -0,0 +1,22 @@
+package test3;
+
+public class MethodRedirectToStatic {
+
+    public static void main(String[] args) {
+        System.out.println(new MethodRedirectToStatic().test());
+    }
+
+    int add(int a, int b) {
+        return a + b;
+    }
+
+    public int test() {
+        return add(1, 2);
+    }
+}
+
+class MethodRedirectToStatic2 {
+    public static int add2(MethodRedirectToStatic target, int a, int b) {
+        return target.add(a * 10, b * 10);
+    }
+}
diff --git a/src/test/test5/InsertBeforeDollarR.java b/src/test/test5/InsertBeforeDollarR.java
new file mode 100644
index 00000000..a2c32aae
--- /dev/null
+++ b/src/test/test5/InsertBeforeDollarR.java
@@ -0,0 +1,14 @@
+package test5;
+
+public class InsertBeforeDollarR {
+    public int run() {
+        if (foo(1, "baz").equals("baz"))
+            return 1;
+        else
+            return 0;
+    }
+
+    public String foo(int i, Object obj) {
+        return String.valueOf(i);
+    }
+}
diff --git a/src/test/test5/SwitchCase.java b/src/test/test5/SwitchCase.java
new file mode 100644
index 00000000..7a0ebe21
--- /dev/null
+++ b/src/test/test5/SwitchCase.java
@@ -0,0 +1,5 @@
+package test5;
+
+public class SwitchCase {
+    public static final String STR1 = "foo"; 
+}