Bug 1437533 - Don't use memset to initialize JSFunction extended slots. Parts of this patch written by Waldo. r=jwalden
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 08 May 2018 11:29:32 +0200
changeset 471508 064597ea4795213662690a40bc01dd70a0f719ee
parent 471507 e640d678c9881de3f465257aac6858e34c76b034
child 471509 d439053dcb4a8f88ca1be465cc01745b088852ed
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1437533
milestone61.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 1437533 - Don't use memset to initialize JSFunction extended slots. Parts of this patch written by Waldo. r=jwalden
js/src/vm/JSFunction-inl.h
js/src/vm/JSFunction.h
js/src/vm/JSObject.cpp
js/src/vm/NativeObject-inl.h
--- a/js/src/vm/JSFunction-inl.h
+++ b/js/src/vm/JSFunction-inl.h
@@ -4,18 +4,22 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef vm_JSFunction_inl_h
 #define vm_JSFunction_inl_h
 
 #include "vm/JSFunction.h"
 
+#include "gc/Allocator.h"
+#include "gc/GCTrace.h"
 #include "vm/EnvironmentObject.h"
 
+#include "vm/JSObject-inl.h"
+
 namespace js {
 
 inline const char*
 GetFunctionNameBytes(JSContext* cx, JSFunction* fun, JSAutoByteString* bytes)
 {
     if (JSAtom* name = fun->explicitName())
         return bytes->encodeLatin1(cx, name);
     return js_anonymous_str;
@@ -81,9 +85,68 @@ CloneFunctionObjectIfNotSingleton(JSCont
     if (!script)
         return nullptr;
     RootedScope enclosingScope(cx, script->enclosingScope());
     return CloneFunctionAndScript(cx, fun, parent, enclosingScope, kind, proto);
 }
 
 } /* namespace js */
 
+/* static */ inline JS::Result<JSFunction*, JS::OOM&>
+JSFunction::create(JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap heap,
+                   js::HandleShape shape, js::HandleObjectGroup group)
+{
+    MOZ_ASSERT(kind == js::gc::AllocKind::FUNCTION ||
+               kind == js::gc::AllocKind::FUNCTION_EXTENDED);
+
+    debugCheckNewObject(group, shape, kind, heap);
+
+    const js::Class* clasp = group->clasp();
+    MOZ_ASSERT(clasp->isJSFunction());
+
+    static constexpr size_t NumDynamicSlots = 0;
+    MOZ_ASSERT(dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), clasp) ==
+               NumDynamicSlots);
+
+    JSObject* obj = js::Allocate<JSObject>(cx, kind, NumDynamicSlots, heap, clasp);
+    if (!obj)
+        return cx->alreadyReportedOOM();
+
+    NativeObject* nobj = static_cast<NativeObject*>(obj);
+    nobj->initGroup(group);
+    nobj->initShape(shape);
+
+    nobj->initSlots(nullptr);
+    nobj->setEmptyElements();
+
+    MOZ_ASSERT(!clasp->hasPrivate());
+    MOZ_ASSERT(shape->slotSpan() == 0);
+
+    JSFunction* fun = static_cast<JSFunction*>(nobj);
+    fun->nargs_ = 0;
+
+    // This must be overwritten by some ultimate caller: there's no default
+    // value to which we could sensibly initialize this.
+    MOZ_MAKE_MEM_UNDEFINED(&fun->u, sizeof(u));
+
+    // Safe: we're initializing for the very first time.
+    fun->atom_.unsafeSet(nullptr);
+
+    if (kind == js::gc::AllocKind::FUNCTION_EXTENDED) {
+        fun->setFlags(JSFunction::EXTENDED);
+        for (js::GCPtrValue& extendedSlot : fun->toExtended()->extendedSlots)
+            extendedSlot.unsafeSet(JS::DoubleValue(+0.0));
+    } else {
+        fun->setFlags(0);
+    }
+
+    MOZ_ASSERT(!clasp->shouldDelayMetadataBuilder(),
+               "Function has no extra data hanging off it, that wouldn't be "
+               "allocated at this point, that would require delaying the "
+               "building of metadata for it");
+    fun = SetNewObjectMetadata(cx, fun);
+
+    js::gc::TraceCreateObject(fun);
+
+    return fun;
+}
+
 #endif /* vm_JSFunction_inl_h */
--- a/js/src/vm/JSFunction.h
+++ b/js/src/vm/JSFunction.h
@@ -179,16 +179,20 @@ class JSFunction : public js::NativeObje
     //   b. If HAS_BOUND_FUNCTION_NAME_PREFIX is not set, |atom_| doesn't
     //      contain the "bound " prefix which is prepended to the "name"
     //      property of bound functions per ECMAScript.
     //   c. Bound functions can never have an inferred or guessed name.
     //   d. |atom_| is never null for bound functions.
     js::GCPtrAtom atom_;
 
   public:
