Bug 755821: Function() should use the parser's argument parsing code

4 files changed, 111 insertions, 128 deletions

diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp
index d4c758b6c2..3fbfdaa1ab 100644
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -7,6 +7,7 @@
 #include "frontend/BytecodeCompiler.h"
 
 #include "mozilla/IntegerPrintfMacros.h"
+#include "mozilla/Maybe.h"
 
 #include "jscntxt.h"
 #include "jsscript.h"
@@ -28,6 +29,7 @@
 using namespace js;
 using namespace js::frontend;
 using mozilla::Maybe;
+using mozilla::Nothing;
 
 class MOZ_STACK_CLASS AutoCompilationTraceLogger
 {
@@ -57,24 +59,24 @@ class MOZ_STACK_CLASS BytecodeCompiler
 
     // Call setters for optional arguments.
     void maybeSetSourceCompressor(SourceCompressionTask* sourceCompressor);
-    void setSourceArgumentsNotIncluded();
 
     JSScript* compileGlobalScript(ScopeKind scopeKind);
     JSScript* compileEvalScript(HandleObject environment, HandleScope enclosingScope);
     ModuleObject* compileModule();
-    bool compileFunctionBody(MutableHandleFunction fun, Handle<PropertyNameVector> formals,
-                             GeneratorKind generatorKind, FunctionAsyncKind asyncKind);
+    bool compileStandaloneFunction(MutableHandleFunction fun, GeneratorKind generatorKind,
+                                   FunctionAsyncKind asyncKind,
+                                   Maybe<uint32_t> parameterListEnd);
 
     ScriptSourceObject* sourceObjectPtr() const;
 
   private:
     JSScript* compileScript(HandleObject environment, SharedContext* sc);
     bool checkLength();
-    bool createScriptSource();
+    bool createScriptSource(Maybe<uint32_t> parameterListEnd);
     bool maybeCompressSource();
     bool canLazilyParse();
     bool createParser();
-    bool createSourceAndParser();
+    bool createSourceAndParser(Maybe<uint32_t> parameterListEnd = Nothing());
     bool createScript();
     bool emplaceEmitter(Maybe<BytecodeEmitter>& emitter, SharedContext* sharedContext);
     bool handleParseFailure(const Directives& newDirectives);
@@ -90,7 +92,6 @@ class MOZ_STACK_CLASS BytecodeCompiler
     SourceBufferHolder& sourceBuffer;
 
     RootedScope enclosingScope;
-    bool sourceArgumentsNotIncluded;
 
     RootedScriptSource sourceObject;
     ScriptSource* scriptSource;
@@ -130,7 +131,6 @@ BytecodeCompiler::BytecodeCompiler(ExclusiveContext* cx,
     options(options),
     sourceBuffer(sourceBuffer),
     enclosingScope(cx, enclosingScope),
-    sourceArgumentsNotIncluded(false),
     sourceObject(cx),
     scriptSource(nullptr),
     sourceCompressor(nullptr),
@@ -147,12 +147,6 @@ BytecodeCompiler::maybeSetSourceCompressor(SourceCompressionTask* sourceCompress
     this->sourceCompressor = sourceCompressor;
 }
 
-void
-BytecodeCompiler::setSourceArgumentsNotIncluded()
-{
-    sourceArgumentsNotIncluded = true;
-}
-
 bool
 BytecodeCompiler::checkLength()
 {
@@ -169,12 +163,12 @@ BytecodeCompiler::checkLength()
 }
 
 bool
-BytecodeCompiler::createScriptSource()
+BytecodeCompiler::createScriptSource(Maybe<uint32_t> parameterListEnd)
 {
     if (!checkLength())
         return false;
 
-    sourceObject = CreateScriptSourceObject(cx, options);
+    sourceObject = CreateScriptSourceObject(cx, options, parameterListEnd);
     if (!sourceObject)
         return false;
 
@@ -193,9 +187,7 @@ BytecodeCompiler::maybeCompressSource()
     if (!cx->compartment()->behaviors().discardSource()) {
         if (options.sourceIsLazy) {
             scriptSource->setSourceRetrievable();
-        } else if (!scriptSource->setSourceCopy(cx, sourceBuffer, sourceArgumentsNotIncluded,
-                                                sourceCompressor))
-        {
+        } else if (!scriptSource->setSourceCopy(cx, sourceBuffer, sourceCompressor)) {
             return false;
         }
     }
@@ -242,9 +234,9 @@ BytecodeCompiler::createParser()
 }
 
 bool
-BytecodeCompiler::createSourceAndParser()
+BytecodeCompiler::createSourceAndParser(Maybe<uint32_t> parameterListEnd /* = Nothing() */)
 {
-    return createScriptSource() &&
+    return createScriptSource(parameterListEnd) &&
            maybeCompressSource() &&
            createParser();
 }
@@ -432,18 +424,19 @@ BytecodeCompiler::compileModule()
     return module;
 }
 
+// Compile a standalone JS function, which might appear as the value of an
+// event handler attribute in an HTML <INPUT> tag, or in a Function()
+// constructor.
 bool
-BytecodeCompiler::compileFunctionBody(MutableHandleFunction fun,
-                                      Handle<PropertyNameVector> formals,
-                                      GeneratorKind generatorKind,
-                                      FunctionAsyncKind asyncKind)
+BytecodeCompiler::compileStandaloneFunction(MutableHandleFunction fun,
+                                            GeneratorKind generatorKind,
+                                            FunctionAsyncKind asyncKind,
+                                            Maybe<uint32_t> parameterListEnd)
 {
     MOZ_ASSERT(fun);
     MOZ_ASSERT(fun->isTenured());
 
-    fun->setArgCount(formals.length());
-
-    if (!createSourceAndParser())
+    if (!createSourceAndParser(parameterListEnd))
         return false;
 
     // Speculatively parse using the default directives implied by the context.
@@ -454,8 +447,8 @@ BytecodeCompiler::compileFunctionBody(MutableHandleFunction fun,
     ParseNode* fn;
     do {
         Directives newDirectives = directives;
-        fn = parser->standaloneFunctionBody(fun, enclosingScope, formals, generatorKind, asyncKind,
-                                            directives, &newDirectives);
+        fn = parser->standaloneFunction(fun, enclosingScope, parameterListEnd, generatorKind,
+                                        asyncKind, directives, &newDirectives);
         if (!fn && !handleParseFailure(newDirectives))
             return false;
     } while (!fn);
@@ -492,14 +485,15 @@ BytecodeCompiler::sourceObjectPtr() const
 }
 
 ScriptSourceObject*
-frontend::CreateScriptSourceObject(ExclusiveContext* cx, const ReadOnlyCompileOptions& options)
+frontend::CreateScriptSourceObject(ExclusiveContext* cx, const ReadOnlyCompileOptions& options,
+                                   Maybe<uint32_t> parameterListEnd /* = Nothing() */)
 {
     ScriptSource* ss = cx->new_<ScriptSource>();
     if (!ss)
         return nullptr;
     ScriptSourceHolder ssHolder(ss);
 
-    if (!ss->initFromOptions(cx, options))
+    if (!ss->initFromOptions(cx, options, parameterListEnd))
         return nullptr;
 
     RootedScriptSource sso(cx, ScriptSourceObject::create(cx, ss));
@@ -676,63 +670,44 @@ frontend::CompileLazyFunction(JSContext* cx, Handle<LazyScript*> lazy, const cha
     return bce.emitFunctionScript(pn->pn_body);
 }
 
-// Compile a JS function body, which might appear as the value of an event
-// handler attribute in an HTML <INPUT> tag, or in a Function() constructor.
-static bool
-CompileFunctionBody(JSContext* cx, MutableHandleFunction fun, const ReadOnlyCompileOptions& options,
-                    Handle<PropertyNameVector> formals, SourceBufferHolder& srcBuf,
-                    HandleScope enclosingScope, GeneratorKind generatorKind,
-                    FunctionAsyncKind asyncKind)
+bool
+frontend::CompileStandaloneFunction(JSContext* cx, MutableHandleFunction fun,
+                                    const ReadOnlyCompileOptions& options,
+                                    JS::SourceBufferHolder& srcBuf,
+                                    Maybe<uint32_t> parameterListEnd,
+                                    HandleScope enclosingScope /* = nullptr */)
 {
-    MOZ_ASSERT(!options.isRunOnce);
-
-    // FIXME: make Function pass in two strings and parse them as arguments and
-    // ProgramElements respectively.
+    RootedScope scope(cx, enclosingScope);
+    if (!scope)
+        scope = &cx->global()->emptyGlobalScope();
 
-    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, enclosingScope,
+    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, scope,
                               TraceLogger_ParserCompileFunction);
-    compiler.setSourceArgumentsNotIncluded();
-    return compiler.compileFunctionBody(fun, formals, generatorKind, asyncKind);
+    return compiler.compileStandaloneFunction(fun, NotGenerator, SyncFunction, parameterListEnd);
 }
 
 bool
-frontend::CompileFunctionBody(JSContext* cx, MutableHandleFunction fun,
-                              const ReadOnlyCompileOptions& options,
-                              Handle<PropertyNameVector> formals, JS::SourceBufferHolder& srcBuf,
-                              HandleScope enclosingScope)
-{
-    return CompileFunctionBody(cx, fun, options, formals, srcBuf, enclosingScope, NotGenerator,
-                               SyncFunction);
-}
-
-bool
-frontend::CompileFunctionBody(JSContext* cx, MutableHandleFunction fun,
-                              const ReadOnlyCompileOptions& options,
-                              Handle<PropertyNameVector> formals, JS::SourceBufferHolder& srcBuf)
+frontend::CompileStandaloneGenerator(JSContext* cx, MutableHandleFunction fun,
+                                     const ReadOnlyCompileOptions& options,
+                                     JS::SourceBufferHolder& srcBuf,
+                                     Maybe<uint32_t> parameterListEnd)
 {
     RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
-    return CompileFunctionBody(cx, fun, options, formals, srcBuf, emptyGlobalScope,
-                               NotGenerator, SyncFunction);
-}
 
-bool
-frontend::CompileStarGeneratorBody(JSContext* cx, MutableHandleFunction fun,
-                                   const ReadOnlyCompileOptions& options,
-                                   Handle<PropertyNameVector> formals,
-                                   JS::SourceBufferHolder& srcBuf)
-{
-    RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
-    return CompileFunctionBody(cx, fun, options, formals, srcBuf, emptyGlobalScope,
-                               StarGenerator, SyncFunction);
+    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, emptyGlobalScope,
+                              TraceLogger_ParserCompileFunction);
+    return compiler.compileStandaloneFunction(fun, StarGenerator, SyncFunction, parameterListEnd);
 }
 
 bool
-frontend::CompileAsyncFunctionBody(JSContext* cx, MutableHandleFunction fun,
-                                   const ReadOnlyCompileOptions& options,
-                                   Handle<PropertyNameVector> formals,
-                                   JS::SourceBufferHolder& srcBuf)
+frontend::CompileStandaloneAsyncFunction(JSContext* cx, MutableHandleFunction fun,
+                                         const ReadOnlyCompileOptions& options,
+                                         JS::SourceBufferHolder& srcBuf,
+                                         Maybe<uint32_t> parameterListEnd)
 {
     RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
-    return CompileFunctionBody(cx, fun, options, formals, srcBuf, emptyGlobalScope,
-                               StarGenerator, AsyncFunction);
+
+    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, emptyGlobalScope,
+                              TraceLogger_ParserCompileFunction);
+    return compiler.compileStandaloneFunction(fun, StarGenerator, AsyncFunction, parameterListEnd);
 }
