Bug 1402649 - Fix valid GC cell pointer asserts and error handling in module instantiation r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 26 Sep 2017 10:23:14 +0100
changeset 670377 f9df54bc39999524555f6d24ff609627fdc7553c
parent 670376 f2881b8508abd84066418c42f1413bdc481262b9
child 670378 998dd0100a61ba5ceafe8fe27b13753b3537a8bd
push id81612
push userbmo:dharvey@mozilla.com
push dateTue, 26 Sep 2017 10:16:26 +0000
reviewerssfink
bugs1402649
milestone58.0a1
Bug 1402649 - Fix valid GC cell pointer asserts and error handling in module instantiation r=sfink
js/public/GCPolicyAPI.h
js/public/HeapAPI.h
js/public/RootingAPI.h
js/src/builtin/ModuleObject.cpp
js/src/gc/Policy.h
js/src/jit-test/tests/modules/bug-1402649.js
js/src/jsgc.cpp
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -126,17 +126,17 @@ struct GCPointerPolicy
             js::UnsafeTraceManuallyBarrieredEdge(trc, vp, name);
     }
     static bool needsSweep(T* vp) {
         if (*vp)
             return js::gc::IsAboutToBeFinalizedUnbarriered(vp);
         return false;
     }
     static bool isValid(T v) {
-        return js::gc::IsCellPointerValid(v);
+        return js::gc::IsCellPointerValidOrNull(v);
     }
 };
 template <> struct GCPolicy<JS::Symbol*> : public GCPointerPolicy<JS::Symbol*> {};
 template <> struct GCPolicy<JSAtom*> : public GCPointerPolicy<JSAtom*> {};
 template <> struct GCPolicy<JSFunction*> : public GCPointerPolicy<JSFunction*> {};
 template <> struct GCPolicy<JSObject*> : public GCPointerPolicy<JSObject*> {};
 template <> struct GCPolicy<JSScript*> : public GCPointerPolicy<JSScript*> {};
 template <> struct GCPolicy<JSString*> : public GCPointerPolicy<JSString*> {};
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -401,29 +401,35 @@ IsInsideNursery(const js::gc::Cell* cell
     auto location = detail::GetCellLocation(cell);
     MOZ_ASSERT(location == ChunkLocation::Nursery || location == ChunkLocation::TenuredHeap);
     return location == ChunkLocation::Nursery;
 }
 
 MOZ_ALWAYS_INLINE bool
 IsCellPointerValid(const void* cell)
 {
-    if (!cell)
-        return true;
     auto addr = uintptr_t(cell);
     if (addr < ChunkSize || addr % CellAlignBytes != 0)
         return false;
     auto location = detail::GetCellLocation(cell);
     if (location == ChunkLocation::TenuredHeap)
         return !!detail::GetGCThingZone(addr);
     if (location == ChunkLocation::Nursery)
         return detail::NurseryCellHasStoreBuffer(cell);
     return false;
 }
 
