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

@@ -9,8 +9,12 @@
 
 #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*
@@ -86,4 +90,63 @@ CloneFunctionObjectIfNotSingleton(JSContext* cx, HandleFunction fun, HandleObjec
 
 } /* 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 */

js/src/vm/JSFunction.h

@@ -184,6 +184,10 @@ class JSFunction : public js::NativeObject
     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());

js/src/vm/JSObject.cpp

@@ -62,6 +62,7 @@
 #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"
@@ -727,7 +728,9 @@ NewObject(JSContext* cx, HandleObjectGroup group, gc::AllocKind kind,
     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/src/vm/NativeObject-inl.h

@@ -533,6 +533,7 @@ NativeObject::create(JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap
 
     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);
 
@@ -554,20 +555,6 @@ NativeObject::create(JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap
     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