+    static inline JS::Result<JSFunction*, JS::OOM&>
+    create(JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap heap,
+           js::HandleShape shape, js::HandleObjectGroup group);
+
     /* Call objects must be created for each invocation of this function. */
     bool needsCallObject() const {
         MOZ_ASSERT(!isInterpretedLazy());
 
         if (isNative())
             return false;
 
         // Note: this should be kept in sync with
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -57,16 +57,17 @@
 #include "gc/Marking-inl.h"
 #include "vm/ArrayObject-inl.h"
 #include "vm/BooleanObject-inl.h"
 #include "vm/Caches-inl.h"
 #include "vm/Interpreter-inl.h"
 #include "vm/JSAtom-inl.h"
 #include "vm/JSCompartment-inl.h"
 #include "vm/JSContext-inl.h"
+#include "vm/JSFunction-inl.h"
 #include "vm/NativeObject-inl.h"
 #include "vm/NumberObject-inl.h"
 #include "vm/Shape-inl.h"
 #include "vm/StringObject-inl.h"
 #include "vm/TypedArrayObject-inl.h"
 #include "vm/UnboxedObject-inl.h"
 
 using namespace js;
@@ -722,17 +723,19 @@ NewObject(JSContext* cx, HandleObjectGro
     RootedShape shape(cx, EmptyShape::getInitialShape(cx, clasp, group->proto(), nfixed,
                                                       initialShapeFlags));
     if (!shape)
         return nullptr;
 
     gc::InitialHeap heap = GetInitialHeap(newKind, clasp);
 
     JSObject* obj;
-    if (MOZ_LIKELY(clasp->isNative())) {
+    if (clasp->isJSFunction()) {
+        JS_TRY_VAR_OR_RETURN_NULL(cx, obj, JSFunction::create(cx, kind, heap, shape, group));
+    } else if (MOZ_LIKELY(clasp->isNative())) {
         JS_TRY_VAR_OR_RETURN_NULL(cx, obj, NativeObject::create(cx, kind, heap, shape, group));
     } else {
         MOZ_ASSERT(IsTypedObjectClass(clasp));
         JS_TRY_VAR_OR_RETURN_NULL(cx, obj, TypedObject::create(cx, kind, heap, shape, group));
     }
 
     if (newKind == SingletonObject) {
         RootedObject nobj(cx, obj);
--- a/js/src/vm/NativeObject-inl.h
+++ b/js/src/vm/NativeObject-inl.h
@@ -528,16 +528,17 @@ NativeObject::isInWholeCellBuffer() cons
 /* static */ inline JS::Result<NativeObject*, JS::OOM&>
 NativeObject::create(JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap heap,
                      js::HandleShape shape, js::HandleObjectGroup group)
 {
     debugCheckNewObject(group, shape, kind, heap);
 
     const js::Class* clasp = group->clasp();
     MOZ_ASSERT(clasp->isNative());
+    MOZ_ASSERT(!clasp->isJSFunction(), "should use JSFunction::create");
 
     size_t nDynamicSlots = dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), clasp);
 
     JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);
     if (!obj)
         return cx->alreadyReportedOOM();
 
     NativeObject* nobj = static_cast<NativeObject*>(obj);
@@ -549,30 +550,16 @@ NativeObject::create(JSContext* cx, js::
     nobj->setEmptyElements();
 
     if (clasp->hasPrivate())
         nobj->initPrivate(nullptr);
 
     if (size_t span = shape->slotSpan())
         nobj->initializeSlotRange(0, span);
 
-    // JSFunction's fixed slots expect POD-style initialization.
-    if (clasp->isJSFunction()) {
-        MOZ_ASSERT(kind == js::gc::AllocKind::FUNCTION ||
-                   kind == js::gc::AllocKind::FUNCTION_EXTENDED);
-        size_t size =
-            kind == js::gc::AllocKind::FUNCTION ? sizeof(JSFunction) : sizeof(js::FunctionExtended);
-        memset(nobj->as<JSFunction>().fixedSlots(), 0, size - sizeof(js::NativeObject));
-        if (kind == js::gc::AllocKind::FUNCTION_EXTENDED) {
-            // SetNewObjectMetadata may gc, which will be unhappy if flags &
-            // EXTENDED doesn't match the arena's AllocKind.
-            nobj->as<JSFunction>().setFlags(JSFunction::EXTENDED);
-        }
-    }
-
     if (clasp->shouldDelayMetadataBuilder())
         cx->compartment()->setObjectPendingMetadata(cx, nobj);
     else
         nobj = SetNewObjectMetadata(cx, nobj);
 
     js::gc::TraceCreateObject(nobj);
 
     return nobj;