8041995: Problems when loading tree expressions with several optimistic program points when optimistically initializing ObjectNodes
Reviewed-by: jlaskey, attila
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Apr 25 14:26:07 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Apr 28 16:37:36 2014 +0200
@@ -75,6 +75,7 @@
import java.util.RandomAccess;
import java.util.Set;
import java.util.TreeMap;
+import java.util.function.Supplier;
import jdk.nashorn.internal.codegen.ClassEmitter.Flag;
import jdk.nashorn.internal.codegen.CompilerConstants.Call;
@@ -1898,6 +1899,37 @@
return false;
}
+ /**
+ * Check if a property value contains a particular program point
+ * @param value value
+ * @param pp program point
+ * @return true if it's there.
+ */
+ private static boolean propertyValueContains(final Expression value, final int pp) {
+ return new Supplier<Boolean>() {
+ boolean contains;
+
+ @Override
+ public Boolean get() {
+ value.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
+ @Override
+ public boolean enterDefault(final Node node) {
+ if (contains) {
+ return false;
+ }
+ if (node instanceof Optimistic && ((Optimistic)node).getProgramPoint() == pp) {
+ contains = true;
+ return false;
+ }
+ return true;
+ }
+ });
+
+ return contains;
+ }
+ }.get();
+ }
+
@Override
public boolean enterObjectNode(final ObjectNode objectNode) {
final List<PropertyNode> elements = objectNode.getElements();
@@ -1926,7 +1958,7 @@
value != null &&
isValid(ccp) &&
value instanceof Optimistic &&
- ((Optimistic)value).getProgramPoint() == ccp;
+ propertyValueContains(value, ccp);
//for literals, a value of null means object type, i.e. the value null or getter setter function
//(I think)
@@ -1949,11 +1981,14 @@
}};
}
oc.makeObject(method);
+
//if this is a rest of method and our continuation point was found as one of the values
//in the properties above, we need to reset the map to oc.getMap() in the continuation
//handler
if (restOfProperty) {
- getContinuationInfo().setObjectLiteralMap(oc.getMap());
+ final ContinuationInfo ci = getContinuationInfo();
+ ci.setObjectLiteralMap(oc.getMap());
+ ci.setObjectLiteralStackDepth(method.getStackSize());
}
method.dup();
@@ -4460,6 +4495,8 @@
private Type returnValueType;
// If we are in the middle of an object literal initialization, we need to update the map
private PropertyMap objectLiteralMap;
+ // Object literal stack depth for object literal - not necessarly top if property is a tree
+ private int objectLiteralStackDepth = -1;
ContinuationInfo() {
this.handlerLabel = new Label("continuation_handler");
@@ -4513,6 +4550,14 @@
this.returnValueType = returnValueType;
}
+ int getObjectLiteralStackDepth() {
+ return objectLiteralStackDepth;
+ }
+
+ void setObjectLiteralStackDepth(final int objectLiteralStackDepth) {
+ this.objectLiteralStackDepth = objectLiteralStackDepth;
+ }
+
PropertyMap getObjectLiteralMap() {
return objectLiteralMap;
}
@@ -4573,20 +4618,21 @@
// Store the RewriteException into an unused local variable slot.
method.store(rewriteExceptionType, lvarCount);
// Load arguments on the stack
+ final int objectLiteralStackDepth = ci.getObjectLiteralStackDepth();
for(int i = 0; i < stackStoreSpec.length; ++i) {
final int slot = stackStoreSpec[i];
method.load(lvarTypes[slot], slot);
method.convert(stackTypes[i]);
- }
-
- // stack: s0=object literal being initialized
- // change map of s0 so that the property we are initilizing when we failed
- // is now ci.returnValueType
- if (ci.getObjectLiteralMap() != null) {
- method.dup(); //dup script object
- assert ScriptObject.class.isAssignableFrom(method.peekType().getTypeClass()) : method.peekType().getTypeClass() + " is not a script object";
- loadConstant(ci.getObjectLiteralMap());
- method.invoke(ScriptObject.SET_MAP);
+ // stack: s0=object literal being initialized
+ // change map of s0 so that the property we are initilizing when we failed
+ // is now ci.returnValueType
+ if (i == objectLiteralStackDepth) {
+ method.dup();
+ assert ci.getObjectLiteralMap() != null;
+ assert ScriptObject.class.isAssignableFrom(method.peekType().getTypeClass()) : method.peekType().getTypeClass() + " is not a script object";
+ loadConstant(ci.getObjectLiteralMap());
+ method.invoke(ScriptObject.SET_MAP);
+ }
}
// Load RewriteException back; get rid of the stored reference.
--- a/nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Fri Apr 25 14:26:07 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Apr 28 16:37:36 2014 +0200
@@ -352,13 +352,6 @@
return (isProgram ? program : extractFunctionFromScript(program)).setName(null, functionName).setSourceURL(null, sourceURL);
}
- private static String getShortDescriptor(final Object value) {
- if (value == null || !value.getClass().isPrimitive() || value.getClass() != Boolean.class) {
- return "O";
- }
- return value.getClass().getSimpleName();
- }
-
private static String stringifyInvalidations(final Map<Integer, Type> ipp) {
if (ipp == null) {
return "";
@@ -367,10 +360,11 @@
final Iterator<Map.Entry<Integer, Type>> iter = ipp.entrySet().iterator();
while (iter.hasNext()) {
final Map.Entry<Integer, Type> entry = iter.next();
+ final char bct = entry.getValue().getBytecodeStackType();
sb.append('[').
append(entry.getKey()).
append("->").
- append(getShortDescriptor(entry.getValue())).
+ append(bct == 'A' ? 'O' : bct).
append(']');
if (iter.hasNext()) {
sb.append(' ');
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8041995.js Mon Apr 28 16:37:36 2014 +0200
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8041995: optimistic object property maps were only updated if the outermost program
+ * point in a property setter failed, not an inner one, which is wrong.
+ *
+ * @test
+ * @run
+ */
+
+function xyzzy() {
+ return 17.4711;
+}
+var obj = {
+ z: -xyzzy()
+};
+print(obj.z);
+
+function phlug() {
+ var obj = {
+ 4: -Infinity,
+ 5: Infinity,
+ length: 5 - Math.pow(2, 32)
+ };
+
+ return Array.prototype.lastIndexOf.call(obj, -Infinity) === 4;
+}
+
+var d = new Date;
+print(phlug());
+var d2 = new Date - d;
+print(d2 < 5000); // if this takes more than five seconds we have read the double length as an int
+
+function wrong() {
+ var obj = {
+ length1: 5 - Math.pow(2, 32),
+ length2: 4 - Math.pow(2, 32),
+ length3: 3 - Math.pow(2, 32),
+ length4: 2 - Math.pow(2, 32),
+ length5: 1 - Math.pow(2, 32),
+ length6: Math.pow(2, 32)
+ };
+ for (var i in obj) {
+ print(obj[i]);
+ }
+}
+
+wrong();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8041995.js.EXPECTED Mon Apr 28 16:37:36 2014 +0200
@@ -0,0 +1,9 @@
+-17.4711
+true
+true
+-4294967291
+-4294967292
+-4294967293
+-4294967294
+-4294967295
+4294967296