stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8036127: Prototype filter needs to be applied to getter guard as well, not just getter

Browse Source 
This manifests itself as a bug in optimistic types, as inner functions may access properties of the wrong type, but it is also a bug in tip. Without optimistic types, we have been unable to find a reproducer due to more similar PropertyMaps

Reviewed-by: attila, jlaskey, sundar
This commit is contained in:
Marcus Lagergren 2014-03-05 09:51:00 +01:00
parent 8d4fc394e0
commit 9d927196e8
11 changed files with 61 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

BIN
nashorn/bin/fastCatchCombinator.jar Normal file
View File

Binary file not shown.

2
nashorn/bin/runoptdualcatch.sh
View File

@ -4,7 +4,7 @@
FILENAME="./optimistic_dual_catch_$(date|sed "s/ /_/g"|sed "s/:/_/g").jfr"
DIR=/Users/marcus/src/tip/
DIR=..
FAST_CATCH_COMBINATOR=$DIR/bin/fastCatchCombinator.jar
NASHORN_JAR=$DIR/dist/nashorn.jar

2
nashorn/src/jdk/nashorn/internal/codegen/ClassEmitter.java
View File

@ -386,7 +386,7 @@ public class ClassEmitter implements Emitter {
return Context.getContext();
}
});
TraceClassVisitor tcv = new TraceClassVisitor(null, new NashornTextifier(ctx.getEnv(), cr), pw);
final TraceClassVisitor tcv = new TraceClassVisitor(null, new NashornTextifier(ctx.getEnv(), cr), pw);
cr.accept(tcv, 0);
}

1
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
View File

