Bug 1181908. The CompileOptions constructor should properly copy the introducerFilename and isRunOnce state. r=luke
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 06 Aug 2015 15:34:06 -0700
changeset 288367 fc5272ac152e8d0fea38ad2e4bd38e6d8aafb744
parent 288366 d1c8f4aeedef5d582b88fa84a1a4934fdf89b808
child 288368 d52716b6cdf0b72efe28a3f2f657e1cd50ba1fbe
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1181908
milestone42.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 1181908. The CompileOptions constructor should properly copy the introducerFilename and isRunOnce state. r=luke
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,16 +94,17 @@ 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.
+            // Inherit most things (principals, version, etc) from the
+            // parent.  Use default values for the rest.
             Rooted<JSScript*> parent(cx, script);
-            CompileOptions options(cx, parser->options());
-            options.setMutedErrors(parent->mutedErrors())
-                   .setNoScriptRval(false)
-                   .setForEval(false)
-                   .setVersion(parent->getVersion());
+            MOZ_ASSERT(parent->getVersion() == parser->options().version);
+            MOZ_ASSERT(parent->mutedErrors() == parser->options().mutedErrors());
+            const TransitiveCompileOptions& transitiveOptions = parser->options();
+            CompileOptions options(cx, transitiveOptions);
 
             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,41 +3808,47 @@ 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::ReadOnlyCompileOptions::copyPODOptions(const ReadOnlyCompileOptions& rhs)
-{
+JS::TransitiveCompileOptions::copyPODTransitiveOptions(const TransitiveCompileOptions& rhs)
+{
+    mutedErrors_ = rhs.mutedErrors_;
     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)
@@ -3857,17 +3863,16 @@ 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());
 }
@@ -4265,18 +4270,17 @@ 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));
 
-    CompileOptions options(cx, optionsArg);
-    if (!frontend::CompileFunctionBody(cx, fun, options, formals, srcBuf, enclosingStaticScope))
+    if (!frontend::CompileFunctionBody(cx, fun, optionsArg, 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,42 +3379,48 @@ 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.
  *
- * So, we have a class hierarchy that reflects these three use cases:
+ * 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:
  *
- * - ReadOnlyCompileOptions is the common base class. It can be used by code
- *   that simply needs to access options set elsewhere, like the compiler.
+ * - 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.
  *
  * - 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 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)
+ * Use this in code that needs to propagate compile options from one compilation
+ * unit to another.
+ */
+class JS_FRIEND_API(TransitiveCompileOptions)
 {
-    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
@@ -3426,29 +3432,24 @@ class JS_FRIEND_API(ReadOnlyCompileOptio
     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.
-    ReadOnlyCompileOptions()
+    TransitiveCompileOptions()
       : 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),
@@ -3456,58 +3457,100 @@ class JS_FRIEND_API(ReadOnlyCompileOptio
         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);
+
+  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;
+    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
+    bool sourceIsLazy;
+
+    // |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.
-    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
-    bool sourceIsLazy;
-
-    // |introductionType| is a statically allocated C string:
-    // one of "eval", "Function", or "GeneratorFunction".
-    const char* introductionType;
-    unsigned introductionLineno;
-    uint32_t introductionOffset;
-    bool hasIntroductionInfo;
 
   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
@@ -3612,18 +3655,32 @@ 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,16 +32,17 @@ 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 {