8066227: CodeGenerator load unitialized slot

Mon, 08 Dec 2014 15:14:11 +0100

author
attila
date
Mon, 08 Dec 2014 15:14:11 +0100
changeset 1118
ce989952a70b
parent 1117
74e8b730f413
child 1119
0172b56c9f4d

8066227: CodeGenerator load unitialized slot
Reviewed-by: hannesw, lagergren

src/jdk/nashorn/internal/codegen/CodeGenerator.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/ir/BinaryNode.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8066227.js file | annotate | diff | comparison | revisions
test/script/basic/JDK-8066227.js.EXPECTED file | annotate | diff | comparison | revisions
     1.1 --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Mon Dec 08 15:13:16 2014 +0100
     1.2 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Mon Dec 08 15:14:11 2014 +0100
     1.3 @@ -3697,8 +3697,7 @@
     1.4              final Expression lhs = assignNode.lhs();
     1.5              final Expression rhs = assignNode.rhs();
     1.6              final Type widestOperationType = assignNode.getWidestOperationType();
     1.7 -            final Type widest = assignNode.isTokenType(TokenType.ASSIGN_ADD) ? Type.OBJECT : widestOperationType;
     1.8 -            final TypeBounds bounds = new TypeBounds(assignNode.getType(), widest);
     1.9 +            final TypeBounds bounds = new TypeBounds(assignNode.getType(), widestOperationType);
    1.10              new OptimisticOperation(assignNode, bounds) {
    1.11                  @Override
    1.12                  void loadStack() {
     2.1 --- a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java	Mon Dec 08 15:13:16 2014 +0100
     2.2 +++ b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java	Mon Dec 08 15:14:11 2014 +0100
     2.3 @@ -28,6 +28,7 @@
     2.4  import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN;
     2.5  import static jdk.nashorn.internal.ir.Expression.isAlwaysFalse;
     2.6  import static jdk.nashorn.internal.ir.Expression.isAlwaysTrue;
     2.7 +
     2.8  import java.util.ArrayDeque;
     2.9  import java.util.ArrayList;
    2.10  import java.util.Collections;
    2.11 @@ -82,7 +83,6 @@
    2.12  import jdk.nashorn.internal.ir.VarNode;
    2.13  import jdk.nashorn.internal.ir.WhileNode;
    2.14  import jdk.nashorn.internal.ir.visitor.NodeVisitor;
    2.15 -import jdk.nashorn.internal.parser.Token;
    2.16  import jdk.nashorn.internal.parser.TokenType;
    2.17  
    2.18  /**
    2.19 @@ -398,48 +398,53 @@
    2.20  
    2.21      @Override
    2.22      public boolean enterBinaryNode(final BinaryNode binaryNode) {
    2.23 +        // NOTE: regardless of operator's lexical associativity, lhs is always evaluated first.
    2.24          final Expression lhs = binaryNode.lhs();
    2.25 -        final Expression rhs = binaryNode.rhs();
    2.26          final boolean isAssignment = binaryNode.isAssignment();
    2.27 -
    2.28 -        final TokenType tokenType = Token.descType(binaryNode.getToken());
    2.29 -        if(tokenType.isLeftAssociative()) {
    2.30 -            assert !isAssignment;
    2.31 -            final boolean isLogical = binaryNode.isLogical();
    2.32 -            final Label joinLabel = isLogical ? new Label("") : null;
    2.33 -            lhs.accept(this);
    2.34 -            if(isLogical) {
    2.35 -                jumpToLabel((JoinPredecessor)lhs, joinLabel);
    2.36 -            }
    2.37 -            rhs.accept(this);
    2.38 -            if(isLogical) {
    2.39 -                jumpToLabel((JoinPredecessor)rhs, joinLabel);
    2.40 -            }
    2.41 -            joinOnLabel(joinLabel);
    2.42 -        } else {
    2.43 -            rhs.accept(this);
    2.44 -            if(isAssignment) {
    2.45 -                if(lhs instanceof BaseNode) {
    2.46 -                    ((BaseNode)lhs).getBase().accept(this);
    2.47 -                    if(lhs instanceof IndexNode) {
    2.48 -                        ((IndexNode)lhs).getIndex().accept(this);
    2.49 -                    } else {
    2.50 -                        assert lhs instanceof AccessNode;
    2.51 -                    }
    2.52 +        LvarType lhsTypeOnLoad = null;
    2.53 +        if(isAssignment) {
    2.54 +            if(lhs instanceof BaseNode) {
    2.55 +                ((BaseNode)lhs).getBase().accept(this);
    2.56 +                if(lhs instanceof IndexNode) {
    2.57 +                    ((IndexNode)lhs).getIndex().accept(this);
    2.58                  } else {
    2.59 -                    assert lhs instanceof IdentNode;
    2.60 -                    if(binaryNode.isSelfModifying()) {
    2.61 -                        ((IdentNode)lhs).accept(this);
    2.62 -                    }
    2.63 +                    assert lhs instanceof AccessNode;
    2.64                  }
    2.65              } else {
    2.66 -                lhs.accept(this);
    2.67 +                assert lhs instanceof IdentNode;
    2.68 +                if(binaryNode.isSelfModifying()) {
    2.69 +                    final IdentNode ident = ((IdentNode)lhs);
    2.70 +                    ident.accept(this);
    2.71 +                    // Self-assignment can cause a change in the type of the variable. For purposes of evaluating
    2.72 +                    // the type of the operation, we must use its type as it was when it was loaded. If we didn't
    2.73 +                    // do this, some awkward expressions would end up being calculated incorrectly, e.g.
    2.74 +                    // "var x; x += x = 0;". In this case we have undefined+int so the result type is double (NaN).
    2.75 +                    // However, if we used the type of "x" on LHS after we evaluated RHS, we'd see int+int, so the
    2.76 +                    // result type would be either optimistic int or pessimistic long, which would be wrong.
    2.77 +                    lhsTypeOnLoad = getLocalVariableTypeIfBytecode(ident.getSymbol());
    2.78 +                }
    2.79              }
    2.80 +        } else {
    2.81 +            lhs.accept(this);
    2.82          }
    2.83  
    2.84 +        final boolean isLogical = binaryNode.isLogical();
    2.85 +        assert !(isAssignment && isLogical); // there are no logical assignment operators in JS
    2.86 +        final Label joinLabel = isLogical ? new Label("") : null;
    2.87 +        if(isLogical) {
    2.88 +            jumpToLabel((JoinPredecessor)lhs, joinLabel);
    2.89 +        }
    2.90 +
    2.91 +        final Expression rhs = binaryNode.rhs();
    2.92 +        rhs.accept(this);
    2.93 +        if(isLogical) {
    2.94 +            jumpToLabel((JoinPredecessor)rhs, joinLabel);
    2.95 +        }
    2.96 +        joinOnLabel(joinLabel);
    2.97 +
    2.98          if(isAssignment && lhs instanceof IdentNode) {
    2.99              if(binaryNode.isSelfModifying()) {
   2.100 -                onSelfAssignment((IdentNode)lhs, binaryNode);
   2.101 +                onSelfAssignment((IdentNode)lhs, binaryNode, lhsTypeOnLoad);
   2.102              } else {
   2.103                  onAssignment((IdentNode)lhs, rhs);
   2.104              }
   2.105 @@ -919,7 +924,8 @@
   2.106  
   2.107          if(unaryNode.isSelfModifying()) {
   2.108              if(expr instanceof IdentNode) {
   2.109 -                onSelfAssignment((IdentNode)expr, unaryNode);
   2.110 +                final IdentNode ident = (IdentNode)expr;
   2.111 +                onSelfAssignment(ident, unaryNode, getLocalVariableTypeIfBytecode(ident.getSymbol()));
   2.112              }
   2.113          }
   2.114          return false;
   2.115 @@ -973,12 +979,41 @@
   2.116          return types;
   2.117      }
   2.118  
   2.119 +    /**
   2.120 +     * Returns the current type of the local variable represented by the symbol. This is the most strict of all
   2.121 +     * {@code getLocalVariableType*} methods, as it will throw an assertion if the type is null. Therefore, it is only
   2.122 +     * safe to be invoked on symbols known to be bytecode locals, and only after they have been initialized.
   2.123 +     * Regardless, it is recommended to use this method in majority of cases, as because of its strictness it is the
   2.124 +     * best suited for catching missing type calculation bugs early.
   2.125 +     * @param symbol a symbol representing a bytecode local variable.
   2.126 +     * @return the current type of the local variable represented by the symbol
   2.127 +     */
   2.128      private LvarType getLocalVariableType(final Symbol symbol) {
   2.129          final LvarType type = getLocalVariableTypeOrNull(symbol);
   2.130          assert type != null;
   2.131          return type;
   2.132      }
   2.133  
   2.134 +    /**
   2.135 +     * Gets the type for a local variable if it is a bytecode local, otherwise null. Can be used in circumstances where
   2.136 +     * the type is irrelevant if the symbol is not a bytecode local. Note that for bytecode locals, it delegates to
   2.137 +     * {@link #getLocalVariableType(Symbol)}, so it will still assert that the type for such variable is already
   2.138 +     * defined (that is, not null).
   2.139 +     * @param symbol the symbol representing the variable.
   2.140 +     * @return the current variable type, if it is a bytecode local, otherwise null.
   2.141 +     */
   2.142 +    private LvarType getLocalVariableTypeIfBytecode(final Symbol symbol) {
   2.143 +        return symbol.isBytecodeLocal() ? getLocalVariableType(symbol) : null;
   2.144 +    }
   2.145 +
   2.146 +    /**
   2.147 +     * Gets the type for a variable represented by a symbol, or null if the type is not know. This is the least strict
   2.148 +     * of all local variable type getters, and as such its use is discouraged except in initialization scenarios (where
   2.149 +     * a just-defined symbol might still be null).
   2.150 +     * @param symbol the symbol
   2.151 +     * @return the current type for the symbol, or null if the type is not known either because the symbol has not been
   2.152 +     * initialized, or because the symbol does not represent a bytecode local variable.
   2.153 +     */
   2.154      private LvarType getLocalVariableTypeOrNull(final Symbol symbol) {
   2.155          return localVariableTypes.get(symbol);
   2.156      }
   2.157 @@ -1358,13 +1393,13 @@
   2.158          jumpToCatchBlock(identNode);
   2.159      }
   2.160  
   2.161 -    private void onSelfAssignment(final IdentNode identNode, final Expression assignment) {
   2.162 +    private void onSelfAssignment(final IdentNode identNode, final Expression assignment, final LvarType typeOnLoad) {
   2.163          final Symbol symbol = identNode.getSymbol();
   2.164          assert symbol != null : identNode.getName();
   2.165          if(!symbol.isBytecodeLocal()) {
   2.166              return;
   2.167          }
   2.168 -        final LvarType type = toLvarType(getType(assignment));
   2.169 +        final LvarType type = toLvarType(getType(assignment, symbol, typeOnLoad.type));
   2.170          // Self-assignment never produce either a boolean or undefined
   2.171          assert type != null && type != LvarType.UNDEFINED && type != LvarType.BOOLEAN;
   2.172          setType(symbol, type);
   2.173 @@ -1445,13 +1480,24 @@
   2.174          symbolIsUsed(symbol, getLocalVariableType(symbol));
   2.175      }
   2.176  
   2.177 +    /**
   2.178 +     * Gets the type of the expression, dependent on the current types of the local variables.
   2.179 +     *
   2.180 +     * @param expr the expression
   2.181 +     * @return the current type of the expression dependent on the current types of the local variables.
   2.182 +     */
   2.183      private Type getType(final Expression expr) {
   2.184          return expr.getType(getSymbolToType());
   2.185      }
   2.186  
   2.187 +    /**
   2.188 +     * Returns a function object from symbols to their types, used by the expressions to evaluate their type.
   2.189 +     * {@link BinaryNode} specifically uses identity of the function to cache type calculations. This method makes
   2.190 +     * sure to return the same function object while the local variable types don't change, and create a new function
   2.191 +     * object if the local variable types have been changed.
   2.192 +     * @return a function object representing a mapping from symbols to their types.
   2.193 +     */
   2.194      private Function<Symbol, Type> getSymbolToType() {
   2.195 -        // BinaryNode uses identity of the function to cache type calculations. Therefore, we must use different
   2.196 -        // function instances for different localVariableTypes instances.
   2.197          if(symbolToType.isStale()) {
   2.198              symbolToType = new SymbolToType();
   2.199          }
   2.200 @@ -1469,4 +1515,41 @@
   2.201              return boundTypes != localVariableTypes;
   2.202          }
   2.203      }
   2.204 +
   2.205 +    /**
   2.206 +     * Gets the type of the expression, dependent on the current types of the local variables and a single overridden
   2.207 +     * symbol type. Used by type calculation on compound operators to ensure the type of the LHS at the time it was
   2.208 +     * loaded (which can potentially be different after RHS evaluation, e.g. "var x; x += x = 0;") is preserved for
   2.209 +     * the calculation.
   2.210 +     *
   2.211 +     * @param expr the expression
   2.212 +     * @param overriddenSymbol the overridden symbol
   2.213 +     * @param overriddenType the overridden type
   2.214 +     * @return the current type of the expression dependent on the current types of the local variables and the single
   2.215 +     * potentially overridden type.
   2.216 +     */
   2.217 +    private Type getType(final Expression expr, final Symbol overriddenSymbol, final Type overriddenType) {
   2.218 +        return expr.getType(getSymbolToType(overriddenSymbol, overriddenType));
   2.219 +    }
   2.220 +
   2.221 +    private Function<Symbol, Type> getSymbolToType(final Symbol overriddenSymbol, final Type overriddenType) {
   2.222 +        return getLocalVariableType(overriddenSymbol).type == overriddenType ? getSymbolToType() :
   2.223 +            new SymbolToTypeOverride(overriddenSymbol, overriddenType);
   2.224 +    }
   2.225 +
   2.226 +    private class SymbolToTypeOverride implements Function<Symbol, Type> {
   2.227 +        private final Function<Symbol, Type> originalSymbolToType = getSymbolToType();
   2.228 +        private final Symbol overriddenSymbol;
   2.229 +        private final Type overriddenType;
   2.230 +
   2.231 +        SymbolToTypeOverride(final Symbol overriddenSymbol, final Type overriddenType) {
   2.232 +            this.overriddenSymbol = overriddenSymbol;
   2.233 +            this.overriddenType = overriddenType;
   2.234 +        }
   2.235 +
   2.236 +        @Override
   2.237 +        public Type apply(final Symbol symbol) {
   2.238 +            return symbol == overriddenSymbol ? overriddenType : originalSymbolToType.apply(symbol);
   2.239 +        }
   2.240 +    }
   2.241  }
     3.1 --- a/src/jdk/nashorn/internal/ir/BinaryNode.java	Mon Dec 08 15:13:16 2014 +0100
     3.2 +++ b/src/jdk/nashorn/internal/ir/BinaryNode.java	Mon Dec 08 15:14:11 2014 +0100
     3.3 @@ -341,10 +341,7 @@
     3.4      @Override
     3.5      public Node accept(final NodeVisitor<? extends LexicalContext> visitor) {
     3.6          if (visitor.enterBinaryNode(this)) {
     3.7 -            if(tokenType().isLeftAssociative()) {
     3.8 -                return visitor.leaveBinaryNode(setLHS((Expression)lhs.accept(visitor)).setRHS((Expression)rhs.accept(visitor)));
     3.9 -            }
    3.10 -            return visitor.leaveBinaryNode(setRHS((Expression)rhs.accept(visitor)).setLHS((Expression)lhs.accept(visitor)));
    3.11 +            return visitor.leaveBinaryNode(setLHS((Expression)lhs.accept(visitor)).setRHS((Expression)rhs.accept(visitor)));
    3.12          }
    3.13  
    3.14          return this;
     4.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     4.2 +++ b/test/script/basic/JDK-8066227.js	Mon Dec 08 15:14:11 2014 +0100
     4.3 @@ -0,0 +1,40 @@
     4.4 +/*
     4.5 + * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved.
     4.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     4.7 + * 
     4.8 + * This code is free software; you can redistribute it and/or modify it
     4.9 + * under the terms of the GNU General Public License version 2 only, as
    4.10 + * published by the Free Software Foundation.
    4.11 + * 
    4.12 + * This code is distributed in the hope that it will be useful, but WITHOUT
    4.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    4.14 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    4.15 + * version 2 for more details (a copy is included in the LICENSE file that
    4.16 + * accompanied this code).
    4.17 + * 
    4.18 + * You should have received a copy of the GNU General Public License version
    4.19 + * 2 along with this work; if not, write to the Free Software Foundation,
    4.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    4.21 + * 
    4.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    4.23 + * or visit www.oracle.com if you need additional information or have any
    4.24 + * questions.
    4.25 + */
    4.26 +
    4.27 +/**
    4.28 + * JDK-8066227: CodeGenerator load unitialized slot
    4.29 + *
    4.30 + * @test
    4.31 + * @run
    4.32 + */
    4.33 +
    4.34 +print((function () { var x; (x += x = 0); return x; })());
    4.35 +print((function () { var x; (x -= x = 0); return x; })());
    4.36 +print((function () { var x; (x *= x = 0); return x; })());
    4.37 +print((function () { var x; (x /= x = 0); return x; })());
    4.38 +print((function () { var x; (x %= x = 0); return x; })());
    4.39 +print((function () { var x; (x <<= x = 0); return x; })());
    4.40 +print((function () { var x; (x >>= x = 0); return x; })());
    4.41 +print((function () { var x; (x >>>= x = 0); return x; })());
    4.42 +print((function () { var x; (x |= x = 0); return x; })());
    4.43 +print((function () { var x; (x &= x = 0); return x; })());
     5.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     5.2 +++ b/test/script/basic/JDK-8066227.js.EXPECTED	Mon Dec 08 15:14:11 2014 +0100
     5.3 @@ -0,0 +1,10 @@
     5.4 +NaN
     5.5 +NaN
     5.6 +NaN
     5.7 +NaN
     5.8 +NaN
     5.9 +0
    5.10 +0
    5.11 +0
    5.12 +0
    5.13 +0

mercurial