stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8026397: Fix ambiguity with array conversion, including passing JS NativeArrays in Java variable arity methods' vararg array position

Browse Source 
Reviewed-by: jlaskey, sundar
This commit is contained in:
Attila Szegedi 2013-10-15 15:57:14 +02:00
parent 3538d0af55
commit 8a727caa63
7 changed files with 203 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
nashorn/src/jdk/internal/dynalink/beans/SingleDynamicMethod.java
View File

@ -91,6 +91,7 @@ import java.util.StringTokenizer;
import jdk.internal.dynalink.CallSiteDescriptor;
import jdk.internal.dynalink.linker.LinkerServices;
import jdk.internal.dynalink.support.Guards;
import jdk.internal.dynalink.support.Lookup;
/**
* Base class for dynamic methods that dispatch to a single target Java method or constructor. Handles adaptation of the
@ -100,6 +101,9 @@ import jdk.internal.dynalink.support.Guards;
* @version $Id: $
*/
abstract class SingleDynamicMethod extends DynamicMethod {
private static final MethodHandle CAN_CONVERT_TO = Lookup.findOwnStatic(MethodHandles.lookup(), "canConvertTo", boolean.class, LinkerServices.class, Class.class, Object.class);
SingleDynamicMethod(String name) {
super(name);
}
@ -201,23 +205,69 @@ abstract class SingleDynamicMethod extends DynamicMethod {
return createConvertingInvocation(target, linkerServices, callSiteType).asVarargsCollector(
callSiteLastArgType);
}
if(!linkerServices.canConvert(callSiteLastArgType, varArgType)) {
// Call site signature guarantees the argument can definitely not be an array (i.e. it is primitive);
// link immediately to a vararg-packing method handle.
return createConvertingInvocation(collectArguments(fixTarget, argsLen), linkerServices, callSiteType);
// This method handle takes the single argument and packs it into a newly allocated single-element array. It
// will be used when the incoming argument can't be converted to the vararg array type (the "vararg packer"
// method).
final MethodHandle varArgCollectingInvocation = createConvertingInvocation(collectArguments(fixTarget,
argsLen), linkerServices, callSiteType);
// Is call site type assignable from an array type (e.g. Object:int[], or Object[]:String[])
final boolean isAssignableFromArray = callSiteLastArgType.isAssignableFrom(varArgType);
// Do we have a custom conversion that can potentially convert the call site type to an array?
final boolean isCustomConvertible = linkerServices.canConvert(callSiteLastArgType, varArgType);
if(!isAssignableFromArray && !isCustomConvertible) {
// Call site signature guarantees the argument can definitely not be converted to an array (i.e. it is
// primitive), and no conversion can help with it either. Link immediately to a vararg-packing method
// handle.
return varArgCollectingInvocation;
}
// Call site signature makes no guarantees that the single argument in the vararg position will be
// compatible across all invocations. Need to insert an appropriate guard and fall back to generic vararg
// method when it is not.
return MethodHandles.guardWithTest(Guards.isInstance(varArgType, fixParamsLen, callSiteType),
createConvertingInvocation(fixTarget, linkerServices, callSiteType),
createConvertingInvocation(collectArguments(fixTarget, argsLen), linkerServices, callSiteType));
// This method handle employs language-specific conversions to convert the last argument into an array of
// vararg type.
final MethodHandle arrayConvertingInvocation = createConvertingInvocation(MethodHandles.filterArguments(
fixTarget, fixParamsLen, linkerServices.getTypeConverter(callSiteLastArgType, varArgType)),
linkerServices, callSiteType);
// This method handle determines whether the value can be converted to the array of vararg type using a
// language-specific conversion.
final MethodHandle canConvertArgToArray = MethodHandles.insertArguments(CAN_CONVERT_TO, 0, linkerServices,
varArgType);
// This one adjusts the previous one for the location of the argument and the call site type.
final MethodHandle canConvertLastArgToArray = MethodHandles.dropArguments(canConvertArgToArray, 0,
MethodType.genericMethodType(fixParamsLen).parameterList()).asType(callSiteType.changeReturnType(boolean.class));
// This one takes the previous ones and combines them into a method handle that converts the argument into
// a vararg array when it can, otherwise falls back to the vararg packer.
final MethodHandle convertToArrayWhenPossible = MethodHandles.guardWithTest(canConvertLastArgToArray,
arrayConvertingInvocation, varArgCollectingInvocation);
if(isAssignableFromArray) {
return MethodHandles.guardWithTest(
// Is incoming parameter already a compatible array?
Guards.isInstance(varArgType, fixParamsLen, callSiteType),
// Yes: just pass it to the method
createConvertingInvocation(fixTarget, linkerServices, callSiteType),
// No: either go through a custom conversion, or if it is not possible, go directly to the
// vararg packer.
isCustomConvertible ? convertToArrayWhenPossible : varArgCollectingInvocation);
}
// Just do the custom conversion with fallback to the vararg packer logic.
assert isCustomConvertible;
return convertToArrayWhenPossible;
}
// Remaining case: more than one vararg.
return createConvertingInvocation(collectArguments(fixTarget, argsLen), linkerServices, callSiteType);
}
@SuppressWarnings("unused")
private static boolean canConvertTo(final LinkerServices linkerServices, Class<?> to, Object obj) {
return obj == null ? false : linkerServices.canConvert(obj.getClass(), to);
}
/**
* Creates a method handle out of the original target that will collect the varargs for the exact component type of
* the varArg array. Note that this will nicely trigger language-specific type converters for exactly those varargs

13
nashorn/src/jdk/internal/dynalink/support/Guards.java
View File

@ -88,6 +88,7 @@ import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.logging.Level;
import java.util.logging.Logger;
import jdk.internal.dynalink.DynamicLinker;
import jdk.internal.dynalink.linker.LinkerServices;
/**
@ -115,11 +116,11 @@ public class Guards {
public static MethodHandle isOfClass(Class<?> clazz, MethodType type) {
final Class<?> declaredType = type.parameterType(0);
if(clazz == declaredType) {
LOG.log(Level.WARNING, "isOfClassGuardAlwaysTrue", new Object[] { clazz.getName(), 0, type });
LOG.log(Level.WARNING, "isOfClassGuardAlwaysTrue", new Object[] { clazz.getName(), 0, type, DynamicLinker.getLinkedCallSiteLocation() });
return constantTrue(type);
}
if(!declaredType.isAssignableFrom(clazz)) {
LOG.log(Level.WARNING, "isOfClassGuardAlwaysFalse", new Object[] { clazz.getName(), 0, type });
LOG.log(Level.WARNING, "isOfClassGuardAlwaysFalse", new Object[] { clazz.getName(), 0, type, DynamicLinker.getLinkedCallSiteLocation() });
return constantFalse(type);
}
return getClassBoundArgumentTest(IS_OF_CLASS, clazz, 0, type);
@ -152,11 +153,11 @@ public class Guards {
public static MethodHandle isInstance(Class<?> clazz, int pos, MethodType type) {
final Class<?> declaredType = type.parameterType(pos);
if(clazz.isAssignableFrom(declaredType)) {
LOG.log(Level.WARNING, "isInstanceGuardAlwaysTrue", new Object[] { clazz.getName(), pos, type });
LOG.log(Level.WARNING, "isInstanceGuardAlwaysTrue", new Object[] { clazz.getName(), pos, type, DynamicLinker.getLinkedCallSiteLocation() });
return constantTrue(type);
}
if(!declaredType.isAssignableFrom(clazz)) {
LOG.log(Level.WARNING, "isInstanceGuardAlwaysFalse", new Object[] { clazz.getName(), pos, type });
LOG.log(Level.WARNING, "isInstanceGuardAlwaysFalse", new Object[] { clazz.getName(), pos, type, DynamicLinker.getLinkedCallSiteLocation() });
return constantFalse(type);
}
return getClassBoundArgumentTest(IS_INSTANCE, clazz, pos, type);
@ -174,11 +175,11 @@ public class Guards {
public static MethodHandle isArray(int pos, MethodType type) {
final Class<?> declaredType = type.parameterType(pos);
if(declaredType.isArray()) {
LOG.log(Level.WARNING, "isArrayGuardAlwaysTrue", new Object[] { pos, type });
LOG.log(Level.WARNING, "isArrayGuardAlwaysTrue", new Object[] { pos, type, DynamicLinker.getLinkedCallSiteLocation() });
return constantTrue(type);
}
if(!declaredType.isAssignableFrom(Object[].class)) {
LOG.log(Level.WARNING, "isArrayGuardAlwaysFalse", new Object[] { pos, type });
LOG.log(Level.WARNING, "isArrayGuardAlwaysFalse", new Object[] { pos, type, DynamicLinker.getLinkedCallSiteLocation() });
return constantFalse(type);
}
return asType(IS_ARRAY, pos, type);

12
nashorn/src/jdk/internal/dynalink/support/messages.properties
View File

@ -76,11 +76,11 @@
# OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
# ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
isInstanceGuardAlwaysTrue=isInstance guard for {0} in position {1} in method type {2} will always return true
isInstanceGuardAlwaysFalse=isInstance guard for {0} in position {1} in method type {2} will always return false
isInstanceGuardAlwaysTrue=isInstance guard for {0} in position {1} in method type {2} at {3} will always return true
isInstanceGuardAlwaysFalse=isInstance guard for {0} in position {1} in method type {2} at {3} will always return false
isOfClassGuardAlwaysTrue=isOfClass guard for {0} in position {1} in method type {2} will always return true
isOfClassGuardAlwaysFalse=isOfClass guard for {0} in position {1} in method type {2} will always return false
isOfClassGuardAlwaysTrue=isOfClass guard for {0} in position {1} in method type {2} at {3} will always return true
isOfClassGuardAlwaysFalse=isOfClass guard for {0} in position {1} in method type {2} at {3} will always return false
isArrayGuardAlwaysTrue=isArray guard in position {0} in method type {1} will always return true
isArrayGuardAlwaysFalse=isArray guard in position {0} in method type {1} will always return false
isArrayGuardAlwaysTrue=isArray guard in position {0} in method type {1} at {2} will always return true
isArrayGuardAlwaysFalse=isArray guard in position {0} in method type {1} at {2} will always return false

6
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
View File

@ -453,7 +453,13 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
public boolean enterFunctionNode(FunctionNode functionNode) {
// function nodes will always leave a constructed function object on stack, no need to load the symbol
// separately as in enterDefault()
lc.pop(functionNode);
functionNode.accept(codegen);
// NOTE: functionNode.accept() will produce a different FunctionNode that we discard. This incidentally
// doesn't cause problems as we're never touching FunctionNode again after it's visited here - codegen
// is the last element in the compilation pipeline, the AST it produces is not used externally. So, we
// re-push the original functionNode.
lc.push(functionNode);
method.convert(type);
return false;
}

19
nashorn/src/jdk/nashorn/internal/runtime/linker/JavaArgumentConverters.java
View File

@ -31,10 +31,8 @@ import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.util.Deque;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.HashMap;
import java.util.Map;
import jdk.internal.dynalink.support.TypeUtilities;
import jdk.nashorn.internal.runtime.ConsString;
import jdk.nashorn.internal.runtime.JSType;
@ -59,13 +57,7 @@ final class JavaArgumentConverters {
}
static MethodHandle getConverter(final Class<?> targetType) {
MethodHandle converter = CONVERTERS.get(targetType);
if(converter == null && targetType.isArray()) {
converter = MH.insertArguments(JSType.TO_JAVA_ARRAY.methodHandle(), 1, targetType.getComponentType());
converter = MH.asType(converter, converter.type().changeReturnType(targetType));
CONVERTERS.putIfAbsent(targetType, converter);
}
return converter;
return CONVERTERS.get(targetType);
}
@SuppressWarnings("unused")
@ -263,12 +255,9 @@ final class JavaArgumentConverters {
return MH.findStatic(MethodHandles.lookup(), JavaArgumentConverters.class, name, MH.type(rtype, types));
}
private static final ConcurrentMap<Class<?>, MethodHandle> CONVERTERS = new ConcurrentHashMap<>();
private static final Map<Class<?>, MethodHandle> CONVERTERS = new HashMap<>();
static {
CONVERTERS.put(List.class, JSType.TO_JAVA_LIST.methodHandle());
CONVERTERS.put(Deque.class, JSType.TO_JAVA_DEQUE.methodHandle());
CONVERTERS.put(Number.class, TO_NUMBER);
CONVERTERS.put(String.class, TO_STRING);

76
nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java
View File

@ -30,6 +30,8 @@ import static jdk.nashorn.internal.lookup.Lookup.MH;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Modifier;
import java.util.Deque;
import java.util.List;
import jdk.internal.dynalink.CallSiteDescriptor;
import jdk.internal.dynalink.linker.ConversionComparator;
import jdk.internal.dynalink.linker.GuardedInvocation;
@ -38,6 +40,8 @@ import jdk.internal.dynalink.linker.LinkRequest;
import jdk.internal.dynalink.linker.LinkerServices;
import jdk.internal.dynalink.linker.TypeBasedGuardingDynamicLinker;
import jdk.internal.dynalink.support.Guards;
import jdk.nashorn.internal.objects.NativeArray;
import jdk.nashorn.internal.runtime.JSType;
import jdk.nashorn.internal.runtime.ScriptFunction;
import jdk.nashorn.internal.runtime.ScriptObject;
import jdk.nashorn.internal.runtime.Undefined;
@ -47,6 +51,13 @@ import jdk.nashorn.internal.runtime.Undefined;
* includes {@link ScriptFunction} and its subclasses) as well as {@link Undefined}.
*/
final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTypeConverterFactory, ConversionComparator {
private static final ClassValue<MethodHandle> ARRAY_CONVERTERS = new ClassValue<MethodHandle>() {
@Override
protected MethodHandle computeValue(Class<?> type) {
return createArrayConverter(type);
}
};
/**
* Returns true if {@code ScriptObject} is assignable from {@code type}, or it is {@code Undefined}.
*/
@ -103,6 +114,12 @@ final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTyp
if (mh != null) {
return new GuardedInvocation(mh, canLinkTypeStatic(sourceType) ? null : IS_NASHORN_OR_UNDEFINED_TYPE);
}
GuardedInvocation inv = getArrayConverter(sourceType, targetType);
if(inv != null) {
return inv;
}
return getSamTypeConverter(sourceType, targetType);
}
@ -129,6 +146,41 @@ final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTyp
return null;
}
/**
* Returns a guarded invocation that converts from a source type that is NativeArray to a Java array or List or
* Deque type.
* @param sourceType the source type (presumably NativeArray a superclass of it)
* @param targetType the target type (presumably an array type, or List or Deque)
* @return a guarded invocation that converts from the source type to the target type. null is returned if
* either the source type is neither NativeArray, nor a superclass of it, or if the target type is not an array
* type, List, or Deque.
*/
private static GuardedInvocation getArrayConverter(final Class<?> sourceType, final Class<?> targetType) {
final boolean isSourceTypeNativeArray = sourceType == NativeArray.class;
// If source type is more generic than ScriptFunction class, we'll need to use a guard
final boolean isSourceTypeGeneric = !isSourceTypeNativeArray && sourceType.isAssignableFrom(NativeArray.class);
if (isSourceTypeNativeArray || isSourceTypeGeneric) {
final MethodHandle guard = isSourceTypeGeneric ? IS_NATIVE_ARRAY : null;
if(targetType.isArray()) {
return new GuardedInvocation(ARRAY_CONVERTERS.get(targetType), guard);
}
if(targetType == List.class) {
return new GuardedInvocation(JSType.TO_JAVA_LIST.methodHandle(), guard);
}
if(targetType == Deque.class) {
return new GuardedInvocation(JSType.TO_JAVA_DEQUE.methodHandle(), guard);
}
}
return null;
}
private static MethodHandle createArrayConverter(final Class<?> type) {
assert type.isArray();
final MethodHandle converter = MH.insertArguments(JSType.TO_JAVA_ARRAY.methodHandle(), 1, type.getComponentType());
return MH.asType(converter, converter.type().changeReturnType(type));
}
private static boolean isAutoConvertibleFromFunction(final Class<?> clazz) {
return isAbstractClass(clazz) && !ScriptObject.class.isAssignableFrom(clazz) &&
JavaAdapterFactory.isAutoConvertibleFromFunction(clazz);
@ -148,7 +200,26 @@ final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTyp
@Override
public Comparison compareConversion(final Class<?> sourceType, final Class<?> targetType1, final Class<?> targetType2) {
if(sourceType == NativeArray.class) {
// Prefer lists, as they're less costly to create than arrays.
if(isList(targetType1)) {
if(!isList(targetType2)) {
return Comparison.TYPE_1_BETTER;
}
} else if(isList(targetType2)) {
return Comparison.TYPE_2_BETTER;
}
// Then prefer arrays
if(targetType1.isArray()) {
if(!targetType2.isArray()) {
return Comparison.TYPE_1_BETTER;
}
} else if(targetType2.isArray()) {
return Comparison.TYPE_2_BETTER;
}
}
if(ScriptObject.class.isAssignableFrom(sourceType)) {
// Prefer interfaces
if(targetType1.isInterface()) {
if(!targetType2.isInterface()) {
return Comparison.TYPE_1_BETTER;
@ -160,7 +231,12 @@ final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTyp
return Comparison.INDETERMINATE;
}
private static boolean isList(Class<?> clazz) {
return clazz == List.class || clazz == Deque.class;
}
private static final MethodHandle IS_SCRIPT_FUNCTION = Guards.isInstance(ScriptFunction.class, MH.type(Boolean.TYPE, Object.class));
private static final MethodHandle IS_NATIVE_ARRAY = Guards.isOfClass(NativeArray.class, MH.type(Boolean.TYPE, Object.class));
private static final MethodHandle IS_NASHORN_OR_UNDEFINED_TYPE = findOwnMH("isNashornTypeOrUndefined",
Boolean.TYPE, Object.class);

44
nashorn/test/src/jdk/nashorn/api/javaaccess/ArrayConversionTest.java
View File

@ -86,11 +86,27 @@ public class ArrayConversionTest {
runTest("assertBooleanArrayConversions", "[false, true, '', 'false', 0, 1, 0.4, 0.6, {}, [], [false], [true], NaN, Infinity, null, undefined]");
}
@Test
public void testArrayAmbiguity() throws ScriptException {
runTest("x", "'abc'");
runTest("x", "['foo', 'bar']");
}
@Test
public void testListArrays() throws ScriptException {
runTest("assertListArray", "[['foo', 'bar'], ['apple', 'orange']]");
}
@Test
public void testVarArgs() throws ScriptException {
// Sole NativeArray in vararg position becomes vararg array itself
runTest("assertVarArg_42_17", "[42, 17]");
// NativeArray in vararg position becomes an argument if there are more arguments
runTest("assertVarArg_array_17", "[42], 18");
// Only NativeArray is converted to vararg array, other objects (e.g. a function) aren't
runTest("assertVarArg_function", "function() { return 'Hello' }");
}
private static void runTest(final String testMethodName, final String argument) throws ScriptException {
e.eval("Java.type('" + ArrayConversionTest.class.getName() + "')." + testMethodName + "(" + argument + ")");
}
@ -188,4 +204,32 @@ public class ArrayConversionTest {
assertEquals(Arrays.asList("foo", "bar"), array[0]);
assertEquals(Arrays.asList("apple", "orange"), array[1]);
}
public static void assertVarArg_42_17(Object... args) throws ScriptException {
assertEquals(2, args.length);
assertEquals(42, ((Number)args[0]).intValue());
assertEquals(17, ((Number)args[1]).intValue());
}
public static void assertVarArg_array_17(Object... args) throws ScriptException {
assertEquals(2, args.length);
e.getBindings(ScriptContext.ENGINE_SCOPE).put("arr", args[0]);
assertTrue((Boolean)e.eval("arr instanceof Array && arr.length == 1 && arr[0] == 42"));
assertEquals(18, ((Number)args[1]).intValue());
}
public static void assertVarArg_function(Object... args) throws ScriptException {
assertEquals(1, args.length);
e.getBindings(ScriptContext.ENGINE_SCOPE).put("fn", args[0]);
assertEquals("Hello", e.eval("fn()"));
}
public static void x(String y) {
assertEquals("abc", y);
}
public static void x(String[] y) {
assertTrue(Arrays.equals(new String[] { "foo", "bar"}, y));
}
}