Bug 1257077 - Implement js::Fixed{Invoke,Construct}Args for args of statically-known count, avoiding js::{Invoke,Construct}Args's fallibility. Also implement js::Any{Invoke,Construct}Args as base classes for args, whether or not their count is statically known. r=efaust
authorJeff Walden <jwalden@mit.edu>
Fri, 18 Mar 2016 16:44:23 -0700
changeset 290182 4029d2aeb270615d941f0a441bc2555978e1fa11
parent 290181 9d73e42396c8b5d1a59fc6ebd1537e6e8599fdf3
child 290183 ca17f948015362597893a0a447295ed1286d3600
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1257077
milestone48.0a1
Bug 1257077 - Implement js::Fixed{Invoke,Construct}Args for args of statically-known count, avoiding js::{Invoke,Construct}Args's fallibility. Also implement js::Any{Invoke,Construct}Args as base classes for args, whether or not their count is statically known. r=efaust
js/src/jsarray.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
js/src/vm/Stack.h
js/src/vm/TypedArrayObject.cpp
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -3099,19 +3099,18 @@ array_of(JSContext* cx, unsigned argc, V
         // IsArrayConstructor(this) will usually be true in practice. This is
         // the most common path.
         return ArrayFromCallArgs(cx, args);
     }
 
     // Step 4.
     RootedObject obj(cx);
     {
-        ConstructArgs cargs(cx);
-        if (!cargs.init(1))
-            return false;
+        FixedConstructArgs<1> cargs(cx);
+
         cargs[0].setNumber(args.length());
 
         if (!Construct(cx, args.thisv(), cargs, args.thisv(), &obj))
             return false;
     }
 
     // Step 8.
     for (unsigned k = 0; k < args.length(); k++) {
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -528,39 +528,39 @@ js::Invoke(JSContext* cx, const Value& t
     if (!Invoke(cx, args))
         return false;
 
     rval.set(args.rval());
     return true;
 }
 
 static bool
-InternalConstruct(JSContext* cx, const CallArgs& args)
+InternalConstruct(JSContext* cx, const AnyConstructArgs& args)
 {
     MOZ_ASSERT(args.array() + args.length() + 1 == args.end(),
                "must pass constructing arguments to a construction attempt");
     MOZ_ASSERT(!JSFunction::class_.construct);
 
     // Callers are responsible for enforcing these preconditions.
     MOZ_ASSERT(IsConstructor(args.calleev()),
                "trying to construct a value that isn't a constructor");
-    MOZ_ASSERT(IsConstructor(args.newTarget()),
+    MOZ_ASSERT(IsConstructor(args.CallArgs::newTarget()),
                "provided new.target value must be a constructor");
 
     JSObject& callee = args.callee();
     if (callee.is<JSFunction>()) {
         RootedFunction fun(cx, &callee.as<JSFunction>());
 
         if (fun->isNative())
             return CallJSNativeConstructor(cx, fun->native(), args);
 
         if (!Invoke(cx, args, CONSTRUCT))
             return false;
 
-        MOZ_ASSERT(args.rval().isObject());
+        MOZ_ASSERT(args.CallArgs::rval().isObject());
         return true;
     }
 
     JSNative construct = callee.constructHook();
     MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?");
 
     return CallJSNativeConstructor(cx, construct, args);
 }
@@ -584,50 +584,52 @@ StackCheckIsConstructorCalleeNewTarget(J
 
 static bool
 ConstructFromStack(JSContext* cx, const CallArgs& args)
 {
     if (!StackCheckIsConstructorCalleeNewTarget(cx, args.calleev(), args.newTarget()))
         return false;
 
     args.setThis(MagicValue(JS_IS_CONSTRUCTING));
-    return InternalConstruct(cx, args);
+    return InternalConstruct(cx, static_cast<const AnyConstructArgs&>(args));
 }
 
 bool
-js::Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
+js::Construct(JSContext* cx, HandleValue fval, const AnyConstructArgs& args, HandleValue newTarget,
               MutableHandleObject objp)
 {
-    args.setCallee(fval);
-    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
-    args.newTarget().set(newTarget);
+    // Explicitly qualify to bypass AnyConstructArgs's deliberate shadowing.
+    args.CallArgs::setCallee(fval);
+    args.CallArgs::setThis(MagicValue(JS_IS_CONSTRUCTING));
+    args.CallArgs::newTarget().set(newTarget);
+
     if (!InternalConstruct(cx, args))
         return false;
 
-    MOZ_ASSERT(args.rval().isObject());
-    objp.set(&args.rval().toObject());
+    MOZ_ASSERT(args.CallArgs::rval().isObject());
+    objp.set(&args.CallArgs::rval().toObject());
     return true;
 }
 
 bool
 js::InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
-                                      const ConstructArgs& args, HandleValue newTarget,
+                                      const AnyConstructArgs& args, HandleValue newTarget,
                                       MutableHandleValue rval)
 {
-    args.setCallee(fval);
+    args.CallArgs::setCallee(fval);
 
     MOZ_ASSERT(thisv.isObject());
-    args.setThis(thisv);
-
-    args.newTarget().set(newTarget);
+    args.CallArgs::setThis(thisv);
+
+    args.CallArgs::newTarget().set(newTarget);
 
     if (!InternalConstruct(cx, args))
         return false;
 
-    rval.set(args.rval());
+    rval.set(args.CallArgs::rval());
     return true;
 }
 
 bool
 js::InvokeGetter(JSContext* cx, const Value& thisv, Value fval, MutableHandleValue rval)
 {
     /*
      * Invoke could result in another try to get or set the same id again, see
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -81,28 +81,28 @@ InvokeSetter(JSContext* cx, const Value&
 
 // ES6 7.3.13 Construct(F, argumentsList, newTarget).  All parameters are
 // required, hopefully forcing callers to be careful not to (say) blindly pass
 // callee as |newTarget| when a different value should have been passed.
 //
 // NOTE: As with the ES6 spec operation, it's the caller's responsibility to
 //       ensure |fval| and |newTarget| are both |IsConstructor|.
 extern bool
-Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
+Construct(JSContext* cx, HandleValue fval, const AnyConstructArgs& args, HandleValue newTarget,
           MutableHandleObject objp);
 
 // Call Construct(fval, args, newTarget), but use the given |thisv| as |this|
 // during construction of |fval|.
 //
 // This method exists only for very rare cases where a |this| was created
 // caller-side for construction of |fval|: basically only for JITs using
 // |CreateThis|.  If that's not you, use Construct()!
 extern bool
 InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
-                                  const ConstructArgs& args, HandleValue newTarget,
+                                  const AnyConstructArgs& args, HandleValue newTarget,
                                   MutableHandleValue rval);
 
 /*
  * Executes a script with the given scopeChain/this. The 'type' indicates
  * whether this is eval code or global code. To support debugging, the
  * evalFrame parameter can point to an arbitrary frame in the context's call
  * stack to simulate executing an eval in that frame.
  */
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -920,56 +920,110 @@ class InterpreterStack
         return allocator_.sizeOfExcludingThis(mallocSizeOf);
     }
 };
 
 void MarkInterpreterActivations(JSRuntime* rt, JSTracer* trc);
 
 /*****************************************************************************/
 
+/** Base class for all function call args. */
+class AnyInvokeArgs : public JS::CallArgs
+{
+};
+
+/** Base class for all function construction args. */
+class AnyConstructArgs : public JS::CallArgs
+{
+    // Only js::Construct (or internal methods that call the qualified CallArgs
+    // versions) should do these things!
+    void setCallee(Value v) = delete;
+    void setThis(Value v) = delete;
+    MutableHandleValue newTarget() const = delete;
+    MutableHandleValue rval() const = delete;
+};
+
 namespace detail {
 
-class GenericInvokeArgs : public JS::CallArgs
+/** Function call/construct args of statically-unknown count. */
+template <MaybeConstruct Construct>
+class GenericArgsBase
+  : public mozilla::Conditional<Construct, AnyConstructArgs, AnyInvokeArgs>::Type
 {
   protected:
     AutoValueVector v_;
 
-    explicit GenericInvokeArgs(JSContext* cx) : v_(cx) {}
+    explicit GenericArgsBase(JSContext* cx) : v_(cx) {}
 
-    bool init(unsigned argc, bool construct) {
-        MOZ_ASSERT(2 + argc + construct > argc);  // no overflow
-        if (!v_.resize(2 + argc + construct))
+  public:
+    bool init(unsigned argc) {
+        // callee, this, arguments[, new.target iff constructing]
+        size_t len = 2 + argc + uint32_t(Construct);
+        MOZ_ASSERT(len > argc);  // no overflow
+        if (!v_.resize(len))
             return false;
 
         *static_cast<JS::CallArgs*>(this) = CallArgsFromVp(argc, v_.begin());
-        constructing_ = construct;
+        this->constructing_ = Construct;
         return true;
     }
 };
 
+/** Function call/construct args of statically-known count. */
+template <MaybeConstruct Construct, size_t N>
+class FixedArgsBase
+  : public mozilla::Conditional<Construct, AnyConstructArgs, AnyInvokeArgs>::Type
+{
+  protected:
+    JS::AutoValueArray<2 + N + uint32_t(Construct)> v_;
+
+    explicit FixedArgsBase(JSContext* cx) : v_(cx) {
+        *static_cast<JS::CallArgs*>(this) = CallArgsFromVp(N, v_.begin());
+        this->constructing_ = Construct;
+    }
+};
+
 } // namespace detail
 
-class InvokeArgs : public detail::GenericInvokeArgs
+/** Function call args of statically-unknown count. */
+class InvokeArgs : public detail::GenericArgsBase<NO_CONSTRUCT>
 {
+    using Base = detail::GenericArgsBase<NO_CONSTRUCT>;
+
   public:
-    explicit InvokeArgs(JSContext* cx) : detail::GenericInvokeArgs(cx) {}
-
-    bool init(unsigned argc) {
-        return detail::GenericInvokeArgs::init(argc, false);
-    }
+    explicit InvokeArgs(JSContext* cx) : Base(cx) {}
 };
 
-class ConstructArgs : public detail::GenericInvokeArgs
+/** Function call args of statically-known count. */
+template <size_t N>
+class FixedInvokeArgs : public detail::FixedArgsBase<NO_CONSTRUCT, N>
 {
+    using Base = detail::FixedArgsBase<NO_CONSTRUCT, N>;
+
   public:
-    explicit ConstructArgs(JSContext* cx) : detail::GenericInvokeArgs(cx) {}
+    explicit FixedInvokeArgs(JSContext* cx) : Base(cx) {}
+};
+
+/** Function construct args of statically-unknown count. */
+class ConstructArgs : public detail::GenericArgsBase<CONSTRUCT>
+{
+    using Base = detail::GenericArgsBase<CONSTRUCT>;
 
-    bool init(unsigned argc) {
-        return detail::GenericInvokeArgs::init(argc, true);
-    }
+  public:
+    explicit ConstructArgs(JSContext* cx) : Base(cx) {}
+};
+
+/** Function call args of statically-known count. */
+template <size_t N>
+class FixedConstructArgs : public detail::FixedArgsBase<CONSTRUCT, N>
+{
+    using Base = detail::FixedArgsBase<CONSTRUCT, N>;
+
+  public:
+    explicit FixedConstructArgs(JSContext* cx) : Base(cx) {}
 };
 
 template <class Args, class Arraylike>
 inline bool
 FillArgumentsFromArraylike(JSContext* cx, Args& args, const Arraylike& arraylike)
 {
     uint32_t len = arraylike.length();
     if (!args.init(len))
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -2482,25 +2482,23 @@ JS_GetDataViewByteLength(JSObject* obj)
     if (!obj)
         return 0;
     return obj->as<DataViewObject>().byteLength();
 }
 
 JS_FRIEND_API(JSObject*)
 JS_NewDataView(JSContext* cx, HandleObject arrayBuffer, uint32_t byteOffset, int32_t byteLength)
 {
-    ConstructArgs cargs(cx);
-    if (!cargs.init(3))
-        return nullptr;
-
     RootedObject constructor(cx);
     JSProtoKey key = JSCLASS_CACHED_PROTO_KEY(&DataViewObject::class_);
     if (!GetBuiltinConstructor(cx, key, &constructor))
         return nullptr;
 
+    FixedConstructArgs<3> cargs(cx);
+
     cargs[0].setObject(*arrayBuffer);
     cargs[1].setNumber(byteOffset);
     cargs[2].setInt32(byteLength);
 
     RootedValue fun(cx, ObjectValue(*constructor));
     RootedObject obj(cx);
     if (!Construct(cx, fun, cargs, fun, &obj))
         return nullptr;