+MOZ_ALWAYS_INLINE bool
+IsCellPointerValidOrNull(const void* cell)
+{
+    if (!cell)
+        return true;
+    return IsCellPointerValid(cell);
+}
+
 } /* namespace gc */
 } /* namespace js */
 
 namespace JS {
 
 static MOZ_ALWAYS_INLINE Zone*
 GetTenuredGCThingZone(GCCellPtr thing)
 {
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -397,17 +397,17 @@ class TenuredHeap : public js::HeapBase<
         static_assert(sizeof(T) == sizeof(TenuredHeap<T>),
                       "TenuredHeap<T> must be binary compatible with T.");
     }
     explicit TenuredHeap(T p) : bits(0) { setPtr(p); }
     explicit TenuredHeap(const TenuredHeap<T>& p) : bits(0) { setPtr(p.getPtr()); }
 
     void setPtr(T newPtr) {
         MOZ_ASSERT((reinterpret_cast<uintptr_t>(newPtr) & flagsMask) == 0);
-        MOZ_ASSERT(js::gc::IsCellPointerValid(newPtr));
+        MOZ_ASSERT(js::gc::IsCellPointerValidOrNull(newPtr));
         if (newPtr)
             AssertGCThingMustBeTenured(newPtr);
         bits = (bits & flagsMask) | reinterpret_cast<uintptr_t>(newPtr);
     }
 
     void setFlags(uintptr_t flagsToSet) {
         MOZ_ASSERT((flagsToSet & ~flagsMask) == 0);
         bits |= flagsToSet;
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -1073,16 +1073,19 @@ ModuleObject::instantiateFunctionDeclara
         if (fun->isAsync()) {
             if (fun->isStarGenerator()) {
                 obj = WrapAsyncGenerator(cx, obj.as<JSFunction>());
             } else {
                 obj = WrapAsyncFunction(cx, obj.as<JSFunction>());
             }
         }
 
+        if (!obj)
+            return false;
+
         value = ObjectValue(*obj);
         if (!SetProperty(cx, env, funDecl.name->asPropertyName(), value))
             return false;
     }
 
     js_delete(funDecls);
     self->setReservedSlot(FunctionDeclarationsSlot, UndefinedValue());
     return true;
--- a/js/src/gc/Policy.h
+++ b/js/src/gc/Policy.h
@@ -122,17 +122,17 @@ struct InternalGCPointerPolicy {
     static T initial() { return nullptr; }
     static void preBarrier(T v) { Type::writeBarrierPre(v); }
     static void postBarrier(T* vp, T prev, T next) { Type::writeBarrierPost(vp, prev, next); }
     static void readBarrier(T v) { Type::readBarrier(v); }
     static void trace(JSTracer* trc, T* vp, const char* name) {
         TraceManuallyBarrieredEdge(trc, vp, name);
     }
     static bool isValid(T v) {
-        return gc::IsCellPointerValid(v);
+        return gc::IsCellPointerValidOrNull(v);
     }
 };
 
 } // namespace js
 
 namespace JS {
 
 #define DEFINE_INTERNAL_GC_POLICY(type) \
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/modules/bug-1402649.js
@@ -0,0 +1,15 @@
+if (!('oomTest' in this))
+   quit();
+
+loadFile(`
+function parseAndEvaluate(source) {
+    let m = parseModule(source);
+    m.declarationInstantiation();
+}
+parseAndEvaluate("async function a() { await 2 + 3; }")
+`);
+function loadFile(lfVarx) {
+    oomTest(function() {
+        eval(lfVarx);
+    });
+}
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -8076,23 +8076,29 @@ JS::AssertGCThingIsNotAnObjectSubclass(C
 {
     MOZ_ASSERT(cell);
     MOZ_ASSERT(cell->getTraceKind() != JS::TraceKind::Object);
 }
 
 JS_FRIEND_API(void)
 js::gc::AssertGCThingHasType(js::gc::Cell* cell, JS::TraceKind kind)
 {
-    MOZ_ASSERT(IsCellPointerValid(cell));
-    if (!cell)
+    if (!cell) {
         MOZ_ASSERT(kind == JS::TraceKind::Null);
-    else if (IsInsideNursery(cell))
+        return;
+    }
+
+    MOZ_ASSERT(IsCellPointerValid(cell));
+
+    if (IsInsideNursery(cell)) {
         MOZ_ASSERT(kind == JS::TraceKind::Object);
-    else
-        MOZ_ASSERT(MapAllocToTraceKind(cell->asTenured().getAllocKind()) == kind);
+        return;
+    }
+
+    MOZ_ASSERT(MapAllocToTraceKind(cell->asTenured().getAllocKind()) == kind);
 }
 #endif
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
 JS::AutoAssertNoGC::AutoAssertNoGC(JSContext* maybecx)
   : cx_(maybecx ? maybecx : TlsContext.get())
 {