[INFER] Improve precision when SETELEM is used on singleton objects, bug 675167.
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 01 Aug 2011 22:24:29 -0700
changeset 76088 674160662e80b4537796dec10668fb3117fd41db
parent 76087 d763fda00eb9a264e53e67f0188581757b81f0e8
child 76089 91281c11a122752991030da2bd8ec7583628013f
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
bugs675167
milestone8.0a1
[INFER] Improve precision when SETELEM is used on singleton objects, bug 675167.
js/src/jsinfer.cpp
js/src/jsinfer.h
js/src/jsinferinlines.h
js/src/jsiter.cpp
js/src/jswrapper.cpp
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -569,16 +569,50 @@ TypeSet::addCallProperty(JSContext *cx, 
     UntrapOpcode untrap(cx, script, callpc);
     if (JSOp(*callpc) == JSOP_NEW)
         return;
 
     add(cx, ArenaNew<TypeConstraintCallProp>(cx->compartment->pool, script, callpc, id));
 }
 
 /*
+ * Constraints for generating 'set' property constraints on a SETELEM only if
+ * the element type may be a number. For SETELEM we only account for integer
+ * indexes, and if the element cannot be an integer (e.g. it must be a string)
+ * then we lose precision by treating it like one.
+ */
+class TypeConstraintSetElement : public TypeConstraint
+{
+public:
+    JSScript *script;
+    jsbytecode *pc;
+
+    TypeSet *objectTypes;
+    TypeSet *valueTypes;
+
+    TypeConstraintSetElement(JSScript *script, jsbytecode *pc,
+                             TypeSet *objectTypes, TypeSet *valueTypes)
+        : TypeConstraint("setelement"), script(script), pc(pc),
+          objectTypes(objectTypes), valueTypes(valueTypes)
+    {
+        JS_ASSERT(script && pc);
+    }
+
+    void newType(JSContext *cx, TypeSet *source, Type type);
+};
+
+void
+TypeSet::addSetElement(JSContext *cx, JSScript *script, jsbytecode *pc,
+                       TypeSet *objectTypes, TypeSet *valueTypes)
+{
+    add(cx, ArenaNew<TypeConstraintSetElement>(cx->compartment->pool, script, pc,
+                                               objectTypes, valueTypes));
+}
+
+/*
  * Constraints for watching call edges as they are discovered and invoking native
  * function handlers, adding constraints for arguments, receiver objects and the
  * return value, and updating script foundOffsets.
  */
 class TypeConstraintCall : public TypeConstraint
 {
 public:
     /* Call site being tracked. */
@@ -821,55 +855,16 @@ public:
 };
 
 void
 TypeSet::addLazyArguments(JSContext *cx, TypeSet *target)
 {
     add(cx, ArenaNew<TypeConstraintLazyArguments>(cx->compartment->pool, target));
 }
 
-/*
- * Type constraint which marks the result of 'for in' loops as unknown if the
- * iterated value could be a generator.
- */
-class TypeConstraintGenerator : public TypeConstraint
-{
-public:
-    TypeSet *target;
-
-    TypeConstraintGenerator(TypeSet *target)
-        : TypeConstraint("generator"), target(target)
-    {}
-
-    void newType(JSContext *cx, TypeSet *source, Type type)
-    {
-        if (type.isUnknown() || type.isAnyObject()) {
-            target->addType(cx, Type::UnknownType());
-            return;
-        }
-
-        if (type.isPrimitive())
-            return;
-
-        /*
-         * Watch for 'for in' on Iterator and Generator objects, which can
-         * produce values other than strings.
-         */
-        JSObject *proto = type.isTypeObject()
-            ? type.typeObject()->proto
-            : type.singleObject()->getProto();
-
-        if (proto) {
-            Class *clasp = proto->getClass();
-            if (clasp == &js_IteratorClass || clasp == &js_GeneratorClass)
-                target->addType(cx, Type::UnknownType());
-        }
-    }
-};
-
 /////////////////////////////////////////////////////////////////////
 // TypeConstraint
 /////////////////////////////////////////////////////////////////////
 
 /* Get the object to use for a property access on type. */
 static inline TypeObject *
 GetPropertyObject(JSContext *cx, JSScript *script, Type type)
 {
@@ -1034,16 +1029,26 @@ TypeConstraintCallProp::newType(JSContex
             /* Bypass addPropagateThis, we already have the callpc. */
             types->add(cx, ArenaNew<TypeConstraintPropagateThis>(cx->compartment->pool,
                                                                  script, callpc, type));
         }
     }
 }
 
 void
