You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2019/11/01 09:12:10 UTC

[groovy] branch master updated: GROOVY-9298: Eliminate some illegal access warnings when indy is enabled (#1058)

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 662e812  GROOVY-9298: Eliminate some illegal access warnings when indy is enabled (#1058)
662e812 is described below

commit 662e812177efd3182acaae26b6a0f0ad954115cf
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Fri Nov 1 17:12:00 2019 +0800

    GROOVY-9298: Eliminate some illegal access warnings when indy is enabled (#1058)
    
    * Show detailed messages for illegal access warnings
    
    * Fix illegal access warning `java.util.HashMap$Node.toString()`
    
    * Fix illegal access warning `java.math.BigInteger.multiply(long)`
    
    * Fix illegal access warning `java.lang.invoke.MethodHandles$Lookup(java.lang.Class,int)`
    
    * Throw GroovyRuntimeException instead
---
 gradle/test.gradle                                 |  4 +--
 .../org/codehaus/groovy/vmplugin/v7/Selector.java  | 15 +++++----
 .../org/codehaus/groovy/vmplugin/v9/Java9.java     | 37 ++++++++++++----------
 src/test/groovy/bugs/groovy9081/Groovy9081.groovy  | 16 +++++++++-
 4 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/gradle/test.gradle b/gradle/test.gradle
index 4d3d282..e9257b3 100644
--- a/gradle/test.gradle
+++ b/gradle/test.gradle
@@ -19,8 +19,8 @@
 allprojects {
     tasks.withType(Test) {
         def jdk8 = ['-XX:+UseConcMarkSweepGC']
-        def jdk9 = ['-Djava.locale.providers=COMPAT,SPI']
-//        def jdk9 = ['-Djava.locale.providers=COMPAT,SPI', '--illegal-access=debug']
+//        def jdk9 = ['-Djava.locale.providers=COMPAT,SPI']
+        def jdk9 = ['-Djava.locale.providers=COMPAT,SPI', '--illegal-access=debug']
         def common = ['-ea', "-Xms${groovyJUnit_ms}", "-Xmx${groovyJUnit_mx}", "-Duser.language=en" ]
         if (JavaVersion.current().isJava9Compatible()) {
             jvmArgs (*common, *jdk9)
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java
index 225bb5c..c686a78 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java
@@ -50,6 +50,7 @@ import org.codehaus.groovy.runtime.metaclass.NewInstanceMetaMethod;
 import org.codehaus.groovy.runtime.metaclass.NewStaticMetaMethod;
 import org.codehaus.groovy.runtime.metaclass.ReflectionMetaMethod;
 import org.codehaus.groovy.runtime.wrappers.Wrapper;
+import org.codehaus.groovy.vmplugin.VMPluginFactory;
 import org.codehaus.groovy.vmplugin.v7.IndyInterface.CALL_TYPES;
 
 import java.lang.invoke.MethodHandle;
@@ -395,9 +396,10 @@ public abstract class Selector {
          * For a constructor call we always use the static meta class from the registry
          */
         @Override
-        public void getMetaClass() {
+        public MetaClass getMetaClass() {
             Object receiver = args[0];
             mc = GroovySystem.getMetaClassRegistry().getMetaClass((Class) receiver);
+            return mc;
         }
 
         /**
@@ -554,7 +556,7 @@ public abstract class Selector {
         /**
          * Gives the meta class to an Object.
          */
-        public void getMetaClass() {
+        public MetaClass getMetaClass() {
             Object receiver = args[0];
             if (receiver == null) {
                 mc = NullObject.getNullObject().getMetaClass();
@@ -573,6 +575,8 @@ public abstract class Selector {
                 this.cache &= !ClassInfo.getClassInfo(receiver.getClass()).hasPerInstanceMetaClasses();
             }
             mc.initialize();
+
+            return mc;
         }
 
         /**
@@ -606,10 +610,8 @@ public abstract class Selector {
             MetaMethod metaMethod = method;
             isCategoryMethod = method instanceof CategoryMethod;
 
-            if (
-                    metaMethod instanceof NumberNumberMetaMethod ||
-                    (method instanceof GeneratedMetaMethod && (name.equals("next") || name.equals("previous")))
-            ) {
+            if (metaMethod instanceof NumberNumberMetaMethod
+                    || (method instanceof GeneratedMetaMethod && (name.equals("next") || name.equals("previous")))) {
                 if (LOG_ENABLED) LOG.info("meta method is number method");
                 if (IndyMath.chooseMathMethod(this, metaMethod)) {
                     catchException = false;
@@ -632,6 +634,7 @@ public abstract class Selector {
             if (metaMethod instanceof CachedMethod) {
                 if (LOG_ENABLED) LOG.info("meta method is CachedMethod instance");
                 CachedMethod cm = (CachedMethod) metaMethod;
+                cm = (CachedMethod) VMPluginFactory.getPlugin().transformMetaMethod(getMetaClass(), cm, cm.getPT(), Selector.class);
                 isVargs = cm.isVargsMethod();
                 try {
                     Method m = cm.getCachedMethod();
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java b/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java
index 39c507b..a1970c9 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java
@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.vmplugin.v9;
 
+import groovy.lang.GroovyRuntimeException;
 import groovy.lang.MetaClass;
 import groovy.lang.MetaMethod;
 import groovy.lang.Tuple;
@@ -92,14 +93,15 @@ public class Java9 extends Java8 {
 
     public static MethodHandles.Lookup of(final Class<?> declaringClass) {
         try {
-            if (getPrivateLookup() != null) {
-                return MethodHandles.Lookup.class.cast(getPrivateLookup().invoke(null, declaringClass, MethodHandles.lookup()));
+            final Method privateLookup = getPrivateLookup();
+            if (privateLookup != null) {
+                return (MethodHandles.Lookup) privateLookup.invoke(null, declaringClass, MethodHandles.lookup());
             }
             return getLookupConstructor().newInstance(declaringClass, MethodHandles.Lookup.PRIVATE).in(declaringClass);
         } catch (final IllegalAccessException | InstantiationException e) {
             throw new IllegalArgumentException(e);
         } catch (final InvocationTargetException e) {
-            throw new RuntimeException(e);
+            throw new GroovyRuntimeException(e);
         }
     }
 
@@ -110,15 +112,12 @@ public class Java9 extends Java8 {
 
     @Override
     public Object getInvokeSpecialHandle(Method method, Object receiver) {
-        if (getLookupConstructor() != null) {
-            Class declaringClass = method.getDeclaringClass();
-            try {
-                return of(declaringClass).unreflectSpecial(method, receiver.getClass()).bindTo(receiver);
-            } catch (ReflectiveOperationException e) {
-                throw new GroovyBugError(e);
-            }
+        final Class<?> receiverType = receiver.getClass();
+        try {
+            return of(receiverType).unreflectSpecial(method, receiverType).bindTo(receiver);
+        } catch (ReflectiveOperationException e) {
+            return super.getInvokeSpecialHandle(method, receiver);
         }
-        return super.getInvokeSpecialHandle(method, receiver);
     }
 
     /**
@@ -235,12 +234,16 @@ public class Java9 extends Java8 {
             return metaMethod;
         }
 
-        if (MULTIPLY.equals(metaMethod.getName())
-                && (1 == params.length && (Integer.class == params[0] || Long.class == params[0] || Short.class == params[0]))) {
-            try {
-                return new CachedMethod(BigInteger.class.getDeclaredMethod(MULTIPLY, BigInteger.class));
-            } catch (NoSuchMethodException e) {
-                throw new GroovyBugError("Failed to transform " + MULTIPLY + " method of BigInteger", e);
+        if (1 == params.length && MULTIPLY.equals(metaMethod.getName())) {
+            Class<?> param = params[0];
+            if (Long.class == param || long.class == param
+                    || Integer.class == param || int.class == param
+                    || Short.class == param || short.class == param) {
+                try {
+                    return new CachedMethod(BigInteger.class.getDeclaredMethod(MULTIPLY, BigInteger.class));
+                } catch (NoSuchMethodException e) {
+                    throw new GroovyBugError("Failed to transform " + MULTIPLY + " method of BigInteger", e);
+                }
             }
         }
 
diff --git a/src/test/groovy/bugs/groovy9081/Groovy9081.groovy b/src/test/groovy/bugs/groovy9081/Groovy9081.groovy
index 0eb8666..4762d24 100644
--- a/src/test/groovy/bugs/groovy9081/Groovy9081.groovy
+++ b/src/test/groovy/bugs/groovy9081/Groovy9081.groovy
@@ -19,7 +19,6 @@
 package groovy.bugs.groovy9081
 
 import groovy.bugs.groovy9081.somepkg.ProtectedConstructor
-import org.junit.Ignore
 import org.junit.Test
 
 import java.awt.Font
@@ -69,4 +68,19 @@ final class Groovy9081 {
     void testAsType2() {
         [run: {}] as ProtectedConstructor
     }
+
+    @Test
+    void testAccessPackagePrivateInnerClassMember() {
+        def m = new HashMap()
+        m.a = 69
+        m.entrySet().iterator().next().toString()
+    }
+
+    @Test
+    void testAccessPackagePrivateMethod() {
+        BigInteger a = 333
+        int b = 2
+        BigDecimal c = a * b
+        assert c == 666
+    }
 }