Painless: Fix bug for static method calls on interfaces (#31348)

Static method calls on interfaces were not being called correctly
which was causing JVM crashes.  This change fixes the issue.
This commit is contained in:
Jack Conradson 2018-06-14 18:30:37 -07:00 committed by GitHub
parent d6d0727aac
commit 0324103737
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 51 additions and 11 deletions

View File

@ -376,7 +376,8 @@ public final class Def {
ref.delegateClassName, ref.delegateClassName,
ref.delegateInvokeType, ref.delegateInvokeType,
ref.delegateMethodName, ref.delegateMethodName,
ref.delegateMethodType ref.delegateMethodType,
ref.isDelegateInterface ? 1 : 0
); );
return callSite.dynamicInvoker().asType(MethodType.methodType(clazz.clazz, captures)); return callSite.dynamicInvoker().asType(MethodType.methodType(clazz.clazz, captures));
} }

View File

@ -20,6 +20,7 @@
package org.elasticsearch.painless; package org.elasticsearch.painless;
import org.elasticsearch.painless.spi.Whitelist; import org.elasticsearch.painless.spi.Whitelist;
import org.objectweb.asm.Opcodes;
import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodHandles;
@ -202,16 +203,28 @@ public final class Definition {
public void write(MethodWriter writer) { public void write(MethodWriter writer) {
final org.objectweb.asm.Type type; final org.objectweb.asm.Type type;
final Class<?> clazz;
if (augmentation != null) { if (augmentation != null) {
assert java.lang.reflect.Modifier.isStatic(modifiers); assert java.lang.reflect.Modifier.isStatic(modifiers);
clazz = augmentation;
type = org.objectweb.asm.Type.getType(augmentation); type = org.objectweb.asm.Type.getType(augmentation);
} else { } else {
clazz = owner.clazz;
type = owner.type; type = owner.type;
} }
if (java.lang.reflect.Modifier.isStatic(modifiers)) { if (java.lang.reflect.Modifier.isStatic(modifiers)) {
writer.invokeStatic(type, method); // invokeStatic assumes that the owner class is not an interface, so this is a
} else if (java.lang.reflect.Modifier.isInterface(owner.clazz.getModifiers())) { // special case for interfaces where the interface method boolean needs to be set to
// true to reference the appropriate class constant when calling a static interface
// method since java 8 did not check, but java 9 and 10 do
if (java.lang.reflect.Modifier.isInterface(clazz.getModifiers())) {
writer.visitMethodInsn(Opcodes.INVOKESTATIC,
type.getInternalName(), name, getMethodType().toMethodDescriptorString(), true);
} else {
writer.invokeStatic(type, method);
}
} else if (java.lang.reflect.Modifier.isInterface(clazz.getModifiers())) {
writer.invokeInterface(type, method); writer.invokeInterface(type, method);
} else { } else {
writer.invokeVirtual(type, method); writer.invokeVirtual(type, method);

View File

@ -66,6 +66,9 @@ public class FunctionRef {
/** delegate method type method as type */ /** delegate method type method as type */
public final Type delegateType; public final Type delegateType;
/** whether a call is made on a delegate interface */
public final boolean isDelegateInterface;
/** /**
* Creates a new FunctionRef, which will resolve {@code type::call} from the whitelist. * Creates a new FunctionRef, which will resolve {@code type::call} from the whitelist.
* @param definition the whitelist against which this script is being compiled * @param definition the whitelist against which this script is being compiled
@ -97,10 +100,13 @@ public class FunctionRef {
// the Painless$Script class can be inferred if owner is null // the Painless$Script class can be inferred if owner is null
if (delegateMethod.owner == null) { if (delegateMethod.owner == null) {
delegateClassName = CLASS_NAME; delegateClassName = CLASS_NAME;
isDelegateInterface = false;
} else if (delegateMethod.augmentation != null) { } else if (delegateMethod.augmentation != null) {
delegateClassName = delegateMethod.augmentation.getName(); delegateClassName = delegateMethod.augmentation.getName();
isDelegateInterface = delegateMethod.augmentation.isInterface();
} else { } else {
delegateClassName = delegateMethod.owner.clazz.getName(); delegateClassName = delegateMethod.owner.clazz.getName();
isDelegateInterface = delegateMethod.owner.clazz.isInterface();
} }
if ("<init>".equals(delegateMethod.name)) { if ("<init>".equals(delegateMethod.name)) {
@ -139,6 +145,7 @@ public class FunctionRef {
delegateInvokeType = H_INVOKESTATIC; delegateInvokeType = H_INVOKESTATIC;
this.delegateMethodName = delegateMethodName; this.delegateMethodName = delegateMethodName;
this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures); this.delegateMethodType = delegateMethodType.dropParameterTypes(0, numCaptures);
isDelegateInterface = false;
this.interfaceMethod = null; this.interfaceMethod = null;
delegateMethod = null; delegateMethod = null;

View File

@ -188,6 +188,10 @@ public final class LambdaBootstrap {
* @param delegateMethodName The name of the method to be called in the Painless script class * @param delegateMethodName The name of the method to be called in the Painless script class
* @param delegateMethodType The type of method call in the Painless script class without * @param delegateMethodType The type of method call in the Painless script class without
* the captured types * the captured types
* @param isDelegateInterface If the method to be called is owned by an interface where
* if the value is '1' if the delegate is an interface and '0'
* otherwise; note this is an int because the bootstrap method
* cannot convert constants to boolean
* @return A {@link CallSite} linked to a factory method for creating a lambda class * @return A {@link CallSite} linked to a factory method for creating a lambda class
* that implements the expected functional interface * that implements the expected functional interface
* @throws LambdaConversionException Thrown when an illegal type conversion occurs at link time * @throws LambdaConversionException Thrown when an illegal type conversion occurs at link time
@ -200,7 +204,8 @@ public final class LambdaBootstrap {
String delegateClassName, String delegateClassName,
int delegateInvokeType, int delegateInvokeType,
String delegateMethodName, String delegateMethodName,
MethodType delegateMethodType) MethodType delegateMethodType,
int isDelegateInterface)
throws LambdaConversionException { throws LambdaConversionException {
Loader loader = (Loader)lookup.lookupClass().getClassLoader(); Loader loader = (Loader)lookup.lookupClass().getClassLoader();
String lambdaClassName = Type.getInternalName(lookup.lookupClass()) + "$$Lambda" + loader.newLambdaIdentifier(); String lambdaClassName = Type.getInternalName(lookup.lookupClass()) + "$$Lambda" + loader.newLambdaIdentifier();
@ -225,7 +230,7 @@ public final class LambdaBootstrap {
generateInterfaceMethod(cw, factoryMethodType, lambdaClassType, interfaceMethodName, generateInterfaceMethod(cw, factoryMethodType, lambdaClassType, interfaceMethodName,
interfaceMethodType, delegateClassType, delegateInvokeType, interfaceMethodType, delegateClassType, delegateInvokeType,
delegateMethodName, delegateMethodType, captures); delegateMethodName, delegateMethodType, isDelegateInterface == 1, captures);
endLambdaClass(cw); endLambdaClass(cw);
@ -369,6 +374,7 @@ public final class LambdaBootstrap {
int delegateInvokeType, int delegateInvokeType,
String delegateMethodName, String delegateMethodName,
MethodType delegateMethodType, MethodType delegateMethodType,
boolean isDelegateInterface,
Capture[] captures) Capture[] captures)
throws LambdaConversionException { throws LambdaConversionException {
@ -434,7 +440,7 @@ public final class LambdaBootstrap {
Handle delegateHandle = Handle delegateHandle =
new Handle(delegateInvokeType, delegateClassType.getInternalName(), new Handle(delegateInvokeType, delegateClassType.getInternalName(),
delegateMethodName, delegateMethodType.toMethodDescriptorString(), delegateMethodName, delegateMethodType.toMethodDescriptorString(),
delegateInvokeType == H_INVOKEINTERFACE); isDelegateInterface);
iface.invokeDynamic(delegateMethodName, Type.getMethodType(interfaceMethodType iface.invokeDynamic(delegateMethodName, Type.getMethodType(interfaceMethodType
.toMethodDescriptorString()).getDescriptor(), DELEGATE_BOOTSTRAP_HANDLE, .toMethodDescriptorString()).getDescriptor(), DELEGATE_BOOTSTRAP_HANDLE,
delegateHandle); delegateHandle);

View File

@ -141,8 +141,8 @@ public final class WriterConstants {
/** invokedynamic bootstrap for lambda expression/method references */ /** invokedynamic bootstrap for lambda expression/method references */
public static final MethodType LAMBDA_BOOTSTRAP_TYPE = public static final MethodType LAMBDA_BOOTSTRAP_TYPE =
MethodType.methodType(CallSite.class, MethodHandles.Lookup.class, String.class, MethodType.methodType(CallSite.class, MethodHandles.Lookup.class, String.class, MethodType.class,
MethodType.class, MethodType.class, String.class, int.class, String.class, MethodType.class); MethodType.class, String.class, int.class, String.class, MethodType.class, int.class);
public static final Handle LAMBDA_BOOTSTRAP_HANDLE = public static final Handle LAMBDA_BOOTSTRAP_HANDLE =
new Handle(Opcodes.H_INVOKESTATIC, Type.getInternalName(LambdaBootstrap.class), new Handle(Opcodes.H_INVOKESTATIC, Type.getInternalName(LambdaBootstrap.class),
"lambdaBootstrap", LAMBDA_BOOTSTRAP_TYPE.toMethodDescriptorString(), false); "lambdaBootstrap", LAMBDA_BOOTSTRAP_TYPE.toMethodDescriptorString(), false);

View File

@ -121,7 +121,8 @@ public final class ECapturingFunctionRef extends AExpression implements ILambda
ref.delegateClassName, ref.delegateClassName,
ref.delegateInvokeType, ref.delegateInvokeType,
ref.delegateMethodName, ref.delegateMethodName,
ref.delegateType ref.delegateType,
ref.isDelegateInterface ? 1 : 0
); );
} }
} }

