Bug 1547478 - XDR BinAST metadata using deduplicated BinAST data so that pointers in the metadata will correctly point into the deduplicated data, not into user-provided data that hasn't been deduplicated yet. r=tcampbell
authorJeff Walden <jwalden@mit.edu>
Fri, 03 May 2019 21:49:52 +0000
changeset 531445 99ad8ab7c1fd93938f5d94f65a1c8e227d80e5b5
parent 531444 ce18611e1dd4f9891a4d410bfedc11dc73c4f5e6
child 531446 6abefa3e063b1ac132fa2e912055d0f184621eef
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1547478
milestone68.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 1547478 - XDR BinAST metadata using deduplicated BinAST data so that pointers in the metadata will correctly point into the deduplicated data, not into user-provided data that hasn't been deduplicated yet. r=tcampbell Depends on D29266 Differential Revision: https://phabricator.services.mozilla.com/D29547
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -2181,35 +2181,16 @@ MOZ_MUST_USE bool ScriptSource::setBinAS
     ReportOutOfMemory(cx);
     return false;
   }
 
   data = SourceType(BinAST(std::move(*deduped)));
   return true;
 }
 
-MOZ_MUST_USE bool ScriptSource::initializeBinAST(
-    JSContext* cx, UniqueChars&& buf, size_t len,
-    MutableHandle<UniquePtr<frontend::BinASTSourceMetadata>> metadata) {
-  MOZ_ASSERT(data.is<Missing>(),
-             "should only be initializing a fresh ScriptSource");
-  MOZ_ASSERT(binASTMetadata_ == nullptr, "shouldn't have BinAST metadata yet");
-
-  auto& cache = cx->zone()->runtimeFromAnyThread()->sharedImmutableStrings();
-  auto deduped = cache.getOrCreate(std::move(buf), len);
-  if (!deduped) {
-    ReportOutOfMemory(cx);
-    return false;
-  }
-
-  data = SourceType(BinAST(std::move(*deduped)));
-  binASTMetadata_ = std::move(metadata.get());
-  return true;
-}
-
 const uint8_t* ScriptSource::binASTSource() {
   MOZ_ASSERT(hasBinASTSource());
   return reinterpret_cast<const uint8_t*>(data.as<BinAST>().string.chars());
 }
 
 #endif /* JS_BUILD_BINAST */
 
 bool ScriptSource::tryCompressOffThread(JSContext* cx) {
@@ -2759,24 +2740,32 @@ XDRResult ScriptSource::codeBinASTData(X
   // XDR the length of the BinAST data.
   uint32_t binASTLength;
   if (mode == XDR_ENCODE) {
     binASTLength = ss->data.as<BinAST>().string.length();
   }
   MOZ_TRY(xdr->codeUint32(&binASTLength));
 
   // XDR the BinAST data.
-  UniquePtr<char[], JS::FreePolicy> bytes;
+  Maybe<SharedImmutableString> binASTData;
   if (mode == XDR_DECODE) {
-    bytes =
-        xdr->cx()->template make_pod_array<char>(Max<size_t>(binASTLength, 1));
+    auto bytes = xdr->cx()->template make_pod_array<char>(Max<size_t>(
+        binASTLength, 1));
     if (!bytes) {
       return xdr->fail(JS::TranscodeResult_Throw);
     }
     MOZ_TRY(xdr->codeBytes(bytes.get(), binASTLength));
+
+    auto& cache =
+        xdr->cx()->zone()->runtimeFromAnyThread()->sharedImmutableStrings();
+    binASTData = cache.getOrCreate(std::move(bytes), binASTLength);
+    if (!binASTData) {
+      ReportOutOfMemory(xdr->cx());
+      return xdr->fail(JS::TranscodeResult_Throw);
+    }
   } else {
     void* bytes = ss->binASTData();
     MOZ_TRY(xdr->codeBytes(bytes, binASTLength));
   }
 
   // XDR any BinAST metadata.
   uint8_t hasMetadata;
   if (mode == XDR_ENCODE) {
@@ -2825,17 +2814,17 @@ XDRResult ScriptSource::codeBinASTData(X
     for (uint32_t i = 0; i < numBinASTKinds; i++) {
       MOZ_TRY(xdr->codeEnum32(&binASTKindBase[i]));
     }
 
     RootedAtom atom(xdr->cx());
     JSAtom** atomsBase = binASTMetadata->atomsBase();
     auto slices = binASTMetadata->sliceBase();
     const char* sourceBase =
-        mode == XDR_ENCODE ? ss->data.as<BinAST>().string.chars() : bytes.get();
+        (mode == XDR_ENCODE ? ss->data.as<BinAST>().string : *binASTData).chars();
 
     for (uint32_t i = 0; i < numStrings; i++) {
       uint8_t isNull;
       if (mode == XDR_ENCODE) {
         atom = binASTMetadata->getAtom(i);
         isNull = !atom;
       }
       MOZ_TRY(xdr->codeUint8(&isNull));
@@ -2862,24 +2851,31 @@ XDRResult ScriptSource::codeBinASTData(X
       if (mode == XDR_DECODE) {
         new (&slices[i]) frontend::BinASTSourceMetadata::CharSlice(
             sourceBase + sliceOffset, sliceLen);
       }
     }
   }
 
   if (mode == XDR_DECODE) {
+    MOZ_ASSERT(binASTData.isSome());
     MOZ_ASSERT(freshMetadata != nullptr);
-    if (!ss->initializeBinAST(xdr->cx(), std::move(bytes), binASTLength,
-                              &freshMetadata)) {
-      return xdr->fail(JS::TranscodeResult_Throw);
-    }
-  }
-
+
+    MOZ_ASSERT(ss->data.is<Missing>(),
+               "should only be initializing a fresh ScriptSource");
+    MOZ_ASSERT(ss->binASTMetadata_ == nullptr,
+               "shouldn't have BinAST metadata yet");
+
+    ss->data = SourceType(BinAST(std::move(*binASTData)));
+    ss->binASTMetadata_ = std::move(freshMetadata.get());
+  }
+
+  MOZ_ASSERT(binASTData.isNothing());
   MOZ_ASSERT(freshMetadata == nullptr);
+  MOZ_ASSERT(ss->data.is<BinAST>());
 
   return Ok();
 #endif  // !defined(JS_BUILD_BINAST)
 }
 
 template <typename Unit, XDRMode mode>
 /* static */
 void ScriptSource::codeRetrievableData(ScriptSource* ss) {
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -995,24 +995,16 @@ class ScriptSource {
   /*
    * Do not take ownership of the given `buf`. Store the canonical, shared
    * and de-duplicated version. If there is no extant shared version of
    * `buf`, make a copy.
    */
   MOZ_MUST_USE bool setBinASTSourceCopy(JSContext* cx, const uint8_t* buf,
                                         size_t len);
 
-  /*
-   * Initialize this as containing BinAST data for |buf|/|len|, using a shared,
-   * deduplicated version of |buf| if necessary.
-   */
-  MOZ_MUST_USE bool initializeBinAST(
-      JSContext* cx, UniqueChars&& buf, size_t len,
-      JS::MutableHandle<UniquePtr<frontend::BinASTSourceMetadata>> metadata);
-
   const uint8_t* binASTSource();
 
 #endif /* JS_BUILD_BINAST */
 
  private:
   void performTaskWork(SourceCompressionTask* task);
 
   struct ConvertToCompressedSourceFromTask {