Back out bug 1181908 (again) for suspicion of causing bug 1191465 and other GC crashes.
authorBill McCloskey <billm@mozilla.com>
Sun, 09 Aug 2015 08:42:40 -0700
changeset 257010 fd69d51a4068e9870d2772d7566fa12f8dbaa3b7
parent 257009 7e586bac29fe2d4dac4378be4f5088ed71641cfa
child 257011 a431fb8b9a403787f2aa140114a60d965dfcc483
child 257036 0d6871d66dc07cda204c3814a488f6dafe019507
push id14559
push userphilringnalda@gmail.com
push dateSun, 09 Aug 2015 23:41:14 +0000
treeherderfx-team@0e269a1f1beb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1181908, 1191465
milestone42.0a1
Back out bug 1181908 (again) for suspicion of causing bug 1191465 and other GC crashes.
js/src/NamespaceImports.h
js/src/frontend/BytecodeEmitter.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jspubtd.h
--- a/js/src/NamespaceImports.h
+++ b/js/src/NamespaceImports.h
@@ -94,17 +94,16 @@ using JS::CallArgs;
 using JS::CallNonGenericMethod;
 using JS::CallReceiver;
 using JS::CompileOptions;
 using JS::IsAcceptableThis;
 using JS::NativeImpl;
 using JS::OwningCompileOptions;
 using JS::ReadOnlyCompileOptions;
 using JS::SourceBufferHolder;
-using JS::TransitiveCompileOptions;
 
 using JS::Rooted;
 using JS::RootedFunction;
 using JS::RootedId;
 using JS::RootedObject;
 using JS::RootedScript;
 using JS::RootedString;
 using JS::RootedSymbol;
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -5762,23 +5762,23 @@ BytecodeEmitter::emitFunction(ParseNode*
                 fun->lazyScript()->setTreatAsRunOnce();
         } else {
             SharedContext* outersc = sc;
 
             if (outersc->isFunctionBox() && outersc->asFunctionBox()->mightAliasLocals())
                 funbox->setMightAliasLocals();      // inherit mightAliasLocals from parent
             MOZ_ASSERT_IF(outersc->strict(), funbox->strictScript);
 
-            // Inherit most things (principals, version, etc) from the
-            // parent.  Use default values for the rest.
+            // Inherit most things (principals, version, etc) from the parent.
             Rooted<JSScript*> parent(cx, script);
-            MOZ_ASSERT(parent->getVersion() == parser->options().version);
-            MOZ_ASSERT(parent->mutedErrors() == parser->options().mutedErrors());
-            const TransitiveCompileOptions& transitiveOptions = parser->options();
-            CompileOptions options(cx, transitiveOptions);
+            CompileOptions options(cx, parser->options());
+            options.setMutedErrors(parent->mutedErrors())
+                   .setNoScriptRval(false)
+                   .setForEval(false)
+                   .setVersion(parent->getVersion());
 
             Rooted<JSObject*> enclosingScope(cx, enclosingStaticScope());
             Rooted<JSObject*> sourceObject(cx, script->sourceObject());
             Rooted<JSScript*> script(cx, JSScript::Create(cx, enclosingScope, false, options,
                                                           parent->staticLevel() + 1,
                                                           sourceObject,
                                                           funbox->bufStart, funbox->bufEnd));
             if (!script)
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3808,47 +3808,41 @@ AutoFile::open(JSContext* cx, const char
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_OPEN,
                                  filename, "No such file or directory");
             return false;
         }
     }
     return true;
 }
 
+JSObject * const JS::ReadOnlyCompileOptions::nullObjectPtr = nullptr;
+
 void
