Bug 1620495 - Cleanup LazyScript overflow checks r=mgaudet
authorTed Campbell <tcampbell@mozilla.com>
Mon, 16 Mar 2020 19:24:50 +0000
changeset 519040 4478047b72deb3382c609d235cb77b48cfc5e602
parent 519039 7b12fa322e5d953f63ca3fea3444e7a18b6cb7b1
child 519041 c1233643f6affa98444a3b75186cc42952421344
push id110273
push usertcampbell@mozilla.com
push dateMon, 16 Mar 2020 20:09:27 +0000
treeherderautoland@cc99b4449f14 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1620495
milestone76.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 1620495 - Cleanup LazyScript overflow checks r=mgaudet The old limits were to due bit packing tricks of a very old form of the LazyScript data structure. Replace this with a limit of UINT32_MAX. For simplicity, just fail the parse if we hit this limit since the full parse will also fail. Differential Revision: https://phabricator.services.mozilla.com/D66846
js/src/frontend/Parser.cpp
js/src/frontend/Stencil.h
js/src/vm/JSScript.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1665,26 +1665,16 @@ bool PerHandlerParser<SyntaxParseHandler
   // each scope regardless of if any bindings are actually closed over.
   {
     AtomVector& COB = pc_->closedOverBindingsForLazy();
     while (!COB.empty() && (COB.back() == nullptr)) {
       COB.popBack();
     }
   }
 
-  // There are too many bindings or inner functions to be saved into the
-  // LazyScript. Do a full parse.
-  if (pc_->closedOverBindingsForLazy().length() >=
-          LazyScript::NumClosedOverBindingsLimit ||
-      pc_->innerFunctionIndexesForLazy.length() >=
-          LazyScript::NumInnerFunctionsLimit) {
-    MOZ_ALWAYS_FALSE(abortIfSyntaxParser());
-    return false;
-  }
-
   FunctionBox* funbox = pc_->functionBox();
   funbox->synchronizeArgCount();
 
   LazyScriptCreationData data(cx_);
   if (!data.init(cx_, pc_->closedOverBindingsForLazy(),
                  std::move(pc_->innerFunctionIndexesForLazy),
                  options().forceStrictMode(), pc_->sc()->strict())) {
     return false;
--- a/js/src/frontend/Stencil.h
+++ b/js/src/frontend/Stencil.h
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef frontend_Stencil_h
 #define frontend_Stencil_h
 
 #include "mozilla/Assertions.h"  // MOZ_ASSERT, MOZ_RELEASE_ASSERT
+#include "mozilla/CheckedInt.h"  // CheckedUint32
 #include "mozilla/Maybe.h"       // mozilla::{Maybe, Nothing}
 #include "mozilla/Range.h"       // mozilla::Range
 #include "mozilla/Span.h"        // mozilla::Span
 
 #include <stdint.h>  // char16_t, uint8_t, uint32_t
 #include <stdlib.h>  // size_t
 
 #include "frontend/AbstractScopePtr.h"   // AbstractScopePtr, ScopeIndex
@@ -83,16 +84,25 @@ struct LazyScriptCreationData {
 
   mozilla::Maybe<FieldInitializers> fieldInitializers;
 
   explicit LazyScriptCreationData(JSContext* cx) : innerFunctionIndexes(cx) {}
 
   bool init(JSContext* cx, const frontend::AtomVector& COB,
             Vector<FunctionIndex>&& innerIndexes, bool isForceStrict,
             bool isStrict) {
+    // Check if we will overflow the `ngcthings` field later.
+    mozilla::CheckedUint32 ngcthings =
+        mozilla::CheckedUint32(COB.length()) +
+        mozilla::CheckedUint32(innerIndexes.length());
+    if (!ngcthings.isValid()) {
+      ReportAllocationOverflow(cx);
+      return false;
+    }
+
     forceStrict = isForceStrict;
     strict = isStrict;
     innerFunctionIndexes = std::move(innerIndexes);
 
     if (!closedOverBindings.appendAll(COB)) {
       ReportOutOfMemory(cx);  // closedOverBindings uses SystemAllocPolicy.
       return false;
     }
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -3142,34 +3142,27 @@ class LazyScript : public BaseScript {
   //  | when the enclosing script     o
   //  | is successfully compiled      | when decoded from XDR,
   //  | (setEnclosingScope)           | and enclosing script is not lazy
   //  v                               | (CreateForXDR with enclosingScope)
   // +-----------------+              |
   // | enclosing Scope |<-------------+
   // +-----------------+
 
-  static const uint32_t NumClosedOverBindingsBits = 20;
-  static const uint32_t NumInnerFunctionsBits = 20;
-
   using BaseScript::BaseScript;
 
   // Create a LazyScript without initializing the closedOverBindings and the
   // innerFunctions. To be GC-safe, the caller must initialize both vectors
   // with valid atoms and functions.
   static LazyScript* CreateRaw(JSContext* cx, uint32_t ngcthings,
                                HandleFunction fun,
                                HandleScriptSourceObject sourceObject,
                                const SourceExtent& extent);
 
  public:
-  static const uint32_t NumClosedOverBindingsLimit =
-      1 << NumClosedOverBindingsBits;
-  static const uint32_t NumInnerFunctionsLimit = 1 << NumInnerFunctionsBits;
-
   // Create a LazyScript and initialize closedOverBindings and innerFunctions
   // with the provided vectors.
   static LazyScript* Create(
       JSContext* cx, const frontend::CompilationInfo& compilationInfo,
       HandleFunction fun, HandleScriptSourceObject sourceObject,
       const frontend::AtomVector& closedOverBindings,
       const Vector<frontend::FunctionIndex>& innerFunctionIndexes,
       const SourceExtent& extent);