Bug 1364648 - Fix OOB column handling for default class constructors' toString offsets. (r=jimb)
authorShu-yu Guo <shu@rfrn.org>
Thu, 18 May 2017 18:17:23 -0700
changeset 359267 20fd2a3c8039f37ae7ec284316a08e1d670bbd45
parent 359266 d7121e5ad8065a5ef546b349d90606cbf0e5baab
child 359268 9aa66595bf51a1cec03f61b88bfecac09742157b
push id43028
push userryanvm@gmail.com
push dateFri, 19 May 2017 16:27:11 +0000
treeherderautoland@a4e84d8efb19 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1364648
milestone55.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 1364648 - Fix OOB column handling for default class constructors' toString offsets. (r=jimb) CompileOptions's column field has been conflating two meanings: the starting column of the ScriptSource, and the starting column of the current thing being compiled. In the shell's evaluate() function, when the "column" option is passed, it's fine to conflate the two meanings above. When delazifying functions, it is incorrect to conflate the two meanings. This is observable when generating SRC_CLASS_SPAN, which is a srcnote used to save the toString offsets for a default class constructor. (Since default class constructors aren't syntactically present, there's no JSFunction made ahead of time. And since class constructors must be toString'd as the class source instead of the function source, we save the offsets in a srcnote to use when we actually create the constructor at runtime.) When we save these offsets, these are offsets into the ScriptSource buffer, and must be de-offset. But it's incorrect to subtract the starting column of the lazy function, which is itself offset from the starting column of the underlying ScriptSource.
js/src/frontend/BytecodeCompiler.cpp
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/SharedContext.h
js/src/jit-test/tests/parser/bug-1364648.js
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsscript.cpp
js/src/jsscript.h
js/src/shell/js.cpp
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -640,20 +640,21 @@ frontend::CompileModule(JSContext* cx, c
     return module;
 }
 
 bool
 frontend::CompileLazyFunction(JSContext* cx, Handle<LazyScript*> lazy, const char16_t* chars, size_t length)
 {
     MOZ_ASSERT(cx->compartment() == lazy->functionNonDelazifying()->compartment());
 
+    uint32_t sourceStartColumn = lazy->scriptSource()->startColumn();
     CompileOptions options(cx, lazy->version());
     options.setMutedErrors(lazy->mutedErrors())
            .setFileAndLine(lazy->filename(), lazy->lineno())
-           .setColumn(lazy->column())
+           .setColumn(lazy->column(), sourceStartColumn)
            .setNoScriptRval(false)
            .setSelfHostingMode(false);
 
     // Update statistics to find out if we are delazifying just after having
     // lazified. Note that we are interested in the delta between end of
     // syntax parsing and start of full parsing, so we do this now rather than
     // after parsing below.
     if (!lazy->scriptSource()->parseEnded().IsNull()) {
@@ -676,17 +677,17 @@ frontend::CompileLazyFunction(JSContext*
     Parser<FullParseHandler, char16_t> parser(cx, cx->tempLifoAlloc(), options, chars, length,
                                               /* foldConstants = */ true, usedNames, nullptr,
                                               lazy);
     if (!parser.checkOptions())
         return false;
 
     Rooted<JSFunction*> fun(cx, lazy->functionNonDelazifying());
     MOZ_ASSERT(!lazy->isLegacyGenerator());
-    ParseNode* pn = parser.standaloneLazyFunction(fun, lazy->toStringStart() + lazy->column(),
+    ParseNode* pn = parser.standaloneLazyFunction(fun, lazy->toStringStart() + sourceStartColumn,
                                                   lazy->strict(), lazy->generatorKind(), lazy->asyncKind());
     if (!pn)
         return false;
 
     RootedScriptSource sourceObject(cx, lazy->sourceObject());
     MOZ_ASSERT(sourceObject);
 
     Rooted<JSScript*> script(cx, JSScript::Create(cx, options, sourceObject,
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -10682,17 +10682,25 @@ BytecodeEmitter::emitClass(ParseNode* pn
             if (!emit2(JSOP_INITHOMEOBJECT, 0))
                 return false;
         }
     } else {
         // In the case of default class constructors, emit the start and end
         // offsets in the source buffer as source notes so that when we
         // actually make the constructor during execution, we can give it the
         // correct toString output.
-        if (!newSrcNote3(SRC_CLASS_SPAN, ptrdiff_t(pn->pn_pos.begin), ptrdiff_t(pn->pn_pos.end)))
+        //
+        // Token positions are already offset from the start column. Since
+        // toString offsets are absolute offsets into the ScriptSource,
+        // de-offset from the starting column.
+        ptrdiff_t classStart = ptrdiff_t(pn->pn_pos.begin) -
+                               tokenStream().options().sourceStartColumn;
+        ptrdiff_t classEnd = ptrdiff_t(pn->pn_pos.end) -
+                             tokenStream().options().sourceStartColumn;
+        if (!newSrcNote3(SRC_CLASS_SPAN, classStart, classEnd))
             return false;
 
         JSAtom *name = names ? names->innerBinding()->pn_atom : cx->names().empty;
         if (heritageExpression) {
             if (!emitAtomOp(name, JSOP_DERIVEDCONSTRUCTOR))
                 return false;
         } else {
             if (!emitAtomOp(name, JSOP_CLASSCONSTRUCTOR))
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -561,32 +561,36 @@ class FunctionBox : public ObjectBox, pu
     }
 
     void setStart(const TokenStream& tokenStream) {
         // Token positions are already offset from the start column in
         // CompileOptions. bufStart and toStringStart, however, refer to
         // absolute positions within the ScriptSource buffer, and need to
         // de-offset from the starting column.
         uint32_t offset = tokenStream.currentToken().pos.begin;
-        MOZ_ASSERT(offset >= tokenStream.options().column);
-        MOZ_ASSERT(toStringStart >= tokenStream.options().column);
-        toStringStart -= tokenStream.options().column;
-        bufStart = offset - tokenStream.options().column;
+        uint32_t sourceStartColumn = tokenStream.options().sourceStartColumn;
+
+        MOZ_ASSERT(offset >= sourceStartColumn);
+        MOZ_ASSERT(toStringStart >= sourceStartColumn);
+        toStringStart -= sourceStartColumn;
+        bufStart = offset - sourceStartColumn;
         tokenStream.srcCoords.lineNumAndColumnIndex(offset, &startLine, &startColumn);
     }
 
     void setEnd(const TokenStream& tokenStream) {
         // For all functions except class constructors, the buffer and
         // toString ending positions are the same. Class constructors override
         // the toString ending position with the end of the class definition.
         //
         // Offsets are de-offset for the same reason as in setStart above.
         uint32_t offset = tokenStream.currentToken().pos.end;
-        MOZ_ASSERT(offset >= tokenStream.options().column);
-        bufEnd = offset - tokenStream.options().column;
+        uint32_t sourceStartColumn = tokenStream.options().sourceStartColumn;
+
+        MOZ_ASSERT(offset >= sourceStartColumn);
+        bufEnd = offset - sourceStartColumn;
         toStringEnd = bufEnd;
     }
 
     void trace(JSTracer* trc) override;
 };
 
 inline FunctionBox*
 SharedContext::asFunctionBox()
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/parser/bug-1364648.js
@@ -0,0 +1,1 @@
+assertEq(evaluate("var f = x=>class { }; f()", { columnNumber: 1729 }).toString(), "class { }");
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3880,16 +3880,17 @@ JS::TransitiveCompileOptions::copyPODTra
 };
 
 void
 JS::ReadOnlyCompileOptions::copyPODOptions(const ReadOnlyCompileOptions& rhs)
 {
     copyPODTransitiveOptions(rhs);
     lineno = rhs.lineno;
     column = rhs.column;
+    sourceStartColumn = rhs.sourceStartColumn;
     isRunOnce = rhs.isRunOnce;
     noScriptRval = rhs.noScriptRval;
 }
 
 JS::OwningCompileOptions::OwningCompileOptions(JSContext* cx)
     : ReadOnlyCompileOptions(),
       elementRoot(cx),
       elementAttributeNameRoot(cx),
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3966,16 +3966,17 @@ class JS_FRIEND_API(ReadOnlyCompileOptio
 {
     friend class CompileOptions;
 
   protected:
     ReadOnlyCompileOptions()
       : TransitiveCompileOptions(),
         lineno(1),
         column(0),
+        sourceStartColumn(0),
         isRunOnce(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);
 
@@ -3988,16 +3989,17 @@ class JS_FRIEND_API(ReadOnlyCompileOptio
     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;
+    unsigned sourceStartColumn;
     // isRunOnce only applies to non-function scripts.
     bool isRunOnce;
     bool noScriptRval;
 
   private:
     void operator=(const ReadOnlyCompileOptions&) = delete;
 };
 
@@ -4060,17 +4062,22 @@ class JS_FRIEND_API(OwningCompileOptions
         return *this;
     }
     OwningCompileOptions& setVersion(JSVersion v) {
         version = v;
         versionSet = true;
         return *this;
     }
     OwningCompileOptions& setUTF8(bool u) { utf8 = u; return *this; }
-    OwningCompileOptions& setColumn(unsigned c) { column = c; return *this; }
+    OwningCompileOptions& setColumn(unsigned c, unsigned ssc) {
+        MOZ_ASSERT(ssc <= c);
+        column = c;
+        sourceStartColumn = ssc;
+        return *this;
+    }
     OwningCompileOptions& setIsRunOnce(bool once) { isRunOnce = once; return *this; }
     OwningCompileOptions& setNoScriptRval(bool nsr) { noScriptRval = nsr; return *this; }
     OwningCompileOptions& setSelfHostingMode(bool shm) { selfHostingMode = shm; return *this; }
     OwningCompileOptions& setCanLazilyParse(bool clp) { canLazilyParse = clp; return *this; }
     OwningCompileOptions& setSourceIsLazy(bool l) { sourceIsLazy = l; return *this; }
     OwningCompileOptions& setIntroductionType(const char* t) { introductionType = t; return *this; }
     bool setIntroductionInfo(JSContext* cx, const char* introducerFn, const char* intro,
                              unsigned line, JSScript* script, uint32_t offset)
@@ -4156,17 +4163,22 @@ class MOZ_STACK_CLASS JS_FRIEND_API(Comp
         return *this;
     }
     CompileOptions& setVersion(JSVersion v) {
         version = v;
         versionSet = true;
         return *this;
     }
     CompileOptions& setUTF8(bool u) { utf8 = u; return *this; }
-    CompileOptions& setColumn(unsigned c) { column = c; return *this; }
+    CompileOptions& setColumn(unsigned c, unsigned ssc) {
+        MOZ_ASSERT(ssc <= c);
+        column = c;
+        sourceStartColumn = ssc;
+        return *this;
+    }
     CompileOptions& setIsRunOnce(bool once) { isRunOnce = once; return *this; }
     CompileOptions& setNoScriptRval(bool nsr) { noScriptRval = nsr; return *this; }
     CompileOptions& setSelfHostingMode(bool shm) { selfHostingMode = shm; return *this; }
     CompileOptions& setCanLazilyParse(bool clp) { canLazilyParse = clp; return *this; }
     CompileOptions& setSourceIsLazy(bool l) { sourceIsLazy = l; return *this; }
     CompileOptions& setIntroductionType(const char* t) { introductionType = t; return *this; }
     CompileOptions& setIntroductionInfo(const char* introducerFn, const char* intro,
                                         unsigned line, JSScript* script, uint32_t offset)
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -2268,16 +2268,17 @@ ScriptSource::initFromOptions(JSContext*
 {
     MOZ_ASSERT(!filename_);
     MOZ_ASSERT(!introducerFilename_);
 
     mutedErrors_ = options.mutedErrors();
 
     introductionType_ = options.introductionType;
     setIntroductionOffset(options.introductionOffset);
+    startColumn_ = options.sourceStartColumn;
     parameterListEnd_ = parameterListEnd.isSome() ? parameterListEnd.value() : 0;
 
     if (options.hasIntroductionInfo) {
         MOZ_ASSERT(options.introductionType != nullptr);
         const char* filename = options.filename() ? options.filename() : "<unknown>";
         char* formatted = FormatIntroducedFilename(cx, filename, options.introductionLineno,
                                                    options.introductionType);
         if (!formatted)
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -425,16 +425,22 @@ class ScriptSource
 
     // The filename of this script.
     UniqueChars filename_;
 
     UniqueTwoByteChars displayURL_;
     UniqueTwoByteChars sourceMapURL_;
     bool mutedErrors_;
 
+    // The start column of the source. Offsets kept for toString and the
+    // function source in LazyScripts are absolute positions within a
+    // ScriptSource buffer. To get their positions, they need to be offset
+    // with the starting column.
+    uint32_t startColumn_;
+
     // bytecode offset in caller script that generated this code.
     // This is present for eval-ed code, as well as "new Function(...)"-introduced
     // scripts.
     uint32_t introductionOffset_;
 
     // If this source is for Function constructor, the position of ")" after
     // parameter list in the source.  This is used to get function body.
     // 0 for other cases.
@@ -501,16 +507,17 @@ class ScriptSource
     explicit ScriptSource()
       : refs(0),
         data(SourceType(Missing())),
         pinnedCharsStack_(nullptr),
         filename_(nullptr),
         displayURL_(nullptr),
         sourceMapURL_(nullptr),
         mutedErrors_(false),
+        startColumn_(0),
         introductionOffset_(0),
         parameterListEnd_(0),
         introducerFilename_(nullptr),
         introductionType_(nullptr),
         xdrEncoder_(nullptr),
         sourceRetrievable_(false),
         hasIntroductionOffset_(false)
     {
@@ -613,16 +620,18 @@ class ScriptSource
     bool hasSourceMapURL() const { return sourceMapURL_ != nullptr; }
     const char16_t * sourceMapURL() {
         MOZ_ASSERT(hasSourceMapURL());
         return sourceMapURL_.get();
     }
 
     bool mutedErrors() const { return mutedErrors_; }
 
+    uint32_t startColumn() const { return startColumn_; }
+
     bool hasIntroductionOffset() const { return hasIntroductionOffset_; }
     uint32_t introductionOffset() const {
         MOZ_ASSERT(hasIntroductionOffset());
         return introductionOffset_;
     }
     void setIntroductionOffset(uint32_t offset) {
         MOZ_ASSERT(!hasIntroductionOffset());
         MOZ_ASSERT(offset <= (uint32_t)INT32_MAX);
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -1384,17 +1384,17 @@ ParseCompileOptions(JSContext* cx, Compi
     }
 
     if (!JS_GetProperty(cx, opts, "columnNumber", &v))
         return false;
     if (!v.isUndefined()) {
         int32_t c;
         if (!ToInt32(cx, v, &c))
             return false;
-        options.setColumn(c);
+        options.setColumn(c, c);
     }
 
     if (!JS_GetProperty(cx, opts, "sourceIsLazy", &v))
         return false;
     if (v.isBoolean())
         options.setSourceIsLazy(v.toBoolean());
 
     return true;