Bug 1533636 - Remove all remaining calls to XDRState::codeAlign as no longer necessary now that codeChars and XDRAtom don't require buffer alignment. r=tcampbell
authorJeff Walden <jwalden@mit.edu>
Tue, 05 Mar 2019 17:37:20 -0800
changeset 524367 abcc9ef3e73da68b0fdbe9674364df36b3c25fd4
parent 524366 b7c332bee855b657db8f0bcbba9dc98465891a1d
child 524368 b9d87882a36584e2f17331b652cbc8681aee17ce
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1533636
milestone67.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 1533636 - Remove all remaining calls to XDRState::codeAlign as no longer necessary now that codeChars and XDRAtom don't require buffer alignment. r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D22655
js/src/vm/JSFunction.cpp
js/src/vm/Xdr.cpp
js/src/vm/Xdr.h
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -589,17 +589,16 @@ XDRResult js::XDRInterpretedFunction(XDR
     // mirror the scope chain.
     MOZ_ASSERT_IF(fun->isSingleton() && !((lazy && lazy->hasBeenCloned()) ||
                                           (script && script->hasBeenCloned())),
                   fun->environment() == nullptr);
   }
 
   // Everything added below can substituted by the non-lazy-script version of
   // this function later.
-  MOZ_TRY(xdr->codeAlign(sizeof(js::XDRAlignment)));
   js::AutoXDRTree funTree(xdr, xdr->getTreeKey(fun));
 
   MOZ_TRY(xdr->codeUint32(&firstword));
 
   if (firstword & HasAtom) {
     MOZ_TRY(XDRAtom(xdr, &atom));
   }
   MOZ_TRY(xdr->codeUint32(&flagsword));
@@ -653,20 +652,16 @@ XDRResult js::XDRInterpretedFunction(XDR
       return xdr->fail(JS::TranscodeResult_Throw);
     }
     objp.set(fun);
   }
 
   // Verify marker at end of function to detect buffer trunction.
   MOZ_TRY(xdr->codeMarker(0x9E35CA1F));
 
-  // Required by AutoXDRTree to copy & paste snipet of sub-trees while keeping
-  // the alignment.
-  MOZ_TRY(xdr->codeAlign(sizeof(js::XDRAlignment)));
-
   return Ok();
 }
 
 template XDRResult js::XDRInterpretedFunction(XDRState<XDR_ENCODE>*,
                                               HandleScope,
                                               HandleScriptSourceObject,
                                               MutableHandleFunction);
 
--- a/js/src/vm/Xdr.cpp
+++ b/js/src/vm/Xdr.cpp
@@ -191,53 +191,42 @@ XDRResult XDRState<mode>::codeScript(Mut
   AutoTraceLog tl(logger, event);
 
 #ifdef DEBUG
   auto sanityCheck = mozilla::MakeScopeExit(
       [&] { MOZ_ASSERT(validateResultCode(cx(), resultCode())); });
 #endif
   auto guard = mozilla::MakeScopeExit([&] { scriptp.set(nullptr); });
 
-  // This should be a no-op when encoding, but when decoding it would eat any
-  // miss-aligned bytes if when encoding the XDR buffer is appended at the end
-  // of an existing buffer, such as in XDRIncrementalEncoder::linearize
-  // function.
-  MOZ_TRY(codeAlign(sizeof(js::XDRAlignment)));
   AutoXDRTree scriptTree(this, getTopLevelTreeKey());
 
   if (mode == XDR_DECODE) {
     scriptp.set(nullptr);
   } else {
     MOZ_ASSERT(!scriptp->enclosingScope());
   }
 
   MOZ_TRY(VersionCheck(this));
   MOZ_TRY(XDRScript(this, nullptr, nullptr, nullptr, scriptp));
-  MOZ_TRY(codeAlign(sizeof(js::XDRAlignment)));
 
   guard.release();
   return Ok();
 }
 
 template class js::XDRState<XDR_ENCODE>;
 template class js::XDRState<XDR_DECODE>;
 
 AutoXDRTree::AutoXDRTree(XDRCoderBase* xdr, AutoXDRTree::Key key)
     : key_(key), parent_(this), xdr_(xdr) {
-  // Expect sub-tree to start with the maximum alignment required.
-  MOZ_ASSERT(xdr->isAligned(sizeof(js::XDRAlignment)));
   if (key_ != AutoXDRTree::noKey) {
     xdr->createOrReplaceSubTree(this);
   }
 }
 
 AutoXDRTree::~AutoXDRTree() {
-  // Expect sub-tree to end with the maximum alignment required.
-  MOZ_ASSERT_IF(xdr_->resultCode() == JS::TranscodeResult_Ok,
-                xdr_->isAligned(sizeof(js::XDRAlignment)));
   if (key_ != AutoXDRTree::noKey) {
     xdr_->endSubTree();
   }
 }
 
 constexpr AutoXDRTree::Key AutoXDRTree::noKey;
 constexpr AutoXDRTree::Key AutoXDRTree::noSubTree;
 constexpr AutoXDRTree::Key AutoXDRTree::topLevel;
