Bug 787813 - Argument object, Use StackIter instead of StackFrame. r=luke
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Thu, 04 Oct 2012 23:13:35 -0700
changeset 109339 47a17015ef4a7ec82a7adf202f08aa3dca50f8f7
parent 109338 84804ed93ccb9b1990fd4f3ed459859f8951b0ef
child 109340 3b458f4e0f4228973455964767c6f7acb119e9b5
push id15976
push usernpierron@mozilla.com
push dateFri, 05 Oct 2012 06:57:54 +0000
treeherdermozilla-inbound@3b458f4e0f42 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs787813
milestone18.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 787813 - Argument object, Use StackIter instead of StackFrame. r=luke
js/src/ion/IonFrameIterator-inl.h
js/src/ion/IonFrameIterator.h
js/src/jit-test/tests/basic/bug632901.js
js/src/jsfun.cpp
js/src/vm/ArgumentsObject.cpp
js/src/vm/ArgumentsObject.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/ion/IonFrameIterator-inl.h
+++ b/js/src/ion/IonFrameIterator-inl.h
@@ -9,71 +9,69 @@
 #define jsion_frame_iterator_inl_h__
 
 #include "ion/IonFrameIterator.h"
 
 namespace js {
 namespace ion {
 
 template <class Op>
-inline bool
+inline void
 SnapshotIterator::readFrameArgs(Op op, const Value *argv, Value *scopeChain, Value *thisv,
                                 unsigned start, unsigned formalEnd, unsigned iterEnd)
 {
     if (scopeChain)
         *scopeChain = read();
     else
         skip();
 
     if (thisv)
         *thisv = read();
     else
         skip();
 
     unsigned i = 0;
+    if (formalEnd < start)
+        i = start;
+
     for (; i < start; i++)
         skip();
-    for (; i < formalEnd; i++) {
+    for (; i < formalEnd && i < iterEnd; i++) {
         // We are not always able to read values from the snapshots, some values
         // such as non-gc things may still be live in registers and cause an
         // error while reading the machine state.
         Value v = maybeRead();
         op(v);
     }
     if (iterEnd >= formalEnd) {
         for (; i < iterEnd; i++)
             op(argv[i]);
     }
-    return true;
 }
 
 template <class Op>
-inline bool
+inline void
 InlineFrameIterator::forEachCanonicalActualArg(Op op, unsigned start, unsigned count) const
 {
     unsigned nactual = numActualArgs();
     if (count == unsigned(-1))
         count = nactual - start;
 
     unsigned end = start + count;
+    unsigned nformal = callee()->nargs;
+
     JS_ASSERT(start <= end && end <= nactual);
 
-    unsigned nformal = callee()->nargs;
-    unsigned formalEnd = end;
-    if (!more() && end > nformal) {
-        formalEnd = nformal;
-    } else {
-        // Currently inlining does not support overflow of arguments, we have to
-        // add this feature in IonBuilder.cpp and in Bailouts.cpp before
-        // continuing. We need to add it to Bailouts.cpp because we need to know
-        // how to walk over the oveflow of arguments.
-        JS_ASSERT(end <= nformal);
-    }
+    // Currently inlining does not support overflow of arguments, we have to
+    // add this feature in IonBuilder.cpp and in Bailouts.cpp before
+    // continuing. We need to add it to Bailouts.cpp because we need to know
+    // how to walk over the oveflow of arguments.
+    JS_ASSERT_IF(more(), end <= nformal);
 
     SnapshotIterator s(si_);
     Value *argv = frame_->actualArgs();
-    return s.readFrameArgs(op, argv, NULL, NULL, start, formalEnd, end);
+    s.readFrameArgs(op, argv, NULL, NULL, start, nformal, end);
 }
 
 } // namespace ion
 } // namespace js
 
 #endif // jsion_frame_iterator_inl_h__
--- a/js/src/ion/IonFrameIterator.h
+++ b/js/src/ion/IonFrameIterator.h
@@ -233,17 +233,17 @@ class SnapshotIterator : public Snapshot
         if (slotReadable(s))
             return slotValue(s);
         if (!silentFailure)
             warnUnreadableSlot();
         return UndefinedValue();
     }
 
     template <class Op>