-JS::TransitiveCompileOptions::copyPODTransitiveOptions(const TransitiveCompileOptions& rhs)
-{
-    mutedErrors_ = rhs.mutedErrors_;
+JS::ReadOnlyCompileOptions::copyPODOptions(const ReadOnlyCompileOptions& rhs)
+{
     version = rhs.version;
     versionSet = rhs.versionSet;
     utf8 = rhs.utf8;
+    lineno = rhs.lineno;
+    column = rhs.column;
+    forEval = rhs.forEval;
+    noScriptRval = rhs.noScriptRval;
     selfHostingMode = rhs.selfHostingMode;
     canLazilyParse = rhs.canLazilyParse;
     strictOption = rhs.strictOption;
     extraWarningsOption = rhs.extraWarningsOption;
     werrorOption = rhs.werrorOption;
     asmJSOption = rhs.asmJSOption;
     forceAsync = rhs.forceAsync;
     installedFile = rhs.installedFile;
     sourceIsLazy = rhs.sourceIsLazy;
     introductionType = rhs.introductionType;
     introductionLineno = rhs.introductionLineno;
     introductionOffset = rhs.introductionOffset;
     hasIntroductionInfo = rhs.hasIntroductionInfo;
-};
-
-void
-JS::ReadOnlyCompileOptions::copyPODOptions(const ReadOnlyCompileOptions& rhs)
-{
-    copyPODTransitiveOptions(rhs);
-    lineno = rhs.lineno;
-    column = rhs.column;
-    isRunOnce = rhs.isRunOnce;
-    forEval = rhs.forEval;
-    noScriptRval = rhs.noScriptRval;
 }
 
 JS::OwningCompileOptions::OwningCompileOptions(JSContext* cx)
     : ReadOnlyCompileOptions(),
       runtime(GetRuntime(cx)),
       elementRoot(cx),
       elementAttributeNameRoot(cx),
       introductionScriptRoot(cx)
@@ -3863,16 +3857,17 @@ JS::OwningCompileOptions::~OwningCompile
     js_free(const_cast<char*>(introducerFilename_));
 }
 
 bool
 JS::OwningCompileOptions::copy(JSContext* cx, const ReadOnlyCompileOptions& rhs)
 {
     copyPODOptions(rhs);
 
+    setMutedErrors(rhs.mutedErrors());
     setElement(rhs.element());
     setElementAttributeName(rhs.elementAttributeName());
     setIntroductionScript(rhs.introductionScript());
 
     return setFileAndLine(cx, rhs.filename(), rhs.lineno) &&
            setSourceMapURL(cx, rhs.sourceMapURL()) &&
            setIntroducerFilename(cx, rhs.introducerFilename());
 }