diff --git a/js/src/frontend/BytecodeCompiler.h b/js/src/frontend/BytecodeCompiler.h
index 1d86f11607..72e9676395 100644
--- a/js/src/frontend/BytecodeCompiler.h
+++ b/js/src/frontend/BytecodeCompiler.h
@@ -7,6 +7,8 @@
 #ifndef frontend_BytecodeCompiler_h
 #define frontend_BytecodeCompiler_h
 
+#include "mozilla/Maybe.h"
+
 #include "NamespaceImports.h"
 
 #include "vm/Scope.h"
@@ -51,22 +53,36 @@ CompileModule(ExclusiveContext* cx, const ReadOnlyCompileOptions& options,
 MOZ_MUST_USE bool
 CompileLazyFunction(JSContext* cx, Handle<LazyScript*> lazy, const char16_t* chars, size_t length);
 
+//
+// Compile a single function. The source in srcBuf must match the ECMA-262
+// FunctionExpression production.
+//
+// If nonzero, parameterListEnd is the offset within srcBuf where the parameter
+// list is expected to end. During parsing, if we find that it ends anywhere
+// else, it's a SyntaxError. This is used to implement the Function constructor;
+// it's how we detect that these weird cases are SyntaxErrors:
+//
+//     Function("/*", "*/x) {")
+//     Function("x){ if (3", "return x;}")
+//
 MOZ_MUST_USE bool
-CompileFunctionBody(JSContext* cx, MutableHandleFunction fun,
-                    const ReadOnlyCompileOptions& options,
-                    Handle<PropertyNameVector> formals, JS::SourceBufferHolder& srcBuf,
-                    HandleScope enclosingScope);
+CompileStandaloneFunction(JSContext* cx, MutableHandleFunction fun,
+                          const ReadOnlyCompileOptions& options,
+                          JS::SourceBufferHolder& srcBuf,
+                          mozilla::Maybe<uint32_t> parameterListEnd,
+                          HandleScope enclosingScope = nullptr);
 
-// As above, but defaults to the global lexical scope as the enclosing scope.
 MOZ_MUST_USE bool
-CompileFunctionBody(JSContext* cx, MutableHandleFunction fun,
-                    const ReadOnlyCompileOptions& options,
-                    Handle<PropertyNameVector> formals, JS::SourceBufferHolder& srcBuf);
+CompileStandaloneGenerator(JSContext* cx, MutableHandleFunction fun,
+                           const ReadOnlyCompileOptions& options,
+                           JS::SourceBufferHolder& srcBuf,
+                           mozilla::Maybe<uint32_t> parameterListEnd);
 
 MOZ_MUST_USE bool
-CompileStarGeneratorBody(JSContext* cx, MutableHandleFunction fun,
-                         const ReadOnlyCompileOptions& options,
-                         Handle<PropertyNameVector> formals, JS::SourceBufferHolder& srcBuf);
+CompileStandaloneAsyncFunction(JSContext* cx, MutableHandleFunction fun,
+                               const ReadOnlyCompileOptions& options,
+                               JS::SourceBufferHolder& srcBuf,
+                               mozilla::Maybe<uint32_t> parameterListEnd);
 
 MOZ_MUST_USE bool
 CompileAsyncFunctionBody(JSContext* cx, MutableHandleFunction fun,
@@ -74,7 +90,8 @@ CompileAsyncFunctionBody(JSContext* cx, MutableHandleFunction fun,
                          Handle<PropertyNameVector> formals, JS::SourceBufferHolder& srcBuf);
 
 ScriptSourceObject*
-CreateScriptSourceObject(ExclusiveContext* cx, const ReadOnlyCompileOptions& options);
+CreateScriptSourceObject(ExclusiveContext* cx, const ReadOnlyCompileOptions& options,
+                         mozilla::Maybe<uint32_t> parameterListEnd = mozilla::Nothing());
 
 /*
  * True if str consists of an IdentifierStart character, followed by one or
diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp
index 49fef2bf95..f42546eb57 100644
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -2245,13 +2245,13 @@ GetYieldHandling(GeneratorKind generatorKind, FunctionAsyncKind asyncKind)
 
 template <>
 ParseNode*
-Parser<FullParseHandler>::standaloneFunctionBody(HandleFunction fun,
-                                                 HandleScope enclosingScope,
-                                                 Handle<PropertyNameVector> formals,
-                                                 GeneratorKind generatorKind,
-                                                 FunctionAsyncKind asyncKind,
-                                                 Directives inheritedDirectives,
-                                                 Directives* newDirectives)
+Parser<FullParseHandler>::standaloneFunction(HandleFunction fun,
+                                             HandleScope enclosingScope,
+                                             Maybe<uint32_t> parameterListEnd,
+                                             GeneratorKind generatorKind,
+                                             FunctionAsyncKind asyncKind,
+                                             Directives inheritedDirectives,
+                                             Directives* newDirectives)
 {
     MOZ_ASSERT(checkOptionsCalled);
 
@@ -2274,25 +2274,14 @@ Parser<FullParseHandler>::standaloneFunctionBody(HandleFunction fun,
     if (!funpc.init())
         return null();
     funpc.setIsStandaloneFunctionBody();
-    funpc.functionScope().useAsVarScope(&funpc);
-
-    if (formals.length() >= ARGNO_LIMIT) {
-        report(ParseError, false, null(), JSMSG_TOO_MANY_FUN_ARGS);
-        return null();
-    }
-
-    bool duplicatedParam = false;
-    for (uint32_t i = 0; i < formals.length(); i++) {
-        if (!notePositionalFormalParameter(fn, formals[i], false, &duplicatedParam))
-            return null();
-    }
-    funbox->hasDuplicateParameters = duplicatedParam;
 
     YieldHandling yieldHandling = GetYieldHandling(generatorKind, asyncKind);
     AutoAwaitIsKeyword awaitIsKeyword(&tokenStream, asyncKind == AsyncFunction);
-    ParseNode* pn = functionBody(InAllowed, yieldHandling, Statement, StatementListBody);
-    if (!pn)
+    if (!functionFormalParametersAndBody(InAllowed, yieldHandling, fn, Statement,
+                                         parameterListEnd))
+    {
         return null();
+    }
 
     TokenKind tt;
     if (!tokenStream.getToken(&tt, TokenStream::Operand))
@@ -2303,15 +2292,7 @@ Parser<FullParseHandler>::standaloneFunctionBody(HandleFunction fun,
         return null();
     }
 
-    if (!FoldConstants(context, &pn, this))
-        return null();
-
-    fn->pn_pos.end = pos().end;
-
-    MOZ_ASSERT(fn->pn_body->isKind(PNK_PARAMSBODY));
-    fn->pn_body->append(pn);
-
-    if (!finishFunction())
+    if (!FoldConstants(context, &fn, this))
         return null();
 
     return fn;
@@ -3415,7 +3396,8 @@ template <typename ParseHandler>
 bool
 Parser<ParseHandler>::functionFormalParametersAndBody(InHandling inHandling,
                                                       YieldHandling yieldHandling,
-                                                      Node pn, FunctionSyntaxKind kind)
+                                                      Node pn, FunctionSyntaxKind kind,
+                                                      Maybe<uint32_t> parameterListEnd /* = Nothing() */)
 {
     // Given a properly initialized parse context, try to parse an actual
     // function without concern for conversion to strict mode, use of lazy
@@ -3447,6 +3429,13 @@ Parser<ParseHandler>::functionFormalParametersAndBody(InHandling inHandling,
         }
     }
 
+    // When parsing something for new Function() we have to make sure to
+    // only treat a certain part of the source as a parameter list.
+    if (parameterListEnd.isSome() && parameterListEnd.value() != pos().begin) {
+        report(ParseError, false, null(), JSMSG_UNEXPECTED_PARAMLIST_END);
+        return false;
+    }
+
     // Parse the function body.
     FunctionBodyType bodyType = StatementListBody;
     TokenKind tt;
@@ -3502,7 +3491,7 @@ Parser<ParseHandler>::functionFormalParametersAndBody(InHandling inHandling,
             report(ParseError, false, null(), JSMSG_CURLY_AFTER_BODY);
             return false;
         }
-        funbox->bufEnd = pos().begin + 1;
+        funbox->bufEnd = pos().end;
     } else {
 #if !JS_HAS_EXPR_CLOSURES
         MOZ_ASSERT(kind == Arrow);
diff --git a/js/src/frontend/Parser.h b/js/src/frontend/Parser.h
index 0ad4d56a0b..b58b021cd7 100644
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -1020,12 +1020,12 @@ class Parser final : private JS::AutoGCRooter, public StrictModeGetter
     // Parse a module.
     Node moduleBody(ModuleSharedContext* modulesc);
 
-    // Parse a function, given only its body. Used for the Function and
-    // Generator constructors.
-    Node standaloneFunctionBody(HandleFunction fun, HandleScope enclosingScope,
-                                Handle<PropertyNameVector> formals,
-                                GeneratorKind generatorKind, FunctionAsyncKind asyncKind,
-                                Directives inheritedDirectives, Directives* newDirectives);
+    // Parse a function, used for the Function, GeneratorFunction, and
+    // AsyncFunction constructors.
+    Node standaloneFunction(HandleFunction fun, HandleScope enclosingScope,
+                            mozilla::Maybe<uint32_t> parameterListEnd,
+                            GeneratorKind generatorKind, FunctionAsyncKind asyncKind,
+                            Directives inheritedDirectives, Directives* newDirectives);
 
     // Parse a function, given only its arguments and body. Used for lazily
     // parsed functions.
@@ -1041,7 +1041,9 @@ class Parser final : private JS::AutoGCRooter, public StrictModeGetter
     // Parse a function's formal parameters and its body assuming its function
     // ParseContext is already on the stack.
     bool functionFormalParametersAndBody(InHandling inHandling, YieldHandling yieldHandling,
-                                         Node pn, FunctionSyntaxKind kind);
+                                         Node pn, FunctionSyntaxKind kind,
+                                         mozilla::Maybe<uint32_t> parameterListEnd = mozilla::Nothing());
+ 
 
     // Determine whether |yield| is a valid name in the current context, or
     // whether it's prohibited due to strictness, JS version, or occurrence