View File

@ -112,7 +112,8 @@ public final class EFunctionRef extends AExpression implements ILambda {
ref.delegateClassName, ref.delegateClassName,
ref.delegateInvokeType, ref.delegateInvokeType,
ref.delegateMethodName, ref.delegateMethodName,
ref.delegateType ref.delegateType,
ref.isDelegateInterface ? 1 : 0
); );
} else { } else {
// TODO: don't do this: its just to cutover :) // TODO: don't do this: its just to cutover :)

View File

@ -222,7 +222,8 @@ public final class ELambda extends AExpression implements ILambda {
ref.delegateClassName, ref.delegateClassName,
ref.delegateInvokeType, ref.delegateInvokeType,
ref.delegateMethodName, ref.delegateMethodName,
ref.delegateType ref.delegateType,
ref.isDelegateInterface ? 1 : 0
); );
} else { } else {
// placeholder // placeholder

View File

@ -264,6 +264,11 @@ public class BasicExpressionTests extends ScriptTestCase {
// assertEquals(null, exec("def a = ['thing': 'bar']; a.other?.cat?.dog = 'wombat'; return a.other?.cat?.dog")); // assertEquals(null, exec("def a = ['thing': 'bar']; a.other?.cat?.dog = 'wombat'; return a.other?.cat?.dog"));
} }
// test to ensure static interface methods are called correctly
public void testStaticInterfaceMethod() {
assertEquals(4, exec("def values = [1, 4, 3, 2]; values.sort(Comparator.comparing(p -> p)); return values[3]"));
}
private void assertMustBeNullable(String script) { private void assertMustBeNullable(String script) {
Exception e = expectScriptThrows(IllegalArgumentException.class, false, () -> exec(script)); Exception e = expectScriptThrows(IllegalArgumentException.class, false, () -> exec(script));
assertEquals("Result of null safe operator must be nullable", e.getMessage()); assertEquals("Result of null safe operator must be nullable", e.getMessage());

View File

@ -184,6 +184,11 @@ public class FunctionRefTests extends ScriptTestCase {
"def map = new HashMap(); f(map::getOrDefault)")); "def map = new HashMap(); f(map::getOrDefault)"));
} }
public void testInterfaceStaticMethod() {
assertEquals(-1, exec("Supplier get(Supplier supplier) { return supplier }" +
"Supplier s = get(Comparator::naturalOrder); s.get().compare(1, 2)"));
}
public void testMethodMissing() { public void testMethodMissing() {
Exception e = expectScriptThrows(IllegalArgumentException.class, () -> { Exception e = expectScriptThrows(IllegalArgumentException.class, () -> {
exec("List l = [2, 1]; l.sort(Integer::bogus); return l.get(0);"); exec("List l = [2, 1]; l.sort(Integer::bogus); return l.get(0);");