From 05d0806c8defad56dcf795c8f5716a3fe44fe87b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 3 Aug 2015 11:51:57 -0400 Subject: [PATCH] Bug 1181908 part 2. The CompileOptions constructor should properly copy the introducerFilename and isRunOnce state. r=luke --- js/src/NamespaceImports.h | 1 + js/src/frontend/BytecodeEmitter.cpp | 12 ++-- js/src/jsapi.cpp | 24 ++++--- js/src/jsapi.h | 107 +++++++++++++++++++++------- js/src/jspubtd.h | 1 + 5 files changed, 104 insertions(+), 41 deletions(-) diff --git a/js/src/NamespaceImports.h b/js/src/NamespaceImports.h index c6a962340927..adde176d0b74 100644 --- a/js/src/NamespaceImports.h +++ b/js/src/NamespaceImports.h @@ -100,6 +100,7 @@ using JS::NativeImpl; using JS::OwningCompileOptions; using JS::ReadOnlyCompileOptions; using JS::SourceBufferHolder; +using JS::TransitiveCompileOptions; using JS::Rooted; using JS::RootedFunction; diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 7ead1ef81e28..d79ee0257e3a 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -5809,13 +5809,13 @@ BytecodeEmitter::emitFunction(ParseNode* pn, bool needsProto) 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 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 enclosingScope(cx, innermostStaticScope()); Rooted sourceObject(cx, script->sourceObject()); diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 9745b1a4c14e..d371e234b785 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3797,18 +3797,13 @@ AutoFile::open(JSContext* cx, const char* filename) 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; @@ -3822,6 +3817,17 @@ JS::ReadOnlyCompileOptions::copyPODOptions(const ReadOnlyCompileOptions& rhs) 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) @@ -3846,7 +3852,6 @@ JS::OwningCompileOptions::copy(JSContext* cx, const ReadOnlyCompileOptions& rhs) { copyPODOptions(rhs); - setMutedErrors(rhs.mutedErrors()); setElement(rhs.element()); setElementAttributeName(rhs.elementAttributeName()); setIntroductionScript(rhs.introductionScript()); @@ -4254,8 +4259,7 @@ CompileFunction(JSContext* cx, const ReadOnlyCompileOptions& optionsArg, MOZ_ASSERT_IF(!enclosingDynamicScope->is(), 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; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 6274d40548c6..0aa794c4fc04 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -3273,10 +3273,20 @@ namespace JS { * 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. * - * - ReadOnlyCompileOptions is the common base class. It can be used by code - * that simply needs to access options set elsewhere, like the compiler. + * So, we have a class hierarchy that reflects these four 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. * * - The usual CompileOptions class must be stack-allocated, and holds * non-owning references to the filename, element, and so on. It's derived @@ -3290,15 +3300,11 @@ namespace JS { /* * 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. + * Use this in code that needs to propagate compile options from one compilation + * unit to another. */ -class JS_FRIEND_API(ReadOnlyCompileOptions) +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 @@ -3320,7 +3326,7 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) // 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), @@ -3328,11 +3334,6 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) version(JSVERSION_UNKNOWN), versionSet(false), utf8(false), - lineno(1), - column(0), - isRunOnce(false), - forEval(false), - noScriptRval(false), selfHostingMode(false), canLazilyParse(true), strictOption(false), @@ -3350,7 +3351,7 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) // 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); + void copyPODTransitiveOptions(const TransitiveCompileOptions& rhs); public: // Read-only accessors for non-POD options. The proper way to set these @@ -3367,12 +3368,6 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) 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; @@ -3391,7 +3386,55 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) bool hasIntroductionInfo; private: - static JSObject * const nullObjectPtr; + 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: void operator=(const ReadOnlyCompileOptions&) = delete; }; @@ -3506,8 +3549,22 @@ class MOZ_STACK_CLASS JS_FRIEND_API(CompileOptions) : public ReadOnlyCompileOpti { 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(); diff --git a/js/src/jspubtd.h b/js/src/jspubtd.h index 0cc334d73693..ae5bb5021a84 100644 --- a/js/src/jspubtd.h +++ b/js/src/jspubtd.h @@ -37,6 +37,7 @@ 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;