Fix certain bad casts in Painless due to boxing/unboxing. (#23282)

This commit is contained in:
Jack Conradson 2017-02-21 10:23:27 -08:00 committed by GitHub
parent 7475175957
commit fac2d954e3
5 changed files with 330 additions and 239 deletions

View File

@ -87,6 +87,7 @@ public final class Definition {
public static final Type CHAR_OBJ_TYPE = getType("Character"); public static final Type CHAR_OBJ_TYPE = getType("Character");
public static final Type OBJECT_TYPE = getType("Object"); public static final Type OBJECT_TYPE = getType("Object");
public static final Type DEF_TYPE = getType("def"); public static final Type DEF_TYPE = getType("def");
public static final Type NUMBER_TYPE = getType("Number");
public static final Type STRING_TYPE = getType("String"); public static final Type STRING_TYPE = getType("String");
public static final Type EXCEPTION_TYPE = getType("Exception"); public static final Type EXCEPTION_TYPE = getType("Exception");
public static final Type PATTERN_TYPE = getType("Pattern"); public static final Type PATTERN_TYPE = getType("Pattern");
@ -434,23 +435,23 @@ public final class Definition {
public final Type from; public final Type from;
public final Type to; public final Type to;
public final boolean explicit; public final boolean explicit;
public final boolean unboxFrom; public final Type unboxFrom;
public final boolean unboxTo; public final Type unboxTo;
public final boolean boxFrom; public final Type boxFrom;
public final boolean boxTo; public final Type boxTo;
public Cast(final Type from, final Type to, final boolean explicit) { public Cast(final Type from, final Type to, final boolean explicit) {
this.from = from; this.from = from;
this.to = to; this.to = to;
this.explicit = explicit; this.explicit = explicit;
this.unboxFrom = false; this.unboxFrom = null;
this.unboxTo = false; this.unboxTo = null;
this.boxFrom = false; this.boxFrom = null;
this.boxTo = false; this.boxTo = null;
} }
public Cast(final Type from, final Type to, final boolean explicit, public Cast(final Type from, final Type to, final boolean explicit,
final boolean unboxFrom, final boolean unboxTo, final boolean boxFrom, final boolean boxTo) { final Type unboxFrom, final Type unboxTo, final Type boxFrom, final Type boxTo) {
this.from = from; this.from = from;
this.to = to; this.to = to;
this.explicit = explicit; this.explicit = explicit;

View File

@ -131,51 +131,48 @@ public final class MethodWriter extends GeneratorAdapter {
public void writeCast(final Cast cast) { public void writeCast(final Cast cast) {
if (cast != null) { if (cast != null) {
final Type from = cast.from; if (cast.from.sort == Sort.CHAR && cast.to.sort == Sort.STRING) {
final Type to = cast.to;
if (from.sort == Sort.CHAR && to.sort == Sort.STRING) {
invokeStatic(UTILITY_TYPE, CHAR_TO_STRING); invokeStatic(UTILITY_TYPE, CHAR_TO_STRING);
} else if (from.sort == Sort.STRING && to.sort == Sort.CHAR) { } else if (cast.from.sort == Sort.STRING && cast.to.sort == Sort.CHAR) {
invokeStatic(UTILITY_TYPE, STRING_TO_CHAR); invokeStatic(UTILITY_TYPE, STRING_TO_CHAR);
} else if (cast.unboxFrom) { } else if (cast.unboxFrom != null) {
if (from.sort == Sort.DEF) { unbox(cast.unboxFrom.type);
writeCast(cast.from, cast.to);
} else if (cast.unboxTo != null) {
if (cast.from.sort == Sort.DEF) {
if (cast.explicit) { if (cast.explicit) {
if (to.sort == Sort.BOOL) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BOOLEAN); if (cast.to.sort == Sort.BOOL_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BOOLEAN);
else if (to.sort == Sort.BYTE) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BYTE_EXPLICIT); else if (cast.to.sort == Sort.BYTE_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BYTE_EXPLICIT);
else if (to.sort == Sort.SHORT) invokeStatic(DEF_UTIL_TYPE, DEF_TO_SHORT_EXPLICIT); else if (cast.to.sort == Sort.SHORT_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_SHORT_EXPLICIT);
else if (to.sort == Sort.CHAR) invokeStatic(DEF_UTIL_TYPE, DEF_TO_CHAR_EXPLICIT); else if (cast.to.sort == Sort.CHAR_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_CHAR_EXPLICIT);
else if (to.sort == Sort.INT) invokeStatic(DEF_UTIL_TYPE, DEF_TO_INT_EXPLICIT); else if (cast.to.sort == Sort.INT_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_INT_EXPLICIT);
else if (to.sort == Sort.LONG) invokeStatic(DEF_UTIL_TYPE, DEF_TO_LONG_EXPLICIT); else if (cast.to.sort == Sort.LONG_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_LONG_EXPLICIT);
else if (to.sort == Sort.FLOAT) invokeStatic(DEF_UTIL_TYPE, DEF_TO_FLOAT_EXPLICIT); else if (cast.to.sort == Sort.FLOAT_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_FLOAT_EXPLICIT);
else if (to.sort == Sort.DOUBLE) invokeStatic(DEF_UTIL_TYPE, DEF_TO_DOUBLE_EXPLICIT); else if (cast.to.sort == Sort.DOUBLE_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_DOUBLE_EXPLICIT);
else throw new IllegalStateException("Illegal tree structure."); else throw new IllegalStateException("Illegal tree structure.");
} else { } else {
if (to.sort == Sort.BOOL) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BOOLEAN); if (cast.to.sort == Sort.BOOL_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BOOLEAN);
else if (to.sort == Sort.BYTE) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BYTE_IMPLICIT); else if (cast.to.sort == Sort.BYTE_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_BYTE_IMPLICIT);
else if (to.sort == Sort.SHORT) invokeStatic(DEF_UTIL_TYPE, DEF_TO_SHORT_IMPLICIT); else if (cast.to.sort == Sort.SHORT_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_SHORT_IMPLICIT);
else if (to.sort == Sort.CHAR) invokeStatic(DEF_UTIL_TYPE, DEF_TO_CHAR_IMPLICIT); else if (cast.to.sort == Sort.CHAR_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_CHAR_IMPLICIT);
else if (to.sort == Sort.INT) invokeStatic(DEF_UTIL_TYPE, DEF_TO_INT_IMPLICIT); else if (cast.to.sort == Sort.INT_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_INT_IMPLICIT);
else if (to.sort == Sort.LONG) invokeStatic(DEF_UTIL_TYPE, DEF_TO_LONG_IMPLICIT); else if (cast.to.sort == Sort.LONG_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_LONG_IMPLICIT);
else if (to.sort == Sort.FLOAT) invokeStatic(DEF_UTIL_TYPE, DEF_TO_FLOAT_IMPLICIT); else if (cast.to.sort == Sort.FLOAT_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_FLOAT_IMPLICIT);
else if (to.sort == Sort.DOUBLE) invokeStatic(DEF_UTIL_TYPE, DEF_TO_DOUBLE_IMPLICIT); else if (cast.to.sort == Sort.DOUBLE_OBJ) invokeStatic(DEF_UTIL_TYPE, DEF_TO_DOUBLE_IMPLICIT);
else throw new IllegalStateException("Illegal tree structure."); else throw new IllegalStateException("Illegal tree structure.");
} }
} else { } else {
unbox(from.type); writeCast(cast.from, cast.to);
writeCast(from, to); unbox(cast.unboxTo.type);
} }
} else if (cast.unboxTo) { } else if (cast.boxFrom != null) {
writeCast(from, to); box(cast.boxFrom.type);
unbox(to.type); writeCast(cast.from, cast.to);
} else if (cast.boxFrom) { } else if (cast.boxTo != null) {
box(from.type); writeCast(cast.from, cast.to);
writeCast(from, to); box(cast.boxTo.type);
} else if (cast.boxTo) {
writeCast(from, to);
box(to.type);
} else { } else {
writeCast(from, to); writeCast(cast.from, cast.to);
} }
} }
} }
@ -269,19 +266,19 @@ public final class MethodWriter extends GeneratorAdapter {
} }
/** Writes a dynamic binary instruction: returnType, lhs, and rhs can be different */ /** Writes a dynamic binary instruction: returnType, lhs, and rhs can be different */
public void writeDynamicBinaryInstruction(Location location, Type returnType, Type lhs, Type rhs, public void writeDynamicBinaryInstruction(Location location, Type returnType, Type lhs, Type rhs,
Operation operation, int flags) { Operation operation, int flags) {
org.objectweb.asm.Type methodType = org.objectweb.asm.Type.getMethodType(returnType.type, lhs.type, rhs.type); org.objectweb.asm.Type methodType = org.objectweb.asm.Type.getMethodType(returnType.type, lhs.type, rhs.type);
switch (operation) { switch (operation) {
case MUL: case MUL:
invokeDefCall("mul", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("mul", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case DIV: case DIV:
invokeDefCall("div", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("div", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case REM: case REM:
invokeDefCall("rem", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("rem", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case ADD: case ADD:
// if either side is primitive, then the + operator should always throw NPE on null, // if either side is primitive, then the + operator should always throw NPE on null,
@ -294,31 +291,31 @@ public final class MethodWriter extends GeneratorAdapter {
invokeDefCall("add", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("add", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case SUB: case SUB:
invokeDefCall("sub", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("sub", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case LSH: case LSH:
invokeDefCall("lsh", methodType, DefBootstrap.SHIFT_OPERATOR, flags); invokeDefCall("lsh", methodType, DefBootstrap.SHIFT_OPERATOR, flags);
break; break;
case USH: case USH:
invokeDefCall("ush", methodType, DefBootstrap.SHIFT_OPERATOR, flags); invokeDefCall("ush", methodType, DefBootstrap.SHIFT_OPERATOR, flags);
break; break;
case RSH: case RSH:
invokeDefCall("rsh", methodType, DefBootstrap.SHIFT_OPERATOR, flags); invokeDefCall("rsh", methodType, DefBootstrap.SHIFT_OPERATOR, flags);
break; break;
case BWAND: case BWAND:
invokeDefCall("and", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("and", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case XOR: case XOR:
invokeDefCall("xor", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("xor", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
case BWOR: case BWOR:
invokeDefCall("or", methodType, DefBootstrap.BINARY_OPERATOR, flags); invokeDefCall("or", methodType, DefBootstrap.BINARY_OPERATOR, flags);
break; break;
default: default:
throw location.createError(new IllegalStateException("Illegal tree structure.")); throw location.createError(new IllegalStateException("Illegal tree structure."));
} }
} }
/** Writes a static binary instruction */ /** Writes a static binary instruction */
public void writeBinaryInstruction(Location location, Type type, Operation operation) { public void writeBinaryInstruction(Location location, Type type, Operation operation) {
final Sort sort = type.sort; final Sort sort = type.sort;

View File

@ -21,7 +21,7 @@ package org.elasticsearch.painless;
/** Tests for explicit casts */ /** Tests for explicit casts */
public class CastTests extends ScriptTestCase { public class CastTests extends ScriptTestCase {
/** /**
* Unary operator with explicit cast * Unary operator with explicit cast
*/ */
@ -34,7 +34,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(5L, exec("long x = 5L; return (long) (+x);")); assertEquals(5L, exec("long x = 5L; return (long) (+x);"));
assertEquals(5D, exec("long x = 5L; return (double) (+x);")); assertEquals(5D, exec("long x = 5L; return (double) (+x);"));
} }
/** /**
* Binary operators with explicit cast * Binary operators with explicit cast
*/ */
@ -73,7 +73,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(6L, exec("long x = 5L; return (long) (++x);")); assertEquals(6L, exec("long x = 5L; return (long) (++x);"));
assertEquals(6D, exec("long x = 5L; return (double) (++x);")); assertEquals(6D, exec("long x = 5L; return (double) (++x);"));
} }
/** /**
* Binary compound postifx with explicit cast * Binary compound postifx with explicit cast
*/ */
@ -86,7 +86,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(5L, exec("long x = 5L; return (long) (x++);")); assertEquals(5L, exec("long x = 5L; return (long) (x++);"));
assertEquals(5D, exec("long x = 5L; return (double) (x++);")); assertEquals(5D, exec("long x = 5L; return (double) (x++);"));
} }
/** /**
* Shift operators with explicit cast * Shift operators with explicit cast
*/ */
@ -99,7 +99,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(10L, exec("long x = 5L; return (long) (x << 1);")); assertEquals(10L, exec("long x = 5L; return (long) (x << 1);"));
assertEquals(10D, exec("long x = 5L; return (double) (x << 1);")); assertEquals(10D, exec("long x = 5L; return (double) (x << 1);"));
} }
/** /**
* Shift compound assignment with explicit cast * Shift compound assignment with explicit cast
*/ */
@ -112,7 +112,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(10L, exec("long x = 5L; return (long) (x <<= 1);")); assertEquals(10L, exec("long x = 5L; return (long) (x <<= 1);"));
assertEquals(10D, exec("long x = 5L; return (double) (x <<= 1);")); assertEquals(10D, exec("long x = 5L; return (double) (x <<= 1);"));
} }
/** /**
* Test that without a cast, we fail when conversions would narrow. * Test that without a cast, we fail when conversions would narrow.
*/ */
@ -136,7 +136,7 @@ public class CastTests extends ScriptTestCase {
exec("long x = 5L; boolean y = (x + x); return y"); exec("long x = 5L; boolean y = (x + x); return y");
}); });
} }
/** /**
* Test that even with a cast, some things aren't allowed. * Test that even with a cast, some things aren't allowed.
*/ */
@ -161,7 +161,7 @@ public class CastTests extends ScriptTestCase {
public void testMethodCallDef() { public void testMethodCallDef() {
assertEquals(5, exec("def x = 5; return (int)x.longValue();")); assertEquals(5, exec("def x = 5; return (int)x.longValue();"));
} }
/** /**
* Currently these do not adopt the argument value, we issue a separate cast! * Currently these do not adopt the argument value, we issue a separate cast!
*/ */
@ -170,7 +170,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(6, exec("def x = 5; def y = 1L; return x + (int)y")); assertEquals(6, exec("def x = 5; def y = 1L; return x + (int)y"));
assertEquals('b', exec("def x = 'abcdeg'; def y = 1L; x.charAt((int)y)")); assertEquals('b', exec("def x = 'abcdeg'; def y = 1L; x.charAt((int)y)"));
} }
/** /**
* Unary operators adopt the return value * Unary operators adopt the return value
*/ */
@ -183,7 +183,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(5L, exec("def x = 5L; return (long) (+x);")); assertEquals(5L, exec("def x = 5L; return (long) (+x);"));
assertEquals(5D, exec("def x = 5L; return (double) (+x);")); assertEquals(5D, exec("def x = 5L; return (double) (+x);"));
} }
/** /**
* Binary operators adopt the return value * Binary operators adopt the return value
*/ */
@ -196,7 +196,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(6L, exec("def x = 5L; return (long) (x + 1);")); assertEquals(6L, exec("def x = 5L; return (long) (x + 1);"));
assertEquals(6D, exec("def x = 5L; return (double) (x + 1);")); assertEquals(6D, exec("def x = 5L; return (double) (x + 1);"));
} }
/** /**
* Binary operators don't yet adopt the return value with compound assignment * Binary operators don't yet adopt the return value with compound assignment
*/ */
@ -209,7 +209,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(6L, exec("def x = 5L; return (long) (x += 1);")); assertEquals(6L, exec("def x = 5L; return (long) (x += 1);"));
assertEquals(6D, exec("def x = 5L; return (double) (x += 1);")); assertEquals(6D, exec("def x = 5L; return (double) (x += 1);"));
} }
/** /**
* Binary operators don't yet adopt the return value with compound assignment * Binary operators don't yet adopt the return value with compound assignment
*/ */
@ -222,7 +222,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(6L, exec("def x = 5L; return (long) (++x);")); assertEquals(6L, exec("def x = 5L; return (long) (++x);"));
assertEquals(6D, exec("def x = 5L; return (double) (++x);")); assertEquals(6D, exec("def x = 5L; return (double) (++x);"));
} }
/** /**
* Binary operators don't yet adopt the return value with compound assignment * Binary operators don't yet adopt the return value with compound assignment
*/ */
@ -235,7 +235,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(5L, exec("def x = 5L; return (long) (x++);")); assertEquals(5L, exec("def x = 5L; return (long) (x++);"));
assertEquals(5D, exec("def x = 5L; return (double) (x++);")); assertEquals(5D, exec("def x = 5L; return (double) (x++);"));
} }
/** /**
* Shift operators adopt the return value * Shift operators adopt the return value
*/ */
@ -248,7 +248,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(10L, exec("def x = 5L; return (long) (x << 1);")); assertEquals(10L, exec("def x = 5L; return (long) (x << 1);"));
assertEquals(10D, exec("def x = 5L; return (double) (x << 1);")); assertEquals(10D, exec("def x = 5L; return (double) (x << 1);"));
} }
/** /**
* Shift operators don't yet adopt the return value with compound assignment * Shift operators don't yet adopt the return value with compound assignment
*/ */
@ -261,7 +261,7 @@ public class CastTests extends ScriptTestCase {
assertEquals(10L, exec("def x = 5L; return (long) (x <<= 1);")); assertEquals(10L, exec("def x = 5L; return (long) (x <<= 1);"));
assertEquals(10D, exec("def x = 5L; return (double) (x <<= 1);")); assertEquals(10D, exec("def x = 5L; return (double) (x <<= 1);"));
} }
/** /**
* Test that without a cast, we fail when conversions would narrow. * Test that without a cast, we fail when conversions would narrow.
*/ */
@ -285,7 +285,21 @@ public class CastTests extends ScriptTestCase {
exec("def x = 5L; boolean y = (x + x); return y"); exec("def x = 5L; boolean y = (x + x); return y");
}); });
} }
public void testUnboxMethodParameters() {
assertEquals('a', exec("'a'.charAt(Integer.valueOf(0))"));
}
public void testIllegalCastInMethodArgument() {
assertEquals('a', exec("'a'.charAt(0)"));
Exception e = expectScriptThrows(ClassCastException.class, () -> exec("'a'.charAt(0L)"));
assertEquals("Cannot cast from [long] to [int].", e.getMessage());
e = expectScriptThrows(ClassCastException.class, () -> exec("'a'.charAt(0.0f)"));
assertEquals("Cannot cast from [float] to [int].", e.getMessage());
e = expectScriptThrows(ClassCastException.class, () -> exec("'a'.charAt(0.0d)"));
assertEquals("Cannot cast from [double] to [int].", e.getMessage());
}
/** /**
* Test that even with a cast, some things aren't allowed. * Test that even with a cast, some things aren't allowed.
* (stuff that methodhandles explicitCastArguments would otherwise allow) * (stuff that methodhandles explicitCastArguments would otherwise allow)

View File

@ -54,6 +54,18 @@ public class FunctionTests extends ScriptTestCase {
assertThat(expected.getMessage(), containsString("Cannot generate an empty function")); assertThat(expected.getMessage(), containsString("Cannot generate an empty function"));
} }
public void testReturnsAreUnboxedIfNeeded() {
assertEquals((byte) 5, exec("byte get() {Byte.valueOf(5)} get()"));
assertEquals((short) 5, exec("short get() {Byte.valueOf(5)} get()"));
assertEquals(5, exec("int get() {Byte.valueOf(5)} get()"));
assertEquals((short) 5, exec("short get() {Short.valueOf(5)} get()"));
assertEquals(5, exec("int get() {Integer.valueOf(5)} get()"));
assertEquals(5.0f, exec("float get() {Float.valueOf(5)} get()"));
assertEquals(5.0d, exec("double get() {Float.valueOf(5)} get()"));
assertEquals(5.0d, exec("double get() {Double.valueOf(5)} get()"));
assertEquals(true, exec("boolean get() {Boolean.TRUE} get()"));
}
public void testDuplicates() { public void testDuplicates() {
Exception expected = expectScriptThrows(IllegalArgumentException.class, () -> { Exception expected = expectScriptThrows(IllegalArgumentException.class, () -> {
exec("void test(int x) {x = 2;} void test(def y) {y = 3;} test()"); exec("void test(int x) {x = 2;} void test(def y) {y = 3;} test()");
@ -61,6 +73,15 @@ public class FunctionTests extends ScriptTestCase {
assertThat(expected.getMessage(), containsString("Duplicate functions")); assertThat(expected.getMessage(), containsString("Duplicate functions"));
} }
public void testBadCastFromMethod() {
Exception e = expectScriptThrows(ClassCastException.class, () -> exec("int get() {5L} get()"));
assertEquals("Cannot cast from [long] to [int].", e.getMessage());
e = expectScriptThrows(ClassCastException.class, () -> exec("int get() {5.1f} get()"));
assertEquals("Cannot cast from [float] to [int].", e.getMessage());
e = expectScriptThrows(ClassCastException.class, () -> exec("int get() {5.1d} get()"));
assertEquals("Cannot cast from [double] to [int].", e.getMessage());
}
public void testInfiniteLoop() { public void testInfiniteLoop() {
Error expected = expectScriptThrows(PainlessError.class, () -> { Error expected = expectScriptThrows(PainlessError.class, () -> {
exec("void test() {boolean x = true; while (x) {}} test()"); exec("void test() {boolean x = true; while (x) {}} test()");