-    inline bool readFrameArgs(Op op, const Value *argv, Value *scopeChain, Value *thisv,
+    inline void readFrameArgs(Op op, const Value *argv, Value *scopeChain, Value *thisv,
                               unsigned start, unsigned formalEnd, unsigned iterEnd);
 
     Value maybeReadSlotByIndex(size_t index) {
         while (index--) {
             JS_ASSERT(moreSlots());
             skip();
         }
 
@@ -284,17 +284,17 @@ class InlineFrameIterator
         return callee_;
     }
     JSFunction *maybeCallee() const {
         return callee_;
     }
     unsigned numActualArgs() const;
 
     template <class Op>
-    inline bool forEachCanonicalActualArg(Op op, unsigned start, unsigned count) const;
+    inline void forEachCanonicalActualArg(Op op, unsigned start, unsigned count) const;
 
     JSScript *script() const {
         return script_;
     }
     jsbytecode *pc() const {
         return pc_;
     }
     SnapshotIterator snapshotIterator() const {
--- a/js/src/jit-test/tests/basic/bug632901.js
+++ b/js/src/jit-test/tests/basic/bug632901.js
@@ -1,9 +1,9 @@
 // don't crash when tracing
 function f(o) {
     var prop = "arguments";
     f[prop] = f[prop];
 }
-for(var i=0; i<10; i++) {
+for(var i = 0; i < 50; i++) {
     f();
 }
 
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -103,30 +103,28 @@ fun_getProperty(JSContext *cx, HandleObj
         if (!iter.isFunctionFrame() || iter.isEvalFrame())
             continue;
         if (iter.callee() == fun)
             break;
     }
     if (iter.done())
         return true;
 
-    StackFrame *fp = iter.fp();
-
     if (JSID_IS_ATOM(id, cx->names().arguments)) {
         if (fun->hasRest()) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_FUNCTION_ARGUMENTS_AND_REST);
             return false;
         }
         /* Warn if strict about f.arguments or equivalent unqualified uses. */
         if (!JS_ReportErrorFlagsAndNumber(cx, JSREPORT_WARNING | JSREPORT_STRICT, js_GetErrorMessage,
                                           NULL, JSMSG_DEPRECATED_USAGE, js_arguments_str)) {
             return false;
         }
 
-        ArgumentsObject *argsobj = ArgumentsObject::createUnexpected(cx, fp);
+        ArgumentsObject *argsobj = ArgumentsObject::createUnexpected(cx, iter);
         if (!argsobj)
             return false;
 
 #ifdef JS_ION
         // If this script hasn't been compiled yet, make sure it will never
         // be compiled. IonMonkey does not guarantee |f.arguments| can be
         // fully recovered, so we try to mitigate observing this behavior by
         // detecting its use early.
@@ -135,16 +133,17 @@ fun_getProperty(JSContext *cx, HandleObj
             ion::ForbidCompilation(cx, script);
 #endif
 
         vp.setObject(*argsobj);
         return true;
     }
 
 #ifdef JS_METHODJIT
+    StackFrame *fp = iter.fp();
     if (iter.isScript() && iter.isIon())
         fp = NULL;
 
     if (JSID_IS_ATOM(id, cx->names().caller) && fp && fp->prev()) {
         /*
          * If the frame was called from within an inlined frame, mark the
          * innermost function as uninlineable to expand its frame and allow us
          * to recover its callee object.
--- a/js/src/vm/ArgumentsObject.cpp
+++ b/js/src/vm/ArgumentsObject.cpp
@@ -11,112 +11,217 @@
 
 #include "vm/GlobalObject.h"
 #include "vm/Stack.h"
 #include "vm/Xdr.h"
 
 #include "jsobjinlines.h"
 
 #include "gc/Barrier-inl.h"
+#include "vm/Stack-inl.h"
 #include "vm/ArgumentsObject-inl.h"
 
 using namespace js;
 using namespace js::gc;
 
-ArgumentsObject *
-ArgumentsObject::create(JSContext *cx, StackFrame *fp)
+/* Erase formals which are not part of the actuals. */
+static void
+SetMissingFormalArgsToUndefined(HeapValue *dstBase, unsigned numActuals, unsigned numFormals)
+{
+    if (numActuals < numFormals) {
+        HeapValue *dst = dstBase + numActuals, *dstEnd = dstBase + numFormals;
+        while (dst != dstEnd)
+            (dst++)->init(UndefinedValue());
+    }
+}
+
+static void
+CopyStackFrameArguments(const StackFrame *fp, HeapValue *dst)
+{
+    HeapValue *dstBase = dst;
+    unsigned numActuals = fp->numActualArgs();
+    unsigned numFormals = fp->callee().nargs;
+
+    /* Copy formal arguments. */
+    Value *src = fp->formals();
+    Value *end = src + numFormals;
+    while (src != end)
+        (dst++)->init(*src++);
+
+    /* Copy actual argument which are not contignous. */
+    if (numFormals < numActuals) {
+        src = fp->actuals() + numFormals;
+        end = src + (numActuals - numFormals);
+        while (src != end)
+            (dst++)->init(*src++);
+    }
+    SetMissingFormalArgsToUndefined(dstBase, numActuals, numFormals);
+}
+
+/* static */ void
+ArgumentsObject::MaybeForwardToCallObject(StackFrame *fp, JSObject *obj, ArgumentsData *data)
+{
+    JSScript *script = fp->script();
+    if (fp->fun()->isHeavyweight() && script->argsObjAliasesFormals()) {
+        obj->initFixedSlot(MAYBE_CALL_SLOT, ObjectValue(fp->callObj()));
+        for (AliasedFormalIter fi(script); fi; fi++)
+            data->args[fi.frameIndex()] = MagicValue(JS_FORWARD_TO_CALL_OBJECT);
+    }
+}
+
+struct CopyStackFrameArgs
 {
-    RootedObject proto(cx, fp->callee().global().getOrCreateObjectPrototype(cx));
+    StackFrame *fp_;
+
+    CopyStackFrameArgs(StackFrame *fp)
+      : fp_(fp)
+    { }
+
+    inline JSScript *script() const { return fp_->script(); }
+    inline JSFunction *callee() const { return &fp_->callee(); }
+    unsigned numActualArgs() const { return fp_->numActualArgs(); }
+
+    void copyArgs(HeapValue *dst) const {
+        CopyStackFrameArguments(fp_, dst);
+    }
+
+    /*
+     * If a call object exists and the arguments object aliases formals, the
+     * call object is the canonical location for formals.
+     */
+    void maybeForwardToCallObject(JSObject *obj, ArgumentsData *data) {
+        ArgumentsObject::MaybeForwardToCallObject(fp_, obj, data);
+    }
+};
+
+struct CopyStackIterArgs
+{
+    StackIter &iter_;
+
+    CopyStackIterArgs(StackIter &iter)
+      : iter_(iter)
+    { }
+
+    inline JSScript *script() const { return iter_.script(); }
+    inline JSFunction *callee() const { return iter_.callee(); }
+    unsigned numActualArgs() const { return iter_.numActualArgs(); }
+
+    void copyArgs(HeapValue *dst) const {
+        if (!iter_.isIon()) {
+            CopyStackFrameArguments(iter_.fp(), dst);
+            return;
+        }
+
+        unsigned numActuals = iter_.numActualArgs();
+        unsigned numFormals = iter_.callee()->nargs;
+
+        iter_.ionForEachCanonicalActualArg(CopyToHeap(dst));
+        SetMissingFormalArgsToUndefined(dst, numActuals, numFormals);
+    }
+
+    /*
+     * Ion frames are copying every argument onto the stack, other locations are
+     * invalid.
+     */
+    void maybeForwardToCallObject(JSObject *obj, ArgumentsData *data) {
+        if (!iter_.isIon())
+            ArgumentsObject::MaybeForwardToCallObject(iter_.fp(), obj, data);
+    }
+};
+
+template <typename CopyArgs>
+/* static */ ArgumentsObject *
+ArgumentsObject::create(JSContext *cx, HandleScript script, HandleFunction callee, CopyArgs &copy)
+{
+    RootedObject proto(cx, copy.callee()->global().getOrCreateObjectPrototype(cx));
     if (!proto)
         return NULL;
 
     RootedTypeObject type(cx, proto->getNewType(cx));
     if (!type)
         return NULL;
 
-    bool strict = fp->callee().inStrictMode();
+    bool strict = copy.callee()->inStrictMode();
     Class *clasp = strict ? &StrictArgumentsObjectClass : &NormalArgumentsObjectClass;
 
     RootedShape shape(cx, EmptyShape::getInitialShape(cx, clasp, TaggedProto(proto),
                                                       proto->getParent(), FINALIZE_KIND,
                                                       BaseShape::INDEXED));
     if (!shape)
         return NULL;
 
-    unsigned numActuals = fp->numActualArgs();
-    unsigned numFormals = fp->numFormalArgs();
+    unsigned numActuals = copy.numActualArgs();
+    unsigned numFormals = copy.callee()->nargs;
     unsigned numDeletedWords = NumWordsForBitArrayOfLength(numActuals);
     unsigned numArgs = Max(numActuals, numFormals);
     unsigned numBytes = offsetof(ArgumentsData, args) +
                         numDeletedWords * sizeof(size_t) +
                         numArgs * sizeof(Value);
 
     ArgumentsData *data = (ArgumentsData *)cx->malloc_(numBytes);
     if (!data)
         return NULL;
 
     data->numArgs = numArgs;
-    data->callee.init(ObjectValue(fp->callee()));
-    data->script = fp->script();
+    data->callee.init(ObjectValue(*copy.callee()));
+    data->script = copy.script();
 
     /* Copy [0, numArgs) into data->slots. */
     HeapValue *dst = data->args, *dstEnd = data->args + numArgs;
-    for (Value *src = fp->formals(), *end = src + numFormals; src != end; ++src, ++dst)
-        dst->init(*src);
-    if (numActuals > numFormals) {
-        for (Value *src = fp->actuals() + numFormals; dst != dstEnd; ++src, ++dst)
-            dst->init(*src);
-    } else if (numActuals < numFormals) {
-        for (; dst != dstEnd; ++dst)
-            dst->init(UndefinedValue());
-    }
+    copy.copyArgs(dst);
 
     data->deletedBits = reinterpret_cast<size_t *>(dstEnd);
     ClearAllBitArrayElements(data->deletedBits, numDeletedWords);
 
     JSObject *obj = JSObject::create(cx, FINALIZE_KIND, shape, type, NULL);
     if (!obj)
         return NULL;
 
     obj->initFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(numActuals << PACKED_BITS_COUNT));
     obj->initFixedSlot(DATA_SLOT, PrivateValue(data));
 
-    /*
-     * If it exists and the arguments object aliases formals, the call object
-     * is the canonical location for formals.
-     */
-    JSScript *script = fp->script();
-    if (fp->fun()->isHeavyweight() && script->argsObjAliasesFormals()) {
-        obj->initFixedSlot(MAYBE_CALL_SLOT, ObjectValue(fp->callObj()));
-        for (AliasedFormalIter fi(script); fi; fi++)
-            data->args[fi.frameIndex()] = MagicValue(JS_FORWARD_TO_CALL_OBJECT);
-    }
+    copy.maybeForwardToCallObject(obj, data);
 
     ArgumentsObject &argsobj = obj->asArguments();
     JS_ASSERT(argsobj.initialLength() == numActuals);
     JS_ASSERT(!argsobj.hasOverriddenLength());
     return &argsobj;
 }
 
 ArgumentsObject *
 ArgumentsObject::createExpected(JSContext *cx, StackFrame *fp)
 {
     JS_ASSERT(fp->script()->needsArgsObj());
-    ArgumentsObject *argsobj = create(cx, fp);
+    RootedScript script(cx, fp->script());
+    RootedFunction callee(cx, &fp->callee());
+    CopyStackFrameArgs copy(fp);
+    ArgumentsObject *argsobj = create(cx, script, callee, copy);
     if (!argsobj)
         return NULL;
 
     fp->initArgsObj(*argsobj);
     return argsobj;
 }
 
 ArgumentsObject *
+ArgumentsObject::createUnexpected(JSContext *cx, StackIter &iter)
+{
+    RootedScript script(cx, iter.script());
+    RootedFunction callee(cx, iter.callee());
+    CopyStackIterArgs copy(iter);
+    return create(cx, script, callee, copy);
+}
+
+ArgumentsObject *
 ArgumentsObject::createUnexpected(JSContext *cx, StackFrame *fp)
 {
-    return create(cx, fp);
+    RootedScript script(cx, fp->script());
+    RootedFunction callee(cx, &fp->callee());
+    CopyStackFrameArgs copy(fp);
+    return create(cx, script, callee, copy);
 }
 
 static JSBool
 args_delProperty(JSContext *cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
     ArgumentsObject &argsobj = obj->asArguments();
     if (JSID_IS_INT(id)) {
         unsigned arg = unsigned(JSID_TO_INT(id));
--- a/js/src/vm/ArgumentsObject.h
+++ b/js/src/vm/ArgumentsObject.h
@@ -98,32 +98,36 @@ class ArgumentsObject : public JSObject
   protected:
     static const uint32_t INITIAL_LENGTH_SLOT = 0;
     static const uint32_t DATA_SLOT = 1;
     static const uint32_t MAYBE_CALL_SLOT = 2;
 
     static const uint32_t LENGTH_OVERRIDDEN_BIT = 0x1;
     static const uint32_t PACKED_BITS_COUNT = 1;
 
-    static ArgumentsObject *create(JSContext *cx, StackFrame *fp);
+    template <typename CopyArgs>
+    static ArgumentsObject *create(JSContext *cx, HandleScript script,
+                                   HandleFunction callee, CopyArgs &copy);
+
     inline ArgumentsData *data() const;
 
   public:
     static const uint32_t RESERVED_SLOTS = 3;
     static const gc::AllocKind FINALIZE_KIND = gc::FINALIZE_OBJECT4;
 
     /* Create an arguments object for a frame that is expecting them. */
     static ArgumentsObject *createExpected(JSContext *cx, StackFrame *fp);
 
     /*
      * Purposefully disconnect the returned arguments object from the frame
      * by always creating a new copy that does not alias formal parameters.
      * This allows function-local analysis to determine that formals are
      * not aliased and generally simplifies arguments objects.
      */
+    static ArgumentsObject *createUnexpected(JSContext *cx, StackIter &iter);
     static ArgumentsObject *createUnexpected(JSContext *cx, StackFrame *fp);
 
     /*
      * Return the initial length of the arguments.  This may differ from the
      * current value of arguments.length!
      */
     inline uint32_t initialLength() const;
 
@@ -192,16 +196,18 @@ class ArgumentsObject : public JSObject
 
     static void finalize(FreeOp *fop, RawObject obj);
     static void trace(JSTracer *trc, RawObject obj);
 
     /* For jit use: */
     static size_t getDataSlotOffset() {
         return getFixedSlotOffset(DATA_SLOT);
     }
+
+    static void MaybeForwardToCallObject(StackFrame *fp, JSObject *obj, ArgumentsData *data);
 };
 
 class NormalArgumentsObject : public ArgumentsObject
 {
   public:
     /*
      * Stores arguments.callee, or MagicValue(JS_ARGS_HOLE) if the callee has
      * been cleared.
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -285,16 +285,23 @@ StackFrame::forEachUnaliasedActual(Op op
 
 struct CopyTo
 {
     Value *dst;
     CopyTo(Value *dst) : dst(dst) {}
     void operator()(const Value &src) { *dst++ = src; }
 };
 
+struct CopyToHeap
+{
+    HeapValue *dst;
+    CopyToHeap(HeapValue *dst) : dst(dst) {}
+    void operator()(const Value &src) { dst->init(src); ++dst; }
+};
+
 inline unsigned
 StackFrame::numFormalArgs() const
 {
     JS_ASSERT(hasArgs());
     return fun()->nargs;
 }
 
 inline unsigned
@@ -564,31 +571,19 @@ ContextStack::currentScript(jsbytecode *
 
 inline HandleObject
 ContextStack::currentScriptedScopeChain() const
 {
     return fp()->scopeChain();
 }
 
 template <class Op>
-inline bool
-StackIter::forEachCanonicalActualArg(Op op, unsigned start /* = 0 */, unsigned count /* = unsigned(-1) */)
+inline void
+StackIter::ionForEachCanonicalActualArg(Op op)
 {
-    switch (state_) {
-      case DONE:
-        break;
-      case SCRIPTED:
-        JS_ASSERT(isFunctionFrame());
-        return fp()->forEachCanonicalActualArg(op, start, count);
-      case ION:
+    JS_ASSERT(isIon());
 #ifdef JS_ION
-        return ionInlineFrames_.forEachCanonicalActualArg(op, start, count);
+    ionInlineFrames_.forEachCanonicalActualArg(op, 0, -1);
 #endif
-      case NATIVE:
-        JS_NOT_REACHED("Unused ?");
-        return false;
-    }
-    JS_NOT_REACHED("Unexpected state");
-    return false;
 }
 
 } /* namespace js */
 #endif /* Stack_inl_h__ */
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -1095,17 +1095,18 @@ ContextStack::pushBailoutArgs(JSContext 
         return false;
 
     JSFunction *fun = it.callee();
     iag->setCallee(ObjectValue(*fun));
 
     CopyTo dst(iag->array());
     Value *src = it.actualArgs();
     Value thisv = iag->thisv();
-    return s.readFrameArgs(dst, src, NULL, &thisv, 0, fun->nargs, argc);
+    s.readFrameArgs(dst, src, NULL, &thisv, 0, fun->nargs, argc);
+    return true;
 }
 
 StackFrame *
 ContextStack::pushBailoutFrame(JSContext *cx, const ion::IonBailoutIterator &it,
                                const CallArgs &args, BailoutFrameGuard *bfg)
 {
     JSFunction *fun = it.callee();
     return pushInvokeFrame(cx, DONT_REPORT_ERROR, args, fun, INITIAL_NONE, bfg);
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1781,17 +1781,17 @@ class StackIter
 
     // These are only valid for the top frame.
     size_t      numFrameSlots() const;
     Value       frameSlotValue(size_t index) const;
 
     CallArgs nativeArgs() const { JS_ASSERT(isNativeCall()); return args_; }
 
     template <class Op>
-    inline bool forEachCanonicalActualArg(Op op, unsigned start = 0, unsigned count = unsigned(-1));
+    inline void ionForEachCanonicalActualArg(Op op);
 };
 
 /* A filtering of the StackIter to only stop at scripts. */
 class ScriptFrameIter : public StackIter
 {
     void settle() {
         while (!done() && !isScript())
             StackIter::operator++();