Bug 1260371 - Rearrange RelocationOverlay so that magic field does not overlay inline string chars r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 30 Mar 2016 12:33:55 +0100
changeset 291057 24b56ce6d8db255d1fbf691753ab07f99c07b3c5
parent 291056 457e7d5b11363cf859bb4c476701cf4803f452c4
child 291058 6c7e6cf636afde50f2dc72099c75e8b5125b97d3
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)
reviewersterrence
bugs1260371
milestone48.0a1
Bug 1260371 - Rearrange RelocationOverlay so that magic field does not overlay inline string chars r=terrence
js/src/builtin/TypedObject.cpp
js/src/builtin/TypedObject.h
js/src/jsgc.h
js/src/jsgcinlines.h
js/src/vm/Debugger.cpp
js/src/vm/Shape.h
js/src/vm/String.h
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -1024,17 +1024,17 @@ size_t
 StructTypeDescr::fieldCount() const
 {
     return fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_NAMES).getDenseInitializedLength();
 }
 
 size_t
 StructTypeDescr::maybeForwardedFieldCount() const
 {
-    ArrayObject& fieldInfo = *MaybeForwarded(&fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_NAMES));
+    ArrayObject& fieldInfo = maybeForwardedFieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_NAMES);
     return fieldInfo.getDenseInitializedLength();
 }
 
 bool
 StructTypeDescr::fieldIndex(jsid id, size_t* out) const
 {
     ArrayObject& fieldNames = fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_NAMES);
     size_t l = fieldNames.getDenseInitializedLength();
