Tue, 21 Oct 2014 14:27:49 +0200
8058610: must not let long operations overflow
Reviewed-by: hannesw, jlaskey, lagergren
1.1 --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Oct 20 12:06:36 2014 +0200 1.2 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java Tue Oct 21 14:27:49 2014 +0200 1.3 @@ -552,10 +552,10 @@ 1.4 } 1.5 1.6 MethodEmitter loadBinaryOperands(final BinaryNode binaryNode) { 1.7 - return loadBinaryOperands(binaryNode.lhs(), binaryNode.rhs(), TypeBounds.UNBOUNDED.notWiderThan(binaryNode.getWidestOperandType()), false); 1.8 + return loadBinaryOperands(binaryNode.lhs(), binaryNode.rhs(), TypeBounds.UNBOUNDED.notWiderThan(binaryNode.getWidestOperandType()), false, false); 1.9 } 1.10 1.11 - private MethodEmitter loadBinaryOperands(final Expression lhs, final Expression rhs, final TypeBounds explicitOperandBounds, final boolean baseAlreadyOnStack) { 1.12 + private MethodEmitter loadBinaryOperands(final Expression lhs, final Expression rhs, final TypeBounds explicitOperandBounds, final boolean baseAlreadyOnStack, final boolean forceConversionSeparation) { 1.13 // ECMAScript 5.1 specification (sections 11.5-11.11 and 11.13) prescribes that when evaluating a binary 1.14 // expression "LEFT op RIGHT", the order of operations must be: LOAD LEFT, LOAD RIGHT, CONVERT LEFT, CONVERT 1.15 // RIGHT, EXECUTE OP. Unfortunately, doing it in this order defeats potential optimizations that arise when we 1.16 @@ -572,15 +572,34 @@ 1.17 final Type narrowestOperandType = Type.narrowest(Type.widest(lhs.getType(), rhs.getType()), explicitOperandBounds.widest); 1.18 final TypeBounds operandBounds = explicitOperandBounds.notNarrowerThan(narrowestOperandType); 1.19 if (noToPrimitiveConversion(lhs.getType(), explicitOperandBounds.widest) || rhs.isLocal()) { 1.20 - // Can reorder. Combine load and convert into single operations. 1.21 - loadExpression(lhs, operandBounds, baseAlreadyOnStack); 1.22 - loadExpression(rhs, operandBounds, false); 1.23 + // Can reorder. We might still need to separate conversion, but at least we can do it with reordering 1.24 + if (forceConversionSeparation) { 1.25 + // Can reorder, but can't move conversion into the operand as the operation depends on operands 1.26 + // exact types for its overflow guarantees. E.g. with {L}{%I}expr1 {L}* {L}{%I}expr2 we are not allowed 1.27 + // to merge {L}{%I} into {%L}, as that can cause subsequent overflows; test for JDK-8058610 contains 1.28 + // concrete cases where this could happen. 1.29 + final TypeBounds safeConvertBounds = TypeBounds.UNBOUNDED.notNarrowerThan(narrowestOperandType); 1.30 + loadExpression(lhs, safeConvertBounds, baseAlreadyOnStack); 1.31 + method.convert(operandBounds.within(method.peekType())); 1.32 + loadExpression(rhs, safeConvertBounds, false); 1.33 + method.convert(operandBounds.within(method.peekType())); 1.34 + } else { 1.35 + // Can reorder and move conversion into the operand. Combine load and convert into single operations. 1.36 + loadExpression(lhs, operandBounds, baseAlreadyOnStack); 1.37 + loadExpression(rhs, operandBounds, false); 1.38 + } 1.39 } else { 1.40 // Can't reorder. Load and convert separately. 1.41 final TypeBounds safeConvertBounds = TypeBounds.UNBOUNDED.notNarrowerThan(narrowestOperandType); 1.42 loadExpression(lhs, safeConvertBounds, baseAlreadyOnStack); 1.43 + final Type lhsType = method.peekType(); 1.44 loadExpression(rhs, safeConvertBounds, false); 1.45 - method.swap().convert(operandBounds.within(method.peekType())).swap().convert(operandBounds.within(method.peekType())); 1.46 + final Type convertedLhsType = operandBounds.within(method.peekType()); 1.47 + if (convertedLhsType != lhsType) { 1.48 + // Do it conditionally, so that if conversion is a no-op we don't introduce a SWAP, SWAP. 1.49 + method.swap().convert(convertedLhsType).swap(); 1.50 + } 1.51 + method.convert(operandBounds.within(method.peekType())); 1.52 } 1.53 assert Type.generic(method.peekType()) == operandBounds.narrowest; 1.54 assert Type.generic(method.peekType(1)) == operandBounds.narrowest; 1.55 @@ -631,19 +650,11 @@ 1.56 } 1.57 1.58 TypeBounds booleanToInt() { 1.59 - return maybeNew(booleanToInt(narrowest), booleanToInt(widest)); 1.60 + return maybeNew(CodeGenerator.booleanToInt(narrowest), CodeGenerator.booleanToInt(widest)); 1.61 } 1.62 1.63 TypeBounds objectToNumber() { 1.64 - return maybeNew(objectToNumber(narrowest), objectToNumber(widest)); 1.65 - } 1.66 - 1.67 - private static Type booleanToInt(final Type t) { 1.68 - return t == Type.BOOLEAN ? Type.INT : t; 1.69 - } 1.70 - 1.71 - private static Type objectToNumber(final Type t) { 1.72 - return t.isObject() ? Type.NUMBER : t; 1.73 + return maybeNew(CodeGenerator.objectToNumber(narrowest), CodeGenerator.objectToNumber(widest)); 1.74 } 1.75 1.76 Type within(final Type type) { 1.77 @@ -662,6 +673,14 @@ 1.78 } 1.79 } 1.80 1.81 + private static Type booleanToInt(final Type t) { 1.82 + return t == Type.BOOLEAN ? Type.INT : t; 1.83 + } 1.84 + 1.85 + private static Type objectToNumber(final Type t) { 1.86 + return t.isObject() ? Type.NUMBER : t; 1.87 + } 1.88 + 1.89 MethodEmitter loadExpressionAsType(final Expression expr, final Type type) { 1.90 if(type == Type.BOOLEAN) { 1.91 return loadExpressionAsBoolean(expr); 1.92 @@ -3543,13 +3562,15 @@ 1.93 void loadStack() { 1.94 final TypeBounds operandBounds; 1.95 final boolean isOptimistic = isValid(getProgramPoint()); 1.96 + boolean forceConversionSeparation = false; 1.97 if(isOptimistic) { 1.98 operandBounds = new TypeBounds(binaryNode.getType(), Type.OBJECT); 1.99 } else { 1.100 // Non-optimistic, non-FP +. Allow it to overflow. 1.101 operandBounds = new TypeBounds(binaryNode.getWidestOperandType(), Type.OBJECT); 1.102 + forceConversionSeparation = binaryNode.getWidestOperationType().narrowerThan(resultBounds.widest); 1.103 } 1.104 - loadBinaryOperands(binaryNode.lhs(), binaryNode.rhs(), operandBounds, false); 1.105 + loadBinaryOperands(binaryNode.lhs(), binaryNode.rhs(), operandBounds, false, forceConversionSeparation); 1.106 } 1.107 1.108 @Override 1.109 @@ -3660,12 +3681,21 @@ 1.110 @Override 1.111 protected void evaluate() { 1.112 final Expression lhs = assignNode.lhs(); 1.113 - final Type widest = assignNode.isTokenType(TokenType.ASSIGN_ADD) ? Type.OBJECT : assignNode.getWidestOperationType(); 1.114 + final Expression rhs = assignNode.rhs(); 1.115 + final Type widestOperationType = assignNode.getWidestOperationType(); 1.116 + final Type widest = assignNode.isTokenType(TokenType.ASSIGN_ADD) ? Type.OBJECT : widestOperationType; 1.117 final TypeBounds bounds = new TypeBounds(assignNode.getType(), widest); 1.118 new OptimisticOperation(assignNode, bounds) { 1.119 @Override 1.120 void loadStack() { 1.121 - loadBinaryOperands(lhs, assignNode.rhs(), bounds, true); 1.122 + final boolean forceConversionSeparation; 1.123 + if (isValid(getProgramPoint()) || widestOperationType == Type.NUMBER) { 1.124 + forceConversionSeparation = false; 1.125 + } else { 1.126 + final Type operandType = Type.widest(booleanToInt(objectToNumber(lhs.getType())), booleanToInt(objectToNumber(rhs.getType()))); 1.127 + forceConversionSeparation = operandType.narrowerThan(widestOperationType); 1.128 + } 1.129 + loadBinaryOperands(lhs, rhs, bounds, true, forceConversionSeparation); 1.130 } 1.131 @Override 1.132 void consumeStack() { 1.133 @@ -3688,7 +3718,7 @@ 1.134 1.135 @Override 1.136 protected void evaluate() { 1.137 - loadBinaryOperands(assignNode.lhs(), assignNode.rhs(), TypeBounds.UNBOUNDED.notWiderThan(assignNode.getWidestOperandType()), true); 1.138 + loadBinaryOperands(assignNode.lhs(), assignNode.rhs(), TypeBounds.UNBOUNDED.notWiderThan(assignNode.getWidestOperandType()), true, false); 1.139 op(); 1.140 } 1.141 } 1.142 @@ -3811,6 +3841,7 @@ 1.143 @Override 1.144 void loadStack() { 1.145 final TypeBounds operandBounds; 1.146 + boolean forceConversionSeparation = false; 1.147 if(numericBounds.narrowest == Type.NUMBER) { 1.148 // Result should be double always. Propagate it into the operands so we don't have lots of I2D 1.149 // and L2D after operand evaluation. 1.150 @@ -3828,9 +3859,10 @@ 1.151 // Non-optimistic, non-FP subtraction or multiplication. Allow them to overflow. 1.152 operandBounds = new TypeBounds(Type.narrowest(node.getWidestOperandType(), 1.153 numericBounds.widest), Type.NUMBER); 1.154 + forceConversionSeparation = node.getWidestOperationType().narrowerThan(numericBounds.widest); 1.155 } 1.156 } 1.157 - loadBinaryOperands(node.lhs(), node.rhs(), operandBounds, false); 1.158 + loadBinaryOperands(node.lhs(), node.rhs(), operandBounds, false, forceConversionSeparation); 1.159 } 1.160 1.161 @Override
2.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 2.2 +++ b/test/script/basic/JDK-8058610.js Tue Oct 21 14:27:49 2014 +0200 2.3 @@ -0,0 +1,77 @@ 2.4 +/* 2.5 + * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved. 2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.7 + * 2.8 + * This code is free software; you can redistribute it and/or modify it 2.9 + * under the terms of the GNU General Public License version 2 only, as 2.10 + * published by the Free Software Foundation. 2.11 + * 2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 2.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 2.15 + * version 2 for more details (a copy is included in the LICENSE file that 2.16 + * accompanied this code). 2.17 + * 2.18 + * You should have received a copy of the GNU General Public License version 2.19 + * 2 along with this work; if not, write to the Free Software Foundation, 2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 2.21 + * 2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 2.23 + * or visit www.oracle.com if you need additional information or have any 2.24 + * questions. 2.25 + */ 2.26 + 2.27 +/** 2.28 + * JDK-8058610: must not let long operations overflow 2.29 + * 2.30 + * @test 2.31 + * @run 2.32 + */ 2.33 + 2.34 +function mul(x) { 2.35 + return x.foo * x.bar; 2.36 +} 2.37 +print("=== mul ===") 2.38 +print(mul({foo: 2147483647, bar: 2147483647})); // 2^31 2.39 +print(mul({foo: 17179869184, bar: 2147483647})); // 2^34 2.40 + 2.41 +function self_mul(x) { 2.42 + return x.foo *= x.bar; 2.43 +} 2.44 +print("=== self_mul ===") 2.45 +print(self_mul({foo: 2147483647, bar: 2147483647})); // 2^31 2.46 +print(self_mul({foo: 17179869184, bar: 2147483647})); // 2^34 2.47 + 2.48 +// We'll need to use this function to obtain long values larger in 2.49 +// magnitude than those precisely representable in a double (2^53), 2.50 +// as Nashorn's parser will reify such literals as a double. For 2.51 +// overflow on add and sub we need (2^63)-1. 2.52 +var parseLong = Java.type("java.lang.Long").parseLong; 2.53 + 2.54 +function sub(x) { 2.55 + return x.foo - x.bar; 2.56 +} 2.57 +print("=== sub ===") 2.58 +print(sub({foo: 2147483647, bar: -2147483647})); // 2^31 2.59 +print(sub({foo: parseLong("9223372036854775807"), bar: parseLong("-9223372036854775807")})); // 2^63-1 2.60 + 2.61 +function self_sub(x) { 2.62 + return x.foo -= x.bar; 2.63 +} 2.64 +print("=== self_sub ===") 2.65 +print(self_sub({foo: 2147483647, bar: -2147483647})); // 2^31 2.66 +print(self_sub({foo: parseLong("9223372036854775807"), bar: parseLong("-9223372036854775807")})); // 2^63-1 2.67 + 2.68 +function add(x) { 2.69 + return x.foo + x.bar; 2.70 +} 2.71 +print("=== add ===") 2.72 +print(add({foo: 2147483647, bar: 2147483647})); // 2^31 2.73 +print(add({foo: parseLong("9223372036854775807"), bar: parseLong("9223372036854775807")})); // 2^63-1 2.74 + 2.75 +function self_add(x) { 2.76 + return x.foo += x.bar; 2.77 +} 2.78 +print("=== self_add ===") 2.79 +print(self_add({foo: 2147483647, bar: 2147483647})); // 2^31 2.80 +print(self_add({foo: parseLong("9223372036854775807"), bar: parseLong("9223372036854775807")})); // 2^63-1
3.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 3.2 +++ b/test/script/basic/JDK-8058610.js.EXPECTED Tue Oct 21 14:27:49 2014 +0200 3.3 @@ -0,0 +1,18 @@ 3.4 +=== mul === 3.5 +4611686014132420600 3.6 +36893488130239234000 3.7 +=== self_mul === 3.8 +4611686014132420600 3.9 +36893488130239234000 3.10 +=== sub === 3.11 +4294967294 3.12 +18446744073709552000 3.13 +=== self_sub === 3.14 +4294967294 3.15 +18446744073709552000 3.16 +=== add === 3.17 +4294967294 3.18 +18446744073709552000 3.19 +=== self_add === 3.20 +4294967294 3.21 +18446744073709552000