@@ -4270,17 +4265,18 @@ CompileFunction(JSContext* cx, const Rea
     if (!fun)
         return false;
 
     // Make sure the static scope chain matches up when we have a
     // non-syntactic scope.
     MOZ_ASSERT_IF(!enclosingDynamicScope->is<GlobalObject>(),
                   HasNonSyntacticStaticScopeChain(enclosingStaticScope));
 
-    if (!frontend::CompileFunctionBody(cx, fun, optionsArg, formals, srcBuf, enclosingStaticScope))
+    CompileOptions options(cx, optionsArg);
+    if (!frontend::CompileFunctionBody(cx, fun, options, formals, srcBuf, enclosingStaticScope))
         return false;
 
     return true;
 }
 
 JS_PUBLIC_API(bool)
 JS::CompileFunction(JSContext* cx, AutoObjectVector& scopeChain,
                     const ReadOnlyCompileOptions& options,
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3379,48 +3379,42 @@ namespace JS {
  * compilation options where a worker thread can find them, and then return
  * immediately. The worker thread will come along at some later point, and use
  * the options.
  *
  * The compiler itself just needs to be able to access a collection of options;
  * it doesn't care who owns them, or what's keeping them alive. It does its own
  * addrefs/copies/tracing/etc.
  *
- * Furthermore, in some cases compile options are propagated from one entity to
- * another (e.g. from a scriipt to a function defined in that script).  This
- * involves copying over some, but not all, of the options.
- *
- * So, we have a class hierarchy that reflects these four use cases:
+ * So, we have a class hierarchy that reflects these three use cases:
  *
- * - TransitiveCompileOptions is the common base class, representing options
- *   that should get propagated from a script to functions defined in that
- *   script.  This is never instantiated directly.
- *
- * - ReadOnlyCompileOptions is the only subclass of TransitiveCompileOptions,
- *   representing a full set of compile options.  It can be used by code that
- *   simply needs to access options set elsewhere, like the compiler.  This,
- *   again, is never instantiated directly.
+ * - ReadOnlyCompileOptions is the common base class. It can be used by code
+ *   that simply needs to access options set elsewhere, like the compiler.
  *
  * - The usual CompileOptions class must be stack-allocated, and holds
  *   non-owning references to the filename, element, and so on. It's derived
  *   from ReadOnlyCompileOptions, so the compiler can use it.
  *
  * - OwningCompileOptions roots / copies / reference counts of all its values,
  *   and unroots / frees / releases them when it is destructed. It too is
  *   derived from ReadOnlyCompileOptions, so the compiler accepts it.
  */
 
 /*
  * The common base class for the CompileOptions hierarchy.
  *
- * Use this in code that needs to propagate compile options from one compilation
- * unit to another.
+ * Use this in code that only needs to access compilation options created
+ * elsewhere, like the compiler. Don't instantiate this class (the constructor
+ * is protected anyway); instead, create instances only of the derived classes:
+ * CompileOptions and OwningCompileOptions.
  */
-class JS_FRIEND_API(TransitiveCompileOptions)
+class JS_FRIEND_API(ReadOnlyCompileOptions)
 {
+    friend class CompileOptions;
+
   protected:
     // The Web Platform allows scripts to be loaded from arbitrary cross-origin
     // sources. This allows an attack by which a malicious website loads a
     // sensitive file (say, a bank statement) cross-origin (using the user's
     // cookies), and sniffs the generated syntax errors (via a window.onerror
     // handler) for juicy morsels of its contents.
     //
     // To counter this attack, HTML5 specifies that script errors should be
@@ -3432,24 +3426,29 @@ class JS_FRIEND_API(TransitiveCompileOpt
     const char* filename_;
     const char* introducerFilename_;
     const char16_t* sourceMapURL_;
 
     // This constructor leaves 'version' set to JSVERSION_UNKNOWN. The structure
     // is unusable until that's set to something more specific; the derived
     // classes' constructors take care of that, in ways appropriate to their
     // purpose.
-    TransitiveCompileOptions()
+    ReadOnlyCompileOptions()
       : mutedErrors_(false),
         filename_(nullptr),
         introducerFilename_(nullptr),
         sourceMapURL_(nullptr),
         version(JSVERSION_UNKNOWN),
         versionSet(false),
         utf8(false),
+        lineno(1),
+        column(0),
+        isRunOnce(false),
+        forEval(false),
+        noScriptRval(false),
         selfHostingMode(false),
         canLazilyParse(true),
         strictOption(false),
         extraWarningsOption(false),
         werrorOption(false),
         asmJSOption(false),
         forceAsync(false),
         installedFile(false),
@@ -3457,33 +3456,39 @@ class JS_FRIEND_API(TransitiveCompileOpt
         introductionType(nullptr),
         introductionLineno(0),
         introductionOffset(0),
         hasIntroductionInfo(false)
     { }
 
     // Set all POD options (those not requiring reference counts, copies,
     // rooting, or other hand-holding) to their values in |rhs|.
-    void copyPODTransitiveOptions(const TransitiveCompileOptions& rhs);
+    void copyPODOptions(const ReadOnlyCompileOptions& rhs);
 
   public:
     // Read-only accessors for non-POD options. The proper way to set these
     // depends on the derived type.
     bool mutedErrors() const { return mutedErrors_; }
     const char* filename() const { return filename_; }
     const char* introducerFilename() const { return introducerFilename_; }
     const char16_t* sourceMapURL() const { return sourceMapURL_; }
     virtual JSObject* element() const = 0;
     virtual JSString* elementAttributeName() const = 0;
     virtual JSScript* introductionScript() const = 0;
 
     // POD options.
     JSVersion version;
     bool versionSet;
     bool utf8;
+    unsigned lineno;
+    unsigned column;
+    // isRunOnce only applies to non-function scripts.
+    bool isRunOnce;
+    bool forEval;
+    bool noScriptRval;
     bool selfHostingMode;
     bool canLazilyParse;
     bool strictOption;
     bool extraWarningsOption;
     bool werrorOption;
     bool asmJSOption;
     bool forceAsync;
     bool installedFile;  // 'true' iff pre-compiling js file in packaged app
@@ -3492,65 +3497,17 @@ class JS_FRIEND_API(TransitiveCompileOpt
     // |introductionType| is a statically allocated C string:
     // one of "eval", "Function", or "GeneratorFunction".
     const char* introductionType;
     unsigned introductionLineno;
     uint32_t introductionOffset;
     bool hasIntroductionInfo;
 
   private:
-    void operator=(const TransitiveCompileOptions&) = delete;
-};
-
-/*
- * The class representing a full set of compile options.
- *
- * Use this in code that only needs to access compilation options created
- * elsewhere, like the compiler. Don't instantiate this class (the constructor
- * is protected anyway); instead, create instances only of the derived classes:
- * CompileOptions and OwningCompileOptions.
- */
-class JS_FRIEND_API(ReadOnlyCompileOptions) : public TransitiveCompileOptions
-{
-    friend class CompileOptions;
-
-  protected:
-    ReadOnlyCompileOptions()
-      : TransitiveCompileOptions(),
-        lineno(1),
-        column(0),
-        isRunOnce(false),
-        forEval(false),
-        noScriptRval(false)
-    { }
-
-    // Set all POD options (those not requiring reference counts, copies,
-    // rooting, or other hand-holding) to their values in |rhs|.
-    void copyPODOptions(const ReadOnlyCompileOptions& rhs);
-
-  public:
-    // Read-only accessors for non-POD options. The proper way to set these
-    // depends on the derived type.
-    bool mutedErrors() const { return mutedErrors_; }
-    const char* filename() const { return filename_; }
-    const char* introducerFilename() const { return introducerFilename_; }
-    const char16_t* sourceMapURL() const { return sourceMapURL_; }
-    virtual JSObject* element() const = 0;
-    virtual JSString* elementAttributeName() const = 0;
-    virtual JSScript* introductionScript() const = 0;
-
-    // POD options.
-    unsigned lineno;
-    unsigned column;
-    // isRunOnce only applies to non-function scripts.
-    bool isRunOnce;
-    bool forEval;
-    bool noScriptRval;
-
-  private:
+    static JSObject * const nullObjectPtr;
     void operator=(const ReadOnlyCompileOptions&) = delete;
 };
 
 /*
  * Compilation options, with dynamic lifetime. An instance of this type
  * makes a copy of / holds / roots all dynamically allocated resources
  * (principals; elements; strings) that it refers to. Its destructor frees
  * / drops / unroots them. This is heavier than CompileOptions, below, but
@@ -3655,32 +3612,18 @@ class MOZ_STACK_CLASS JS_FRIEND_API(Comp
   public:
     explicit CompileOptions(JSContext* cx, JSVersion version = JSVERSION_UNKNOWN);
     CompileOptions(js::ContextFriendFields* cx, const ReadOnlyCompileOptions& rhs)
       : ReadOnlyCompileOptions(), elementRoot(cx), elementAttributeNameRoot(cx),
         introductionScriptRoot(cx)
     {
         copyPODOptions(rhs);
 
+        mutedErrors_ = rhs.mutedErrors_;
         filename_ = rhs.filename();
-        introducerFilename_ = rhs.introducerFilename();
-        sourceMapURL_ = rhs.sourceMapURL();
-        elementRoot = rhs.element();
-        elementAttributeNameRoot = rhs.elementAttributeName();
-        introductionScriptRoot = rhs.introductionScript();
-    }
-
-    CompileOptions(js::ContextFriendFields* cx, const TransitiveCompileOptions& rhs)
-      : ReadOnlyCompileOptions(), elementRoot(cx), elementAttributeNameRoot(cx),
-        introductionScriptRoot(cx)
-    {
-        copyPODTransitiveOptions(rhs);
-
-        filename_ = rhs.filename();
-        introducerFilename_ = rhs.introducerFilename();
         sourceMapURL_ = rhs.sourceMapURL();
         elementRoot = rhs.element();
         elementAttributeNameRoot = rhs.elementAttributeName();
         introductionScriptRoot = rhs.introductionScript();
     }
 
     JSObject* element() const override { return elementRoot; }
     JSString* elementAttributeName() const override { return elementAttributeNameRoot; }
--- a/js/src/jspubtd.h
+++ b/js/src/jspubtd.h
@@ -32,17 +32,16 @@ typedef AutoVectorRooter<jsid> AutoIdVec
 class CallArgs;
 
 template <typename T>
 class Rooted;
 
 class JS_FRIEND_API(CompileOptions);
 class JS_FRIEND_API(ReadOnlyCompileOptions);
 class JS_FRIEND_API(OwningCompileOptions);
-class JS_FRIEND_API(TransitiveCompileOptions);
 class JS_PUBLIC_API(CompartmentOptions);
 
 class Value;
 struct Zone;
 
 } /* namespace JS */
 
 namespace js {