@@ -402,28 +391,16 @@ XDRResult XDRIncrementalEncoder::lineari
   if (oom_) {
     ReportOutOfMemory(cx());
     return fail(JS::TranscodeResult_Throw);
   }
 
   // Do not linearize while we are currently adding bytes.
   MOZ_ASSERT(scope_ == nullptr);
 
-  // Ensure the content of the buffer is properly aligned within the buffer.
-  // This alignment difference should be consumed by the codeAlign function
-  // call of codeScript when decoded.
-  size_t alignLen = sizeof(js::XDRAlignment);
-  if (buffer.length() % alignLen) {
-    alignLen = ComputeByteAlignment(buffer.length(), alignLen);
-    if (!buffer.appendN(0, alignLen)) {
-      ReportOutOfMemory(cx());
-      return fail(JS::TranscodeResult_Throw);
-    }
-  }
-
   // Visit the tree parts in a depth first order to linearize the bits.
   // Calculate the total length first so we don't incur repeated copying
   // and zeroing of memory for large trees.
   DepthFirstSliceIterator dfs(cx(), tree_);
 
   size_t totalLength = buffer.length();
   auto sliceCounter = [&](const Slice& slice) -> bool {
     totalLength += slice.sliceLength;
@@ -440,18 +417,16 @@ XDRResult XDRIncrementalEncoder::lineari
     return fail(JS::TranscodeResult_Throw);
   }
 
   auto sliceCopier = [&](const Slice& slice) -> bool {
     // Copy the bytes associated with the current slice to the transcode
     // buffer which would be serialized.
     MOZ_ASSERT(slice.sliceBegin <= slices_.length());
     MOZ_ASSERT(slice.sliceBegin + slice.sliceLength <= slices_.length());
-    MOZ_ASSERT(buffer.length() % sizeof(XDRAlignment) == 0);
-    MOZ_ASSERT(slice.sliceLength % sizeof(XDRAlignment) == 0);
 
     buffer.infallibleAppend(slices_.begin() + slice.sliceBegin,
                             slice.sliceLength);
     return true;
   };
 
   if (!dfs.iterate(sliceCopier)) {
     ReportOutOfMemory(cx());
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -41,34 +41,16 @@ class XDRBufferBase {
 #endif
   {
   }
 
   JSContext* cx() const { return context_; }
 
   size_t cursor() const { return cursor_; }
 
-  // This function records if the cursor got changed by codeAlign or by any
-  // other read/write of data. This is used for AutoXDRTree assertions, as a
-  // way to ensure that the last thing done is properly setting the alignment
-  // with codeAlign function.
-  void setAligned(bool aligned) {
-#ifdef DEBUG
-    aligned_ = aligned;
-#endif
-  }
-
-  bool isAligned() const {
-#ifdef DEBUG
-    return aligned_;
-#else
-    return true;
-#endif
-  }
-
  protected:
   JSContext* const context_;
   size_t cursor_;
 #ifdef DEBUG
   bool aligned_;
 #endif
 };
 
@@ -78,17 +60,16 @@ class XDRBuffer;
 template <>
 class XDRBuffer<XDR_ENCODE> : public XDRBufferBase {
  public:
   XDRBuffer(JSContext* cx, JS::TranscodeBuffer& buffer, size_t cursor = 0)
       : XDRBufferBase(cx, cursor), buffer_(buffer) {}
 
   uint8_t* write(size_t n) {
     MOZ_ASSERT(n != 0);
-    setAligned(false);
     if (!buffer_.growByUninitialized(n)) {
       ReportOutOfMemory(cx());
       return nullptr;
     }
     uint8_t* ptr = &buffer_[cursor_];
     cursor_ += n;
     return ptr;
   }
@@ -113,17 +94,16 @@ class XDRBuffer<XDR_DECODE> : public XDR
   XDRBuffer(JSContext* cx, const JS::TranscodeRange& range)
       : XDRBufferBase(cx), buffer_(range) {}
 
   XDRBuffer(JSContext* cx, JS::TranscodeBuffer& buffer, size_t cursor = 0)
       : XDRBufferBase(cx, cursor), buffer_(buffer.begin(), buffer.length()) {}
 
   const uint8_t* read(size_t n) {
     MOZ_ASSERT(cursor_ < buffer_.length());
-    setAligned(false);
     uint8_t* ptr = &buffer_[cursor_];
     cursor_ += n;
 
     // Don't let buggy code read past our buffer
     if (cursor_ > buffer_.length()) {
       return nullptr;
     }
 
@@ -141,18 +121,16 @@ class XDRBuffer<XDR_DECODE> : public XDR
   }
 
  private:
   const JS::TranscodeRange buffer_;
 };
 
 class XDRCoderBase;
 class XDRIncrementalEncoder;
-using XDRAlignment = char16_t;
-static const uint8_t AlignPadding[sizeof(XDRAlignment)] = {0, 0};
 
 // An AutoXDRTree is used to identify section encoded by an
 // XDRIncrementalEncoder.
 //
 // Its primary goal is to identify functions, such that we can first encode them
 // as LazyScript, and later replaced by them by their corresponding bytecode
 // once delazified.
 //
@@ -208,17 +186,16 @@ class XDRCoderBase {
   virtual AutoXDRTree::Key getTopLevelTreeKey() const {
     return AutoXDRTree::noKey;
   }
   virtual AutoXDRTree::Key getTreeKey(JSFunction* fun) const {
     return AutoXDRTree::noKey;
   }
   virtual void createOrReplaceSubTree(AutoXDRTree* child){};
   virtual void endSubTree(){};
-  virtual bool isAligned(size_t n) = 0;
 
 #ifdef DEBUG
   // Record logical failures of XDR.
   JS::TranscodeResult resultCode() const { return resultCode_; }
   void setResultCode(JS::TranscodeResult code) {
     MOZ_ASSERT(resultCode() == JS::TranscodeResult_Ok);
     resultCode_ = code;
   }
@@ -267,56 +244,16 @@ class XDRState : public XDRCoderBase {
     const uint8_t* ptr = buf.read(length);
     if (!ptr) {
       return fail(JS::TranscodeResult_Failure_BadDecode);
     }
     *pptr = ptr;
     return Ok();
   }
 
-  // Alignment is required when doing memcpy of data which contains element
-  // largers than 1 byte.
-  bool isAligned(size_t n) override {
-    MOZ_ASSERT(mozilla::IsPowerOfTwo(n));
-    size_t mask = n - 1;
-    size_t offset = buf.uptr() & mask;
-    // In debug build, we not only check if the cursor is aligned, but also
-    // if the last cursor manipulation was made by the codeAlign function.
-    return offset == 0 && buf.isAligned();
-  }
-  XDRResult codeAlign(size_t n) {
-    MOZ_ASSERT(mozilla::IsPowerOfTwo(n));
-    size_t mask = n - 1;
-    MOZ_ASSERT_IF(mode == XDR_ENCODE,
-                  (buf.uptr() & mask) == (buf.cursor() & mask));
-    size_t offset = buf.uptr() & mask;
-    if (offset) {
-      size_t padding = n - offset;
-      MOZ_ASSERT(padding < sizeof(AlignPadding));
-      if (mode == XDR_ENCODE) {
-        uint8_t* ptr = buf.write(padding);
-        if (!ptr) {
-          return fail(JS::TranscodeResult_Throw);
-        }
-        memcpy(ptr, AlignPadding, padding);
-      } else {
-        const uint8_t* ptr = buf.read(padding);
-        if (!ptr) {
-          return fail(JS::TranscodeResult_Failure_BadDecode);
-        }
-        if (memcmp(ptr, AlignPadding, padding) != 0) {
-          return fail(JS::TranscodeResult_Failure_BadDecode);
-        }
-      }
-    }
-    buf.setAligned(true);
-    MOZ_ASSERT(isAligned(n));
-    return Ok();
-  }
-
   XDRResult codeUint8(uint8_t* n) {
     if (mode == XDR_ENCODE) {
       uint8_t* ptr = buf.write(sizeof(*n));
       if (!ptr) {
         return fail(JS::TranscodeResult_Throw);
       }
       *ptr = *n;
     } else {