+TypeConstraintSetElement::newType(JSContext *cx, TypeSet *source, Type type)
+{
+    if (type.isUnknown() ||
+        type.isPrimitive(JSVAL_TYPE_INT32) ||
+        type.isPrimitive(JSVAL_TYPE_DOUBLE)) {
+        objectTypes->addSetProperty(cx, script, pc, valueTypes, JSID_VOID);
+    }
+}
+
+void
 TypeConstraintCall::newType(JSContext *cx, TypeSet *source, Type type)
 {
     JSScript *script = callsite->script;
     jsbytecode *pc = callsite->pc;
 
     if (type.isUnknown() || type.isAnyObject()) {
         /* Monitor calls on unknown functions. */
         cx->compartment->types.monitorBytecode(cx, script, pc - script->code);
@@ -3470,17 +3475,17 @@ ScriptAnalysis::analyzeTypesBytecode(JSC
             poppedTypes(pc, 1)->addFilterPrimitives(cx, &pushed[1], true);
         if (CheckNextTest(pc))
             pushed[0].addType(cx, Type::UndefinedType());
         break;
       }
 
       case JSOP_SETELEM:
       case JSOP_SETHOLE:
-        poppedTypes(pc, 2)->addSetProperty(cx, script, pc, poppedTypes(pc, 0), JSID_VOID);
+        poppedTypes(pc, 1)->addSetElement(cx, script, pc, poppedTypes(pc, 2), poppedTypes(pc, 0));
         poppedTypes(pc, 0)->addSubset(cx, &pushed[0]);
         break;
 
       case JSOP_TOID:
         /*
          * This is only used for element inc/dec ops; any id produced which
          * is not an integer must be monitored.
          */
@@ -3692,44 +3697,29 @@ ScriptAnalysis::analyzeTypesBytecode(JSC
          * This loses some precision when a script mixes 'for in' and
          * 'for each' loops together, oh well.
          */
         if (!state.forTypes) {
           state.forTypes = TypeSet::make(cx, "forTypes");
           if (!state.forTypes)
               return false;
         }
-        poppedTypes(pc, 0)->addSubset(cx, state.forTypes);
 
         if (pc[1] & JSITER_FOREACH)
             state.forTypes->addType(cx, Type::UnknownType());
-
-        /*
-         * Feed any types from the pushed type set into the forTypes. If a
-         * custom __iterator__ hook is added it will appear as an unknown
-         * value pushed by this opcode.
-         */
-        pushed[0].addSubset(cx, state.forTypes);
-
+        else
+            state.forTypes->addType(cx, Type::StringType());
         break;
       }
 
       case JSOP_ITERNEXT:
-        /*
-         * The value bound is a string, unless this is a 'for each' loop or the
-         * iterated object is a generator or has an __iterator__ hook, which
-         * we'll detect dynamically.
-         */
-        pushed[0].addType(cx, Type::StringType());
-        state.forTypes->add(cx,
-            ArenaNew<TypeConstraintGenerator>(cx->compartment->pool, &pushed[0]));
+        state.forTypes->addSubset(cx, &pushed[0]);
         break;
 
       case JSOP_MOREITER:
-        poppedTypes(pc, 0)->addSubset(cx, &pushed[0]);
         pushed[1].addType(cx, Type::BooleanType());
         break;
 
       case JSOP_ENUMELEM:
       case JSOP_ENUMCONSTELEM:
       case JSOP_ARRAYPUSH:
         cx->compartment->types.monitorBytecode(cx, script, offset);
         break;
@@ -3915,17 +3905,22 @@ ScriptAnalysis::analyzeTypes(JSContext *
 
     /*
      * Replay any dynamic type results which have been generated for the script
      * either because we ran the interpreter some before analyzing or because
      * we are reanalyzing after a GC.
      */
     TypeResult *result = script->types->dynamicList;
     while (result) {
-        pushedTypes(result->offset)->addType(cx, result->type);
+        if (result->offset != uint32(-1)) {
+            pushedTypes(result->offset)->addType(cx, result->type);
+        } else {
+            /* Custom for-in loop iteration has happened in this script. */
+            state.forTypes->addType(cx, Type::UnknownType());
+        }
         result = result->next;
     }
 
     if (!script->usesArguments || script->createdArgs)
         return;
 
     /*
      * Do additional analysis to determine whether the arguments object in the
@@ -4569,27 +4564,67 @@ MarkIteratorUnknownSlow(JSContext *cx)
 {
     /* Check whether we are actually at an ITER opcode. */
 
     jsbytecode *pc;
     JSScript *script = cx->stack.currentScript(&pc);
     if (!script || !pc)
         return;
 
-    /*
-     * Watch out if the caller is in a different compartment from this one.
-     * This must have gone through a cross-compartment wrapper.
-     */
-    if (script->compartment != cx->compartment)
+    UntrapOpcode untrap(cx, script, pc);
+
+    if (JSOp(*pc) != JSOP_ITER)
         return;
 
-    js::analyze::UntrapOpcode untrap(cx, script, pc);
-
-    if (JSOp(*pc) == JSOP_ITER)
-        TypeDynamicResult(cx, script, pc, Type::UnknownType());
+    AutoEnterTypeInference enter(cx);
+
+    /*
+     * This script is iterating over an actual Iterator or Generator object, or
+     * an object with a custom __iterator__ hook. In such cases 'for in' loops
+     * can produce values other than strings, and the types of the ITER opcodes
+     * in the script need to be updated. During analysis this is done with the
+     * forTypes in the analysis state, but we don't keep a pointer to this type
+     * set and need to scan the script to fix affected opcodes.
+     */
+
+    TypeResult *result = script->types->dynamicList;
+    while (result) {
+        if (result->offset == uint32(-1)) {
+            /* Already know about custom iterators used in this script. */
+            JS_ASSERT(result->type.isUnknown());
+            return;
+        }
+    }
+
+    InferSpew(ISpewOps, "externalType: customIterator #%u", script->id());
+
+    result = cx->new_<TypeResult>(uint32(-1), Type::UnknownType());
+    if (!result) {
+        cx->compartment->types.setPendingNukeTypes(cx);
+        return;
+    }
+    result->next = script->types->dynamicList;
+    script->types->dynamicList = result;
+
+    if (!script->hasAnalysis() || !script->analysis()->ranInference())
+        return;
+
+    ScriptAnalysis *analysis = script->analysis();
+
+    for (unsigned i = 0; i < script->length; i++) {
+        jsbytecode *pc = script->code + i;
+        if (!analysis->maybeCode(pc))
+            continue;
+        if (js_GetOpcode(cx, script, pc) == JSOP_ITERNEXT)
+            analysis->pushedTypes(pc, 0)->addType(cx, Type::UnknownType());
+    }
+
+    /* Trigger recompilation of any inline callers. */
+    if (script->hasFunction && !script->function()->hasLazyType())
+        ObjectStateChange(cx, script->function()->type(), false, true);
 }
 
 void
 TypeMonitorCallSlow(JSContext *cx, JSObject *callee,
                     const CallArgs &args, bool constructing)
 {
     unsigned nargs = callee->getFunctionPrivate()->nargs;
     JSScript *script = callee->getFunctionPrivate()->script();
--- a/js/src/jsinfer.h
+++ b/js/src/jsinfer.h
@@ -429,16 +429,18 @@ class TypeSet
     /* Add specific kinds of constraints to this set. */
     inline void add(JSContext *cx, TypeConstraint *constraint, bool callExisting = true);
     void addSubset(JSContext *cx, TypeSet *target);
     void addGetProperty(JSContext *cx, JSScript *script, jsbytecode *pc,
                         TypeSet *target, jsid id);
     void addSetProperty(JSContext *cx, JSScript *script, jsbytecode *pc,
                         TypeSet *target, jsid id);
     void addCallProperty(JSContext *cx, JSScript *script, jsbytecode *pc, jsid id);
+    void addSetElement(JSContext *cx, JSScript *script, jsbytecode *pc,
+                       TypeSet *objectTypes, TypeSet *valueTypes);
     void addCall(JSContext *cx, TypeCallsite *site);
     void addArith(JSContext *cx, TypeSet *target, TypeSet *other = NULL);
     void addTransformThis(JSContext *cx, JSScript *script, TypeSet *target);
     void addPropagateThis(JSContext *cx, JSScript *script, jsbytecode *pc, Type type);
     void addFilterPrimitives(JSContext *cx, TypeSet *target, bool onlyNullVoid);
     void addSubsetBarrier(JSContext *cx, JSScript *script, jsbytecode *pc, TypeSet *target);
     void addLazyArguments(JSContext *cx, TypeSet *target);
 
@@ -500,17 +502,17 @@ class TypeSet
     inline uint32 baseObjectCount() const;
     inline void setBaseObjectCount(uint32 count);
 };
 
 /*
  * Handler which persists information about dynamic types pushed within a
  * script which can affect its behavior and are not covered by JOF_TYPESET ops,
  * such as integer operations which overflow to a double. These persist across
- * GCs and are regenerated when the script is reanalyzed.
+ * GCs, and are used to re-seed script types when they are reanalyzed.
  */
 struct TypeResult
 {
     uint32 offset;
     Type type;
     TypeResult *next;
 
     TypeResult(uint32 offset, Type type)
--- a/js/src/jsinferinlines.h
+++ b/js/src/jsinferinlines.h
@@ -284,17 +284,17 @@ GetTypeNewObject(JSContext *cx, JSProtoK
 
 /* Get a type object for the immediate allocation site within a native. */
 inline TypeObject *
 GetTypeCallerInitObject(JSContext *cx, JSProtoKey key)
 {
     if (cx->typeInferenceEnabled()) {
         jsbytecode *pc;
         JSScript *script = cx->stack.currentScript(&pc);
-        if (script && script->compartment == cx->compartment)
+        if (script)
             return TypeScript::InitObject(cx, script, pc, key);
     }
     return GetTypeNewObject(cx, key);
 }
 
 /*
  * When using a custom iterator within the initialization of a 'for in' loop,
  * mark the iterator values as unknown.
@@ -593,22 +593,24 @@ TypeScript::MonitorUnknown(JSContext *cx
     if (cx->typeInferenceEnabled())
         TypeDynamicResult(cx, script, pc, Type::UnknownType());
 }
 
 /* static */ inline void
 TypeScript::MonitorAssign(JSContext *cx, JSScript *script, jsbytecode *pc,
                           JSObject *obj, jsid id, const js::Value &rval)
 {
-    if (cx->typeInferenceEnabled() && !obj->hasLazyType()) {
+    if (cx->typeInferenceEnabled() && !obj->hasSingletonType()) {
         /*
          * Mark as unknown any object which has had dynamic assignments to
          * non-integer properties at SETELEM opcodes. This avoids making large
-         * numbers of type properties for hashmap-style objects. :FIXME: this
-         * is too aggressive for things like prototype library initialization.
+         * numbers of type properties for hashmap-style objects. We don't need
+         * to do this for objects with singleton type, because type properties
+         * are only constructed for them when analyzed scripts depend on those
+         * specific properties.
          */
         uint32 i;
         if (js_IdIsIndex(id, &i))
             return;
         MarkTypeObjectUnknownProperties(cx, obj->type());
     }
 }
 
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -349,25 +349,16 @@ GetCustomIterator(JSContext *cx, JSObjec
         return false;
 
     /* If there is no custom __iterator__ method, we are done here. */
     if (!vp->isObject()) {
         vp->setUndefined();
         return true;
     }
 
-    /*
-     * Notify type inference of the custom iterator.  This only needs to be done
-     * if this is coming from a 'for in' loop, not a call to Iterator itself.
-     * If an Iterator object is used in a for loop then the values fetched in
-     * that loop are unknown, whether there is a custom __iterator__ or not.
-     */
-    if (!(flags & JSITER_OWNONLY))
-        types::MarkIteratorUnknown(cx);
-
     /* Otherwise call it and return that object. */
     LeaveTrace(cx);
     Value arg = BooleanValue((flags & JSITER_FOREACH) == 0);
     if (!ExternalInvoke(cx, ObjectValue(*obj), *vp, 1, &arg, vp))
         return false;
     if (vp->isPrimitive()) {
         /*
          * We are always coming from js_ValueToIterator, and we are no longer on
@@ -574,16 +565,17 @@ GetIterator(JSContext *cx, JSObject *obj
     if (obj) {
         /* Enumerate Iterator.prototype directly. */
         JSIteratorOp op = obj->getClass()->ext.iteratorObject;
         if (op && (obj->getClass() != &js_IteratorClass || obj->getNativeIterator())) {
             JSObject *iterobj = op(cx, obj, !(flags & JSITER_FOREACH));
             if (!iterobj)
                 return false;
             vp->setObject(*iterobj);
+            types::MarkIteratorUnknown(cx);
             return true;
         }
 
         if (keysOnly) {
             /*
              * Check to see if this is the same as the most recent object which
              * was iterated over.  We don't explicitly check for shapeless
              * objects here, as they are not inserted into the cache and
@@ -641,22 +633,26 @@ GetIterator(JSContext *cx, JSObject *obj
                     if (shapes.length() == 2)
                         cx->compartment->nativeIterCache.last = iterobj;
                     return true;
                 }
             }
         }
 
       miss:
-        if (obj->isProxy())
+        if (obj->isProxy()) {
+            types::MarkIteratorUnknown(cx);
             return JSProxy::iterate(cx, obj, flags, vp);
+        }
         if (!GetCustomIterator(cx, obj, flags, vp))
             return false;
-        if (!vp->isUndefined())
+        if (!vp->isUndefined()) {
+            types::MarkIteratorUnknown(cx);
             return true;
+        }
     }
 
     /* NB: for (var p in null) succeeds by iterating over no properties. */
 
     AutoIdVector keys(cx);
     if (flags & JSITER_FOREACH) {
         if (JS_LIKELY(obj != NULL) && !Snapshot(cx, obj, flags, &keys))
             return false;
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -671,20 +671,16 @@ Reify(JSContext *cx, JSCompartment *orig
     if (isKeyIter)
         return VectorToKeyIterator(cx, obj, ni->flags, keys, vp);
     return VectorToValueIterator(cx, obj, ni->flags, keys, vp); 
 }
 
 bool
 JSCrossCompartmentWrapper::iterate(JSContext *cx, JSObject *wrapper, uintN flags, Value *vp)
 {
-    /* Notify type inference of custom iterators, see GetCustomIterator. */
-    if (!(flags & JSITER_OWNONLY))
-        types::MarkIteratorUnknown(cx);
-
     PIERCE(cx, wrapper, GET,
            NOTHING,
            JSWrapper::iterate(cx, wrapper, flags, vp),
            CanReify(vp) ? Reify(cx, call.origin, vp) : call.origin->wrap(cx, vp));
 }
 
 bool
 JSCrossCompartmentWrapper::call(JSContext *cx, JSObject *wrapper, uintN argc, Value *vp)