Harden Function()'s parsing of function literals.

R=rossberg@chromium.org
BUG=v8:2470
TEST=mjsunit/regress/regress-2470

Review URL: https://codereview.chromium.org/12613007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13867 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/compiler.cc b/src/compiler.cc
index 100d100..c4ef8eb 100644 (file)
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -681,6 +681,7 @@ Handle<SharedFunctionInfo> Compiler::CompileEval(Handle<String> source,
                                                  Handle<Context> context,
                                                  bool is_global,
                                                  LanguageMode language_mode,
+                                                 ParseRestriction restriction,
                                                  int scope_position) {
   Isolate* isolate = source->GetIsolate();
   int source_length = source->length();
@@ -707,6 +708,7 @@ Handle<SharedFunctionInfo> Compiler::CompileEval(Handle<String> source,
     info.MarkAsEval();
     if (is_global) info.MarkAsGlobal();
     info.SetLanguageMode(language_mode);
+    info.SetParseRestriction(restriction);
     info.SetContext(context);
     result = MakeFunctionInfo(&info);
     if (!result.is_null()) {

diff --git a/src/compiler.h b/src/compiler.h
index 0191bb2..5e69661 100644 (file)
--- a/src/compiler.h
+++ b/src/compiler.h
@@ -40,6 +40,13 @@ static const int kPrologueOffsetNotSet = -1;
 class ScriptDataImpl;
 class HydrogenCodeStub;
 
+// ParseRestriction is used to restrict the set of valid statements in a
+// unit of compilation.  Restriction violations cause a syntax error.
+enum ParseRestriction {
+  NO_PARSE_RESTRICTION,         // All expressions are allowed.
+  ONLY_SINGLE_FUNCTION_LITERAL  // Only a single FunctionLiteral expression.
+};
+
 // CompilationInfo encapsulates some information known at compile time.  It
 // is constructed based on the resources available at compile-time.
 class CompilationInfo {
@@ -55,9 +62,7 @@ class CompilationInfo {
     ASSERT(Isolate::Current() == isolate_);
     return isolate_;
   }
-  Zone* zone() {
-    return zone_;
-  }
+  Zone* zone() { return zone_; }
   bool is_lazy() const { return IsLazy::decode(flags_); }
   bool is_eval() const { return IsEval::decode(flags_); }
   bool is_global() const { return IsGlobal::decode(flags_); }
@@ -138,6 +143,14 @@ class CompilationInfo {
     return SavesCallerDoubles::decode(flags_);
   }
 
+  void SetParseRestriction(ParseRestriction restriction) {
+    flags_ = ParseRestricitonField::update(flags_, restriction);
+  }
+
+  ParseRestriction parse_restriction() const {
+    return ParseRestricitonField::decode(flags_);
+  }
+
   void SetFunction(FunctionLiteral* literal) {
     ASSERT(function_ == NULL);
     function_ = literal;
@@ -285,7 +298,8 @@ class CompilationInfo {
   class IsNonDeferredCalling: public BitField<bool, 11, 1> {};
   // If the compiled code saves double caller registers that it clobbers.
   class SavesCallerDoubles: public BitField<bool, 12, 1> {};
-
+  // If the set of valid statements is restricted.
+  class ParseRestricitonField: public BitField<ParseRestriction, 13, 1> {};
 
   unsigned flags_;
 
@@ -502,6 +516,7 @@ class Compiler : public AllStatic {
                                                 Handle<Context> context,
                                                 bool is_global,
                                                 LanguageMode language_mode,
+                                                ParseRestriction restriction,
                                                 int scope_position);
 
   // Compile from function info (used for lazy compilation). Returns true on

diff --git a/src/parser.cc b/src/parser.cc
index 62b633e..2b2cf65 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -656,6 +656,16 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info,
       CheckConflictingVarDeclarations(top_scope_, &ok);
     }
 
+    if (ok && info->parse_restriction() == ONLY_SINGLE_FUNCTION_LITERAL) {
+      if (body->length() != 1 ||
+          !body->at(0)->IsExpressionStatement() ||
+          !body->at(0)->AsExpressionStatement()->
+              expression()->IsFunctionLiteral()) {
+        ReportMessage("unable_to_parse", Vector<const char*>::empty());
+        ok = false;
+      }
+    }
+
     if (ok) {
       result = factory()->NewFunctionLiteral(
           no_name,

diff --git a/src/runtime.cc b/src/runtime.cc
index 5bbca4d..f88651a 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -8957,8 +8957,9 @@ bool CodeGenerationFromStringsAllowed(Isolate* isolate,
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_CompileString) {
   HandleScope scope(isolate);
-  ASSERT_EQ(1, args.length());
+  ASSERT_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(String, source, 0);
+  CONVERT_BOOLEAN_ARG_CHECKED(function_literal_only, 1);
 
   // Extract native context.
   Handle<Context> context(isolate->context()->native_context());
@@ -8974,8 +8975,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CompileString) {
   }
 
   // Compile source string in the native context.
+  ParseRestriction restriction = function_literal_only
+      ? ONLY_SINGLE_FUNCTION_LITERAL : NO_PARSE_RESTRICTION;
   Handle<SharedFunctionInfo> shared = Compiler::CompileEval(
-      source, context, true, CLASSIC_MODE, RelocInfo::kNoPosition);
+      source, context, true, CLASSIC_MODE, restriction, RelocInfo::kNoPosition);
   if (shared.is_null()) return Failure::Exception();
   Handle<JSFunction> fun =
       isolate->factory()->NewFunctionFromSharedFunctionInfo(shared,
@@ -9011,6 +9014,7 @@ static ObjectPair CompileGlobalEval(Isolate* isolate,
       Handle<Context>(isolate->context()),
       context->IsNativeContext(),
       language_mode,
+      NO_PARSE_RESTRICTION,
       scope_position);
   if (shared.is_null()) return MakePair(Failure::Exception(), NULL);
   Handle<JSFunction> compiled =
@@ -12003,6 +12007,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) {
                             context,
                             context->IsNativeContext(),
                             CLASSIC_MODE,
+                            NO_PARSE_RESTRICTION,
                             RelocInfo::kNoPosition);
   if (shared.is_null()) return Failure::Exception();
   Handle<JSFunction> compiled_function =
@@ -12109,6 +12114,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluateGlobal) {
                             context,
                             is_global,
                             CLASSIC_MODE,
+                            NO_PARSE_RESTRICTION,
                             RelocInfo::kNoPosition);
   if (shared.is_null()) return Failure::Exception();
   Handle<JSFunction> compiled_function =

diff --git a/src/runtime.h b/src/runtime.h
index cd6805c..ac4a207 100644 (file)
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -265,7 +265,7 @@ namespace internal {
   /* Numbers */ \
   \
   /* Globals */ \
-  F(CompileString, 1, 1) \
+  F(CompileString, 2, 1) \
   F(GlobalPrint, 1, 1) \
   \
   /* Eval */ \

diff --git a/src/v8natives.js b/src/v8natives.js
index f295c3a..826ae43 100644 (file)
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -174,7 +174,7 @@ function GlobalEval(x) {
                          'be the global object from which eval originated');
   }
 
-  var f = %CompileString(x);
+  var f = %CompileString(x, false);
   if (!IS_FUNCTION(f)) return f;
 
   return %_CallFunction(global_receiver, f);
@@ -1704,14 +1704,18 @@ function NewFunction(arg1) {  // length == 1
     // character - it may make the combined function expression
     // compile. We avoid this problem by checking for this early on.
     if (p.indexOf(')') != -1) throw MakeSyntaxError('unable_to_parse',[]);
+    // If the formal parameters include an unbalanced block comment, the
+    // function must be rejected. Since JavaScript does not allow nested
+    // comments we can include a trailing block comment to catch this.
+    p += '\n/' + '**/';
   }
   var body = (n > 0) ? ToString(%_Arguments(n - 1)) : '';
-  var source = '(function(' + p + ') {\n' + body + '\n})';
+  var source = '(function(\n' + p + '\n){\n' + body + '\n})';
 
   // The call to SetNewFunctionAttributes will ensure the prototype
   // property of the resulting function is enumerable (ECMA262, 15.3.5.2).
   var global_receiver = %GlobalReceiver(global);
-  var f = %_CallFunction(global_receiver, %CompileString(source));
+  var f = %_CallFunction(global_receiver, %CompileString(source, true));
 
   %FunctionMarkNameShouldPrintAsAnonymous(f);
   return %SetNewFunctionAttributes(f);

diff --git a/test/mjsunit/new-function.js b/test/mjsunit/new-function.js
index 9e8cc27..794bd76 100644 (file)
--- a/test/mjsunit/new-function.js
+++ b/test/mjsunit/new-function.js
@@ -25,10 +25,10 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-var x;
+var x = 0;
 try {
   Function("}), x = this, (function() {");
 } catch(e) {
   print("Caught " + e);
 }
-assertTrue(x == "[object global]");
+assertTrue(x === 0);

diff --git a/test/mjsunit/regress/regress-2470.js b/test/mjsunit/regress/regress-2470.js
new file mode 100644 (file)
index 0000000..eed3816
--- /dev/null
+++ b/test/mjsunit/regress/regress-2470.js
@@ -0,0 +1,42 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test whether the opening parenthesis can be eaten up by a comment.
+assertThrows('Function("/*", "*/){");', SyntaxError);
+
+// Test whether the function literal can be closed prematurely.
+assertThrows('Function("});(function(){");', SyntaxError);
+
+// Test whether block comments are handled correctly.
+assertDoesNotThrow('Function("/*", "*/", "/**/");');
+assertDoesNotThrow('Function("/*", "a", "*/", "/**/");');
+assertThrows('Function("a", "/*", "*/", "/**/");', SyntaxError);
+
+// Test whether line comments are handled correctly.
+assertDoesNotThrow('Function("//", "//")');
+assertDoesNotThrow('Function("//", "//", "//")');
+assertThrows('Function("a", "//", "//")', SyntaxError);