@ -1320,7 +1320,6 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
lc.nextFreeSlot(block);
final boolean isFunctionBody = lc.isFunctionBody();
final FunctionNode function = lc.getCurrentFunction();
if (isFunctionBody) {
if (method.hasScope()) {

11
nashorn/src/jdk/nashorn/internal/codegen/FieldObjectCreator.java
View File

@ -101,7 +101,6 @@ public abstract class FieldObjectCreator<T> extends ObjectCreator<T> {
@Override
protected void makeObject(final MethodEmitter method) {
makeMap();
final String className = getClassName();
try {
// NOTE: we must load the actual structure class here, because the API operates with Nashorn Type objects,
@ -166,14 +165,16 @@ public abstract class FieldObjectCreator<T> extends ObjectCreator<T> {
private void putField(final MethodEmitter method, final String key, final int fieldIndex, final MapTuple<T> tuple) {
method.dup();
loadTuple(method, tuple);
final boolean isPrimitive = tuple.isPrimitive();
final Type fieldType = isPrimitive ? PRIMITIVE_FIELD_TYPE : Type.OBJECT;
final Type fieldType = tuple.isPrimitive() ? PRIMITIVE_FIELD_TYPE : Type.OBJECT;
final String fieldClass = getClassName();
final String fieldName = getFieldName(fieldIndex, fieldType);
final String fieldDesc = typeDescriptor(fieldType.getTypeClass());
assert fieldName.equals(getFieldName(fieldIndex, PRIMITIVE_FIELD_TYPE)) || fieldType.isObject() : key + " object keys must store to L*-fields";
assert fieldName.equals(getFieldName(fieldIndex, Type.OBJECT)) || fieldType.isPrimitive() : key + " primitive keys must store to J*-fields";
loadTuple(method, tuple);
method.putField(fieldClass, fieldName, fieldDesc);
}

26
nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
View File

@ -306,16 +306,16 @@ public final class ObjectClassGenerator {
addFields(classEmitter, fieldCount);
final MethodEmitter init = newInitMethod(classEmitter);
final MethodEmitter init = newInitMethod(className, classEmitter);
init.returnVoid();
init.end();
final MethodEmitter initWithSpillArrays = newInitWithSpillArraysMethod(classEmitter, ScriptObject.class);
final MethodEmitter initWithSpillArrays = newInitWithSpillArraysMethod(className, classEmitter, ScriptObject.class);
initWithSpillArrays.returnVoid();
initWithSpillArrays.end();
newEmptyInit(classEmitter, className);
newAllocate(classEmitter, className);
newEmptyInit(className, classEmitter);
newAllocate(className, classEmitter);
return toByteArray(classEmitter);
}
@ -335,17 +335,17 @@ public final class ObjectClassGenerator {
final ClassEmitter classEmitter = newClassEmitter(className, superName);
final List<String> initFields = addFields(classEmitter, fieldCount);
final MethodEmitter init = newInitScopeMethod(classEmitter);
final MethodEmitter init = newInitScopeMethod(className, classEmitter);
initializeToUndefined(init, className, initFields);
init.returnVoid();
init.end();
final MethodEmitter initWithSpillArrays = newInitWithSpillArraysMethod(classEmitter, FunctionScope.class);
final MethodEmitter initWithSpillArrays = newInitWithSpillArraysMethod(className, classEmitter, FunctionScope.class);
initializeToUndefined(initWithSpillArrays, className, initFields);
initWithSpillArrays.returnVoid();
initWithSpillArrays.end();
final MethodEmitter initWithArguments = newInitScopeWithArgumentsMethod(classEmitter);
final MethodEmitter initWithArguments = newInitScopeWithArgumentsMethod(className, classEmitter);
initializeToUndefined(initWithArguments, className, initFields);
initWithArguments.returnVoid();
initWithArguments.end();
@ -399,7 +399,7 @@ public final class ObjectClassGenerator {
*
* @return Open method emitter.
*/
private static MethodEmitter newInitMethod(final ClassEmitter classEmitter) {
private static MethodEmitter newInitMethod(final String className, final ClassEmitter classEmitter) {
final MethodEmitter init = classEmitter.init(PropertyMap.class);
init.begin();
init.load(Type.OBJECT, JAVA_THIS.slot());
@ -409,7 +409,7 @@ public final class ObjectClassGenerator {
return init;
}
private static MethodEmitter newInitWithSpillArraysMethod(final ClassEmitter classEmitter, final Class<?> superClass) {
private static MethodEmitter newInitWithSpillArraysMethod(final String className, final ClassEmitter classEmitter, final Class<?> superClass) {
final MethodEmitter init = classEmitter.init(PropertyMap.class, long[].class, Object[].class);
init.begin();
init.load(Type.OBJECT, JAVA_THIS.slot());
@ -426,7 +426,7 @@ public final class ObjectClassGenerator {
* @param classEmitter Open class emitter.
* @return Open method emitter.
*/
private static MethodEmitter newInitScopeMethod(final ClassEmitter classEmitter) {
private static MethodEmitter newInitScopeMethod(final String className, final ClassEmitter classEmitter) {
final MethodEmitter init = classEmitter.init(PropertyMap.class, ScriptObject.class);
init.begin();
init.load(Type.OBJECT, JAVA_THIS.slot());
@ -442,7 +442,7 @@ public final class ObjectClassGenerator {
* @param classEmitter Open class emitter.
* @return Open method emitter.
*/
private static MethodEmitter newInitScopeWithArgumentsMethod(final ClassEmitter classEmitter) {
private static MethodEmitter newInitScopeWithArgumentsMethod(final String className, final ClassEmitter classEmitter) {
final MethodEmitter init = classEmitter.init(PropertyMap.class, ScriptObject.class, ScriptObject.class);
init.begin();
init.load(Type.OBJECT, JAVA_THIS.slot());
@ -460,7 +460,7 @@ public final class ObjectClassGenerator {
* @param classEmitter Open class emitter.
* @param className Name of JavaScript class.
*/
private static void newEmptyInit(final ClassEmitter classEmitter, final String className) {
private static void newEmptyInit(final String className, final ClassEmitter classEmitter) {
final MethodEmitter emptyInit = classEmitter.init();
emptyInit.begin();
emptyInit.load(Type.OBJECT, JAVA_THIS.slot());
@ -476,7 +476,7 @@ public final class ObjectClassGenerator {
* @param classEmitter Open class emitter.
* @param className Name of JavaScript class.
*/
private static void newAllocate(final ClassEmitter classEmitter, final String className) {
private static void newAllocate(final String className, final ClassEmitter classEmitter) {
final MethodEmitter allocate = classEmitter.method(EnumSet.of(Flag.PUBLIC, Flag.STATIC), ALLOCATE.symbolName(), ScriptObject.class, PropertyMap.class);
allocate.begin();
allocate._new(className, Type.typeFor(ScriptObject.class));

1
nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java
View File

@ -536,7 +536,6 @@ public class AccessorProperty extends Property {
//all this does is add a return value filter for object fields only
if (GETTER_CACHE[i] == null) {
//System.err.println(this + " needs new getter for " + type);
GETTER_CACHE[i] = debug(
createGetter(
getCurrentType(),

8
nashorn/src/jdk/nashorn/internal/runtime/PropertyListenerManager.java
View File

@ -105,7 +105,7 @@ public class PropertyListenerManager implements PropertyListener {
*/
protected synchronized final void notifyPropertyAdded(final ScriptObject object, final Property prop) {
if (listeners != null) {
for (PropertyListener listener : listeners.keySet()) {
for (final PropertyListener listener : listeners.keySet()) {
listener.propertyAdded(object, prop);
}
}
@ -119,7 +119,7 @@ public class PropertyListenerManager implements PropertyListener {
*/
protected synchronized final void notifyPropertyDeleted(final ScriptObject object, final Property prop) {
if (listeners != null) {
for (PropertyListener listener : listeners.keySet()) {
for (final PropertyListener listener : listeners.keySet()) {
listener.propertyDeleted(object, prop);
}
}
@ -134,7 +134,7 @@ public class PropertyListenerManager implements PropertyListener {
*/
protected synchronized final void notifyPropertyModified(final ScriptObject object, final Property oldProp, final Property newProp) {
if (listeners != null) {
for (PropertyListener listener : listeners.keySet()) {
for (final PropertyListener listener : listeners.keySet()) {
listener.propertyModified(object, oldProp, newProp);
}
}
@ -149,7 +149,7 @@ public class PropertyListenerManager implements PropertyListener {
*/
protected synchronized final void notifyProtoChanged(final ScriptObject object, final ScriptObject oldProto, final ScriptObject newProto) {
if (listeners != null) {
for (PropertyListener listener : listeners.keySet()) {
for (final PropertyListener listener : listeners.keySet()) {
listener.protoChanged(object, oldProto, newProto);
}
}

18
nashorn/src/jdk/nashorn/internal/runtime/PropertyMap.java
View File

@ -71,7 +71,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
private int fieldCount;
/** Number of fields available. */
private int fieldMaximum;
private final int fieldMaximum;
/** Length of spill in use. */
private int spillLength;
@ -167,7 +167,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
* @return New {@link PropertyMap}.
*/
public static PropertyMap newMap(final Collection<Property> properties, final int fieldCount, final int fieldMaximum, final int spillLength) {
PropertyHashMap newProperties = EMPTY_HASHMAP.immutableAdd(properties);
final PropertyHashMap newProperties = EMPTY_HASHMAP.immutableAdd(properties);
return new PropertyMap(newProperties, fieldCount, fieldMaximum, spillLength, false);
}
@ -181,7 +181,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
* @return New {@link PropertyMap}.
*/
public static PropertyMap newMap(final Collection<Property> properties) {
return (properties == null || properties.isEmpty())? newMap() : newMap(properties, 0, 0, 0);
return properties == null || properties.isEmpty()? newMap() : newMap(properties, 0, 0, 0);
}
/**
@ -356,10 +356,10 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
* the old property is an AccessorProperty and the new one is a UserAccessorProperty property.
*/
final boolean sameType = (oldProperty.getClass() == newProperty.getClass());
final boolean sameType = oldProperty.getClass() == newProperty.getClass();
assert sameType ||
(oldProperty instanceof AccessorProperty &&
newProperty instanceof UserAccessorProperty) :
oldProperty instanceof AccessorProperty &&
newProperty instanceof UserAccessorProperty :
"arbitrary replaceProperty attempted " + sameType + " oldProperty=" + oldProperty.getClass() + " newProperty=" + newProperty.getClass() + " [" + oldProperty.getCurrentType() + " => " + newProperty.getCurrentType() + "]";
newMap.flags = getClonedFlags();
@ -468,7 +468,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
PropertyMap freeze() {
PropertyHashMap newProperties = EMPTY_HASHMAP;
for (Property oldProperty : properties.getProperties()) {
for (final Property oldProperty : properties.getProperties()) {
int propertyFlags = Property.NOT_CONFIGURABLE;
if (!(oldProperty instanceof UserAccessorProperty)) {
@ -553,7 +553,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
final PropertyMap cachedMap;
if (protoHistory != null) {
final WeakReference<PropertyMap> weakMap = protoHistory.get(newProto);
cachedMap = (weakMap != null ? weakMap.get() : null);
cachedMap = weakMap != null ? weakMap.get() : null;
} else {
cachedMap = null;
}
@ -608,7 +608,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
*/
private PropertyMap checkHistory(final Property property) {
if (history != null) {
PropertyMap historicMap = history.get(property);
final PropertyMap historicMap = history.get(property);
if (historicMap != null) {
if (Context.DEBUG) {

9
nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java
View File

@ -237,12 +237,13 @@ public abstract class ScriptFunction extends ScriptObject {
if (Context.DEBUG) {
allocations++;
}
assert !isBoundFunction(); // allocate never invoked on bound functions
final ScriptObject object = data.allocate();
if (object != null) {
Object prototype = getPrototype();
final Object prototype = getPrototype();
if (prototype instanceof ScriptObject) {
object.setProto((ScriptObject)prototype);
}
@ -378,7 +379,7 @@ public abstract class ScriptFunction extends ScriptObject {
* @return self's prototype
*/
public static Object G$prototype(final Object self) {
return (self instanceof ScriptFunction) ?
return self instanceof ScriptFunction ?
((ScriptFunction)self).getPrototype() :
UNDEFINED;
}
@ -610,7 +611,7 @@ public abstract class ScriptFunction extends ScriptObject {
// Create the same arguments for the delegate linking request that would be passed in an actual apply'd invocation
final Object[] appliedArgs = new Object[isApply ? 3 : appliedType.parameterCount()];
appliedArgs[0] = appliedFn;
appliedArgs[1] = passesThis ? (appliedFnNeedsWrappedThis ? ScriptFunctionData.wrapThis(args[2]) : args[2]) : ScriptRuntime.UNDEFINED;
appliedArgs[1] = passesThis ? appliedFnNeedsWrappedThis ? ScriptFunctionData.wrapThis(args[2]) : args[2] : ScriptRuntime.UNDEFINED;
if(isApply) {
appliedArgs[2] = passesArgs ? NativeFunction.toApplyArgs(args[3]) : ScriptRuntime.EMPTY_ARRAY;
} else {
@ -757,7 +758,7 @@ public abstract class ScriptFunction extends ScriptObject {
@SuppressWarnings("unused")
private static Object[] addZerothElement(final Object[] args, final Object value) {
// extends input array with by adding new zeroth element
final Object[] src = (args == null)? ScriptRuntime.EMPTY_ARRAY : args;
final Object[] src = args == null? ScriptRuntime.EMPTY_ARRAY : args;
final Object[] result = new Object[src.length + 1];
System.arraycopy(src, 0, result, 1, src.length);
result[0] = value;

36
nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java
View File

@ -1840,7 +1840,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
final int listIndex = depth - 1; // We don't need 0-deep walker
MethodHandle filter = listIndex < PROTO_FILTERS.size() ? PROTO_FILTERS.get(listIndex) : null;
if(filter == null) {
if (filter == null) {
filter = addProtoFilter(GETPROTO, depth - 1);
PROTO_FILTERS.add(null);
PROTO_FILTERS.set(listIndex, filter);
@ -1874,7 +1874,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
return new GuardedInvocation(
GETPROTO,
explicitInstanceOfCheck ?
NashornGuards.getScriptObjectGuard() :
getScriptObjectGuard(desc.getMethodType(), explicitInstanceOfCheck) :
null,
null,
explicitInstanceOfCheck ?
@ -1902,9 +1902,9 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
mh = find.getGetter(returnType, programPoint);
//we never need a guard if noGuard is set
final boolean noGuard = OBJECT_FIELDS_ONLY && NashornCallSiteDescriptor.isFastScope(desc) && !property.canChangeType();
final boolean noGuard =/* OBJECT_FIELDS_ONLY &&*/ NashornCallSiteDescriptor.isFastScope(desc) && !property.canChangeType();
// getMap() is fine as we have the prototype switchpoint depending on where the property was found
final MethodHandle guard;
MethodHandle guard;
final Class<? extends Throwable> exception;
if (noGuard) {
guard = null;
@ -1925,9 +1925,13 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
}
if (!property.hasGetterFunction(find.getOwner())) {
// If not a scope bind to actual prototype as changing prototype will change the property map.
// For scopes we install a filter that replaces the self object with the prototype owning the property.
mh = isScope() ? addProtoFilter(mh, find.getProtoChainLength()) : bindTo(mh, find.getOwner());
if (isScope()) {
mh = addProtoFilter(mh, find.getProtoChainLength());
guard = NashornGuards.getMapGuard(find.getOwner().getMap(), explicitInstanceOfCheck);
guard = addProtoFilter(guard, find.getProtoChainLength());
} else {
mh = bindTo(mh, find.getOwner());
}
}
}
@ -1942,13 +1946,12 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
private static GuardedInvocation findMegaMorphicGetMethod(final CallSiteDescriptor desc, final String name, final boolean isMethod) {
ObjectClassGenerator.LOG.warning("Megamorphic getter: " + desc + " " + name + " " +isMethod);
return new GuardedInvocation(MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod), null, null, ClassCastException.class);
return new GuardedInvocation(MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod), getScriptObjectGuard(desc.getMethodType(), true), null, null);
}
@SuppressWarnings("unused")
private Object megamorphicGet(final String key, final boolean isMethod) {
final FindProperty find = findProperty(key, true);
if (find != null) {
return getObjectValue(find);
}
@ -1981,7 +1984,11 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
}
final MethodHandle mh = findGetIndexMethodHandle(returnClass, name, keyClass, desc);
return new GuardedInvocation(mh, NashornGuards.getScriptObjectGuard(explicitInstanceOfCheck), null, explicitInstanceOfCheck ? null : ClassCastException.class);
return new GuardedInvocation(mh, getScriptObjectGuard(callType, explicitInstanceOfCheck), null, explicitInstanceOfCheck ? null : ClassCastException.class);
}
private static MethodHandle getScriptObjectGuard(final MethodType type, final boolean explicitInstanceOfCheck) {
return ScriptObject.class.isAssignableFrom(type.parameterType(0)) ? null : NashornGuards.getScriptObjectGuard(explicitInstanceOfCheck);
}
/**
@ -2014,13 +2021,14 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
* @return GuardedInvocation to be invoked at call site.
*/
protected GuardedInvocation findSetMethod(final CallSiteDescriptor desc, final LinkRequest request) {
final String name = desc.getNameToken(CallSiteDescriptor.NAME_OPERAND);
final String name = desc.getNameToken(CallSiteDescriptor.NAME_OPERAND);
if (request.isCallSiteUnstable() || hasWithScope()) {
return findMegaMorphicSetMethod(desc, name);
}
final boolean scope = isScope();
final boolean explicitInstanceOfCheck = explicitInstanceOfCheck(desc, request);
final boolean explicitInstanceOfCheck = explicitInstanceOfCheck(desc, request);
/*
* If doing property set on a scope object, we should stop proto search on the first
@ -2050,7 +2058,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
if (PROTO_PROPERTY_NAME.equals(name)) {
return new GuardedInvocation(
SETPROTOCHECK,
NashornGuards.getScriptObjectGuard(explicitInstanceOfCheck),
getScriptObjectGuard(desc.getMethodType(), explicitInstanceOfCheck),
null,
explicitInstanceOfCheck ? null : ClassCastException.class);
} else if (!isExtensible()) {
@ -2124,7 +2132,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
MethodHandle methodHandle = findOwnMH_V(clazz, "set", void.class, keyClass, valueClass, boolean.class);
methodHandle = MH.insertArguments(methodHandle, 3, isStrict);
return new GuardedInvocation(methodHandle, NashornGuards.getScriptObjectGuard(explicitInstanceOfCheck), null, explicitInstanceOfCheck ? null : ClassCastException.class);
return new GuardedInvocation(methodHandle, getScriptObjectGuard(callType, explicitInstanceOfCheck), null, explicitInstanceOfCheck ? null : ClassCastException.class);
}
/**