@@ -1060,33 +1060,33 @@ StructTypeDescr::fieldOffset(size_t inde
     ArrayObject& fieldOffsets = fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_OFFSETS);
     MOZ_ASSERT(index < fieldOffsets.getDenseInitializedLength());
     return AssertedCast<size_t>(fieldOffsets.getDenseElement(index).toInt32());
 }
 
 size_t
 StructTypeDescr::maybeForwardedFieldOffset(size_t index) const
 {
-    ArrayObject& fieldOffsets = *MaybeForwarded(&fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_OFFSETS));
+    ArrayObject& fieldOffsets = maybeForwardedFieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_OFFSETS);
     MOZ_ASSERT(index < fieldOffsets.getDenseInitializedLength());
     return AssertedCast<size_t>(fieldOffsets.getDenseElement(index).toInt32());
 }
 
 TypeDescr&
 StructTypeDescr::fieldDescr(size_t index) const
 {
     ArrayObject& fieldDescrs = fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_TYPES);
     MOZ_ASSERT(index < fieldDescrs.getDenseInitializedLength());
     return fieldDescrs.getDenseElement(index).toObject().as<TypeDescr>();
 }
 
 TypeDescr&
 StructTypeDescr::maybeForwardedFieldDescr(size_t index) const
 {
-    ArrayObject& fieldDescrs = *MaybeForwarded(&fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_TYPES));
+    ArrayObject& fieldDescrs = maybeForwardedFieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_TYPES);
     MOZ_ASSERT(index < fieldDescrs.getDenseInitializedLength());
     return MaybeForwarded(&fieldDescrs.getDenseElement(index).toObject())->as<TypeDescr>();
 }
 
 /******************************************************************************
  * Creating the TypedObject "module"
  *
  * We create one global, `TypedObject`, which contains the following
--- a/js/src/builtin/TypedObject.h
+++ b/js/src/builtin/TypedObject.h
@@ -462,16 +462,19 @@ class StructTypeDescr : public ComplexTy
     // Return the offset of the field at index `index`.
     size_t fieldOffset(size_t index) const;
     size_t maybeForwardedFieldOffset(size_t index) const;
 
   private:
     ArrayObject& fieldInfoObject(size_t slot) const {
         return getReservedSlot(slot).toObject().as<ArrayObject>();
     }
+    ArrayObject& maybeForwardedFieldInfoObject(size_t slot) const {
+        return MaybeForwarded(&getReservedSlot(slot).toObject())->as<ArrayObject>();
+    }
 };
 
 typedef Handle<StructTypeDescr*> HandleStructTypeDescr;
 
 /*
  * This object exists in order to encapsulate the typed object types
  * somewhat, rather than sticking them all into the global object.
  * Eventually it will go away and become a module.
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1029,50 +1029,40 @@ MergeCompartments(JSCompartment* source,
  * This structure overlays a Cell in the Nursery and re-purposes its memory
  * for managing the Nursery collection process.
  */
 class RelocationOverlay
 {
     /* The low bit is set so this should never equal a normal pointer. */
     static const uintptr_t Relocated = uintptr_t(0xbad0bad1);
 
-    // Arrange the fields of the RelocationOverlay so that JSObject's group
-    // pointer is not overwritten during compacting.
-
-    /* A list entry to track all relocated things. */
-    RelocationOverlay* next_;
-
     /* Set to Relocated when moved. */
     uintptr_t magic_;
 
     /* The location |this| was moved to. */
     Cell* newLocation_;
 
+    /* A list entry to track all relocated things. */
+    RelocationOverlay* next_;
+
   public:
     static RelocationOverlay* fromCell(Cell* cell) {
         return reinterpret_cast<RelocationOverlay*>(cell);
     }
 
     bool isForwarded() const {
         return magic_ == Relocated;
     }
 
     Cell* forwardingAddress() const {
         MOZ_ASSERT(isForwarded());
         return newLocation_;
     }
 
-    void forwardTo(Cell* cell) {
-        MOZ_ASSERT(!isForwarded());
-        static_assert(offsetof(JSObject, group_) == offsetof(RelocationOverlay, next_),
-                      "next pointer and group should be at same location, "
-                      "so that group is not overwritten during compacting");
-        newLocation_ = cell;
-        magic_ = Relocated;
-    }
+    void forwardTo(Cell* cell);
 
     RelocationOverlay*& nextRef() {
         MOZ_ASSERT(isForwarded());
         return next_;
     }
 
     RelocationOverlay* next() const {
         MOZ_ASSERT(isForwarded());
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -327,12 +327,26 @@ class GCZoneGroupIter {
     }
 
     operator JS::Zone*() const { return get(); }
     JS::Zone* operator->() const { return get(); }
 };
 
 typedef CompartmentsIterT<GCZoneGroupIter> GCCompartmentGroupIter;
 
+inline void
+RelocationOverlay::forwardTo(Cell* cell)
+{
+    MOZ_ASSERT(!isForwarded());
+    // The location of magic_ is important because it must never be valid to see
+    // the value Relocated there in a GC thing that has not been moved.
+    static_assert(offsetof(RelocationOverlay, magic_) == offsetof(JSObject, group_) &&
+                  offsetof(RelocationOverlay, magic_) == offsetof(js::Shape, base_) &&
+                  offsetof(RelocationOverlay, magic_) == offsetof(JSString, d.u1.flags),
+                  "RelocationOverlay::magic_ is in the wrong location");
+    newLocation_ = cell;
+    magic_ = Relocated;
+}
+
 } /* namespace gc */
 } /* namespace js */
 
 #endif /* jsgcinlines_h */
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2543,17 +2543,17 @@ Debugger::markCrossCompartmentEdges(JSTr
 /* static */ void
 Debugger::markIncomingCrossCompartmentEdges(JSTracer* trc)
 {
     JSRuntime* rt = trc->runtime();
     gc::State state = rt->gc.state();
     MOZ_ASSERT(state == gc::MARK_ROOTS || state == gc::COMPACT);
 
     for (Debugger* dbg : rt->debuggerList) {
-        Zone* zone = dbg->object->zone();
+        Zone* zone = MaybeForwarded(dbg->object.get())->zone();
         if ((state == gc::MARK_ROOTS && !zone->isCollecting()) ||
             (state == gc::COMPACT && !zone->isGCCompacting()))
         {
             dbg->markCrossCompartmentEdges(trc);
         }
     }
 }
 
@@ -4817,28 +4817,30 @@ GetScriptReferent(JSObject* obj)
     }
     return AsVariant(static_cast<JSScript*>(nullptr));
 }
 
 void
 DebuggerScript_trace(JSTracer* trc, JSObject* obj)
 {
     /* This comes from a private pointer, so no barrier needed. */
-    DebuggerScriptReferent referent = GetScriptReferent(obj);
-    if (referent.is<JSScript*>()) {
-        if (JSScript* script = referent.as<JSScript*>()) {
+    gc::Cell* cell = GetScriptReferentCell(obj);
+    if (cell) {
+        if (cell->getTraceKind() == JS::TraceKind::Script) {
+            JSScript* script = static_cast<JSScript*>(cell);
             TraceManuallyBarrieredCrossCompartmentEdge(trc, obj, &script,
                                                        "Debugger.Script script referent");
             obj->as<NativeObject>().setPrivateUnbarriered(script);
-        }
-    } else {
-        JSObject* wasm = referent.as<WasmModuleObject*>();
-        TraceManuallyBarrieredCrossCompartmentEdge(trc, obj, &wasm,
-                                                   "Debugger.Script wasm referent");
-        obj->as<NativeObject>().setPrivateUnbarriered(wasm);
+        } else {
+            JSObject* wasm = static_cast<JSObject*>(cell);
+            TraceManuallyBarrieredCrossCompartmentEdge(trc, obj, &wasm,
+                                                       "Debugger.Script wasm referent");
+            MOZ_ASSERT(wasm->is<WasmModuleObject>());
+            obj->as<NativeObject>().setPrivateUnbarriered(wasm);
+        }
     }
 }
 
 const Class DebuggerScript_class = {
     "Script",
     JSCLASS_HAS_PRIVATE |
     JSCLASS_HAS_RESERVED_SLOTS(JSSLOT_DEBUGSCRIPT_COUNT),
     nullptr, nullptr, nullptr, nullptr,
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -535,16 +535,17 @@ class Shape : public gc::TenuredCell
     friend class Bindings;
     friend class NativeObject;
     friend class PropertyTree;
     friend class StaticBlockScope;
     friend class TenuringTracer;
     friend struct StackBaseShape;
     friend struct StackShape;
     friend struct JS::ubi::Concrete<Shape>;
+    friend class js::gc::RelocationOverlay;
 
   protected:
     HeapPtrBaseShape    base_;
     PreBarrieredId      propid_;
 
     enum SlotInfo : uint32_t
     {
         /* Number of fixed slots in objects with this shape. */
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -311,16 +311,18 @@ class JSString : public js::gc::TenuredC
                       "shadow::String::TYPE_FLAGS_MASK must match JSString::TYPE_FLAGS_MASK");
         static_assert(ROPE_FLAGS == String::ROPE_FLAGS,
                       "shadow::String::ROPE_FLAGS must match JSString::ROPE_FLAGS");
     }
 
     /* Avoid lame compile errors in JSRope::flatten */
     friend class JSRope;
 
+    friend class js::gc::RelocationOverlay;
+
   protected:
     template <typename CharT>
     MOZ_ALWAYS_INLINE
     void setNonInlineChars(const CharT* chars);
 
   public:
     /* All strings have length. */