Bug 1471062 - Pad source notes to target alignment. r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Wed, 26 Jun 2019 20:49:43 +0000
changeset 543071 f6226246197b62d0bf4d1a733789ad3e480d00b1
parent 543070 0964e2a0ed2905151cef7aab24722ae133085715
child 543072 8718427bcb0f63c8c9789301a8b4efdb5588cad8
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1471062
milestone69.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 1471062 - Pad source notes to target alignment. r=jandem Pad the source notes with SRC_NULL such that the code and notes arrays together maintain uint32_t alignment. This is will later be used to add optional trailing arrays with uint32_t alignment. The allocator would already be rounding up our allocation so actually memory usage should be neutral. Differential Revision: https://phabricator.services.mozilla.com/D34788
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -9577,24 +9577,16 @@ bool BytecodeEmitter::setSrcNoteOffset(u
     *sn++ = (jssrcnote)(SN_4BYTE_OFFSET_FLAG | (offsetValue >> 24));
     *sn++ = (jssrcnote)(offsetValue >> 16);
     *sn++ = (jssrcnote)(offsetValue >> 8);
   }
   *sn = (jssrcnote)offsetValue;
   return true;
 }
 
-void BytecodeEmitter::copySrcNotes(jssrcnote* destination, uint32_t nsrcnotes) {
-  unsigned count = bytecodeSection().notes().length();
-  // nsrcnotes includes SN_MAKE_TERMINATOR in addition to the srcnotes.
-  MOZ_ASSERT(nsrcnotes == count + 1);
-  PodCopy(destination, bytecodeSection().notes().begin(), count);
-  SN_MAKE_TERMINATOR(&destination[count]);
-}
-
 const JSSrcNoteSpec js_SrcNoteSpec[] = {
 #define DEFINE_SRC_NOTE_SPEC(sym, name, arity) {name, arity},
     FOR_EACH_SRC_NOTE_TYPE(DEFINE_SRC_NOTE_SPEC)
 #undef DEFINE_SRC_NOTE_SPEC
 };
 
 static int SrcNoteArity(jssrcnote* sn) {
   MOZ_ASSERT(SN_TYPE(sn) < SRC_LAST);
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -370,17 +370,16 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   // notes dynamic array, updating noteCount. Return the new note's index
   // within the array pointed at by current->notes as outparam.
   MOZ_MUST_USE bool newSrcNote(SrcNoteType type, unsigned* indexp = nullptr);
   MOZ_MUST_USE bool newSrcNote2(SrcNoteType type, ptrdiff_t offset,
                                 unsigned* indexp = nullptr);
   MOZ_MUST_USE bool newSrcNote3(SrcNoteType type, ptrdiff_t offset1,
                                 ptrdiff_t offset2, unsigned* indexp = nullptr);
 
-  void copySrcNotes(jssrcnote* destination, uint32_t nsrcnotes);
   MOZ_MUST_USE bool setSrcNoteOffset(unsigned index, unsigned which,
                                      BytecodeOffsetDiff offset);
 
   // Control whether emitTree emits a line number note.
   enum EmitLineNumberNote { EMIT_LINENOTE, SUPPRESS_LINENOTE };
 
   // Emit code for the tree rooted at pn.
   MOZ_MUST_USE bool emitTree(ParseNode* pn,
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -721,26 +721,35 @@ SharedScriptData::SharedScriptData(uint3
 
   // Default-initialize trailing arrays.
 
   static_assert(alignof(SharedScriptData) >= alignof(GCPtrAtom),
                 "Incompatible alignment");
   initElements<GCPtrAtom>(cursor, natoms);
   cursor += natoms * sizeof(GCPtrAtom);
 
-  static_assert(alignof(GCPtrAtom) >= alignof(jsbytecode),
-                "Incompatible alignment");
-  codeOffset_ = cursor;
-  initElements<jsbytecode>(cursor, codeLength);
-  cursor += codeLength * sizeof(jsbytecode);
-
-  static_assert(alignof(jsbytecode) >= alignof(jssrcnote),
-                "Incompatible alignment");
-  initElements<jssrcnote>(cursor, noteLength);
-  cursor += noteLength * sizeof(jssrcnote);
+  // The following arrays are byte-aligned with additional padding to ensure
+  // that together they maintain uint32_t-alignment.
+  {
+    MOZ_ASSERT(cursor % CodeNoteAlign == 0);
+
+    static_assert(CodeNoteAlign >= alignof(jsbytecode),
+                  "Incompatible alignment");
+    codeOffset_ = cursor;
+    initElements<jsbytecode>(cursor, codeLength);
+    cursor += codeLength * sizeof(jsbytecode);
+
+    static_assert(alignof(jsbytecode) >= alignof(jssrcnote),
+                  "Incompatible alignment");
+    initElements<jssrcnote>(cursor, noteLength);
+    cursor += noteLength * sizeof(jssrcnote);
+
+    MOZ_ASSERT(cursor % CodeNoteAlign == 0);
+  }
+
   tailOffset_ = cursor;
 
   // Check that we correctly recompute the expected values.
   MOZ_ASSERT(this->natoms() == natoms);
   MOZ_ASSERT(this->codeLength() == codeLength);
   MOZ_ASSERT(this->noteLength() == noteLength);
 
   // Sanity check
@@ -3397,16 +3406,18 @@ SharedScriptData* js::SharedScriptData::
 
   // Constuct the SharedScriptData. Trailing arrays are uninitialized but
   // GCPtrs are put into a safe state.
   return new (raw) SharedScriptData(codeLength, noteLength, natoms);
 }
 
 bool JSScript::createSharedScriptData(JSContext* cx, uint32_t codeLength,
                                       uint32_t noteLength, uint32_t natoms) {
+  MOZ_ASSERT((codeLength + noteLength) % sizeof(uint32_t) == 0,
+             "Source notes should have been padded already");
   MOZ_ASSERT(!scriptData_);
   scriptData_ = SharedScriptData::new_(cx, codeLength, noteLength, natoms);
   return !!scriptData_;
 }
 
 void JSScript::freeScriptData() { scriptData_ = nullptr; }
 
 /*
@@ -3875,27 +3886,29 @@ bool JSScript::initFunctionPrototype(JSC
   if (!functionProtoScope) {
     return false;
   }
 
   mozilla::Span<JS::GCCellPtr> gcthings = script->data_->gcthings();
   gcthings[0] = JS::GCCellPtr(functionProtoScope);
 
   uint32_t codeLength = 1;
-  uint32_t noteLength = 1;
+  uint32_t noteLength = 3;
   uint32_t numAtoms = 0;
   if (!script->createSharedScriptData(cx, codeLength, noteLength, numAtoms)) {
     return false;
   }
 
   jsbytecode* code = script->scriptData_->code();
   code[0] = JSOP_RETRVAL;
 
   jssrcnote* notes = script->scriptData_->notes();
   notes[0] = SRC_NULL;
+  notes[1] = SRC_NULL;
+  notes[2] = SRC_NULL;
 
   return script->shareScriptData(cx);
 }
 
 static void InitAtomMap(frontend::AtomIndexMap& indices, GCPtrAtom* atoms) {
   for (frontend::AtomIndexMap::Range r = indices.all(); !r.empty();
        r.popFront()) {
     JSAtom* atom = r.front().key();
@@ -4933,26 +4946,28 @@ bool JSScript::hasBreakpointsAt(jsbyteco
 /* static */ bool SharedScriptData::InitFromEmitter(
     JSContext* cx, js::HandleScript script, frontend::BytecodeEmitter* bce,
     uint32_t nslots) {
   uint32_t natoms = bce->perScriptData().atomIndices()->count();
 
   size_t codeLength = bce->bytecodeSection().code().length();
   MOZ_RELEASE_ASSERT(codeLength <= frontend::MaxBytecodeLength);
 
-  // The + 1 is to account for the final SN_MAKE_TERMINATOR that is appended
-  // when the notes are copied to their final destination by copySrcNotes.
-  static_assert(frontend::MaxSrcNotesLength < UINT32_MAX,
-                "Length + 1 shouldn't overflow UINT32_MAX");
-  size_t noteLengthNoTerminator = bce->bytecodeSection().notes().length();
-  size_t noteLength = noteLengthNoTerminator + 1;
-  MOZ_RELEASE_ASSERT(noteLengthNoTerminator <= frontend::MaxSrcNotesLength);
+  // There are 1-4 copies of SN_MAKE_TERMINATOR appended after the source
+  // notes. These are a combination of sentinel and padding values.
+  static_assert(frontend::MaxSrcNotesLength <= UINT32_MAX - CodeNoteAlign,
+                "Length + CodeNoteAlign shouldn't overflow UINT32_MAX");
+  size_t noteLength = bce->bytecodeSection().notes().length();
+  MOZ_RELEASE_ASSERT(noteLength <= frontend::MaxSrcNotesLength);
+
+  size_t nullLength = ComputeNotePadding(codeLength, noteLength);
 
   // Create and initialize SharedScriptData
-  if (!script->createSharedScriptData(cx, codeLength, noteLength, natoms)) {
+  if (!script->createSharedScriptData(cx, codeLength, noteLength + nullLength,
+                                      natoms)) {
     return false;
   }
 
   js::SharedScriptData* data = script->scriptData_;
 
   // Initialize POD fields
   data->mainOffset = bce->mainOffset();
   data->nfixed = bce->maxFixedSlots;
@@ -4963,19 +4978,21 @@ bool JSScript::hasBreakpointsAt(jsbyteco
       std::min<uint32_t>(uint32_t(JSScript::MaxBytecodeTypeSets),
                          bce->bytecodeSection().numTypeSets());
 
   if (bce->sc->isFunctionBox()) {
     data->funLength = bce->sc->asFunctionBox()->length;
   }
 
   // Initialize trailing arrays
+  InitAtomMap(*bce->perScriptData().atomIndices(), data->atoms());
   std::copy_n(bce->bytecodeSection().code().begin(), codeLength, data->code());
-  bce->copySrcNotes(data->notes(), noteLength);
-  InitAtomMap(*bce->perScriptData().atomIndices(), data->atoms());
+  std::copy_n(bce->bytecodeSection().notes().begin(), noteLength,
+              data->notes());
+  std::fill_n(data->notes() + noteLength, nullLength, SRC_NULL);
 
   return true;
 }
 
 void SharedScriptData::traceChildren(JSTracer* trc) {
   MOZ_ASSERT(refCount() != 0);
   for (uint32_t i = 0; i < natoms(); ++i) {
     TraceNullableEdge(trc, &atoms()[i], "atom");
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1673,16 +1673,32 @@ class alignas(uintptr_t) SharedScriptDat
 
   // Initialize to GC-safe state
   SharedScriptData(uint32_t codeLength, uint32_t noteLength, uint32_t natoms);
 
  public:
   static SharedScriptData* new_(JSContext* cx, uint32_t codeLength,
                                 uint32_t noteLength, uint32_t natoms);
 
+  // The code() and note() arrays together maintain an target alignment by
+  // padding the source notes with null. This allows arrays with stricter
+  // alignment requirements to follow them.
+  static constexpr size_t CodeNoteAlign = sizeof(uint32_t);
+
+  // Compute number of null notes to pad out source notes with.
+  static uint32_t ComputeNotePadding(uint32_t codeLength, uint32_t noteLength) {
+    uint32_t nullLength =
+        CodeNoteAlign - (codeLength + noteLength) % CodeNoteAlign;
+
+    // The source notes must have at least one null-terminator.
+    MOZ_ASSERT(nullLength >= 1);
+
+    return nullLength;
+  }
+
   uint32_t refCount() const { return refCount_; }
   void AddRef() { refCount_++; }
   void Release() {
     MOZ_ASSERT(refCount_ != 0);
     uint32_t remain = --refCount_;
     if (remain == 0) {
       js_free(this);
     }