[v8-dev] Strict mode parameter validation. (issue6382006)

Thu, 20 Jan 2011 16:00:39 -0800 

Reviewers: Kevin Millikin, Lasse Reichstein, Mads Ager,

Description:
Strict mode parameter validation.
 * Duplicate parameter names
 * Parameters named "eval" or "arguments"


BUG=
TEST=test/mjsunit/strict-mode.js

Please review this at http://codereview.chromium.org/6382006/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/parser.cc
  M src/scopes.h
  M test/mjsunit/strict-mode.js


Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 2637281f080ed9f7f32fba93701c4da068a4f34e..89d73f52b6d6789aa0fc177281dac06b724c66b7 100644 
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3296,10 +3296,25 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name, 
     //    '(' (Identifier)*[','] ')'
     Expect(Token::LPAREN, CHECK_OK);
     int start_pos = scanner().location().beg_pos;
+ Scanner::Location name_loc(RelocInfo::kNoPosition, RelocInfo::kNoPosition); + Scanner::Location dupe_loc(RelocInfo::kNoPosition, RelocInfo::kNoPosition); 

     bool done = (peek() == Token::RPAREN);
     while (!done) {
       Handle<String> param_name = ParseIdentifier(CHECK_OK);
+
+      // Store locations for possible future error reports.
+      if (name_loc.beg_pos == RelocInfo::kNoPosition &&
+          IsEvalOrArguments(param_name)) {
+        // Store location for later
+        name_loc = scanner().location();
+      }
+      if (dupe_loc.beg_pos == RelocInfo::kNoPosition &&
+        top_scope_->IsDeclared(param_name)) {
+        // Store location for later
+        dupe_loc = scanner().location();
+      }
+
Variable* parameter = top_scope_->DeclareLocal(param_name, Variable::VAR); 
       top_scope_->AddParameter(parameter);
       num_parameters++;
@@ -3384,6 +3399,18 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name, 
         *ok = false;
         return NULL;
       }
+      if (name_loc.beg_pos != RelocInfo::kNoPosition) {
+        ReportMessageAt(name_loc, "strict_param_name",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
+      if (dupe_loc.beg_pos != RelocInfo::kNoPosition) {
+        ReportMessageAt(dupe_loc, "strict_param_dupe",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
       // TODO(mmaly): Check for octal escape sequence here.
     }

Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index 09901ade615237a771a34653a9922d08206ce788..a9220eb827566e94e7a126f1085bdc90d6c3a67c 100644 
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -289,6 +289,17 @@ class Scope: public ZoneObject {
   int ContextChainLength(Scope* scope);
// --------------------------------------------------------------------------- 
+  // Strict mode support.
+  bool IsDeclared(Handle<String> name) {
+    // During formal parameter list parsing the scope only contains
+    // two variables inserted at initialization: "this" and "arguments".
+ // "this" is an invalid parameter name and "arguments" is invalid parameter + // name in strict mode. Therefore looking up with the map which includes 
+    // "this" and "arguments" in addition to all formal parameters is safe.
+    return variables_.Lookup(name) != NULL;
+  }
+
+ // --------------------------------------------------------------------------- 
   // Debugging.

 #ifdef DEBUG
Index: test/mjsunit/strict-mode.js
diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js
index 924b34f936c274684e0e3cc2921b2563fb76aa05..68a0d7db847f26afa4b18e7401e0983ce18f3551 100644 
--- a/test/mjsunit/strict-mode.js
+++ b/test/mjsunit/strict-mode.js
@@ -76,19 +76,19 @@ CheckStrictMode("function eval() {}", SyntaxError)
 CheckStrictMode("function arguments() {}", SyntaxError)

 // Function parameter named 'eval'.
-//CheckStrictMode("function foo(a, b, eval, c, d) {}", SyntaxError)
+CheckStrictMode("function foo(a, b, eval, c, d) {}", SyntaxError)

 // Function parameter named 'arguments'.
-//CheckStrictMode("function foo(a, b, arguments, c, d) {}", SyntaxError)
+CheckStrictMode("function foo(a, b, arguments, c, d) {}", SyntaxError)

 // Property accessor parameter named 'eval'.
-//CheckStrictMode("var o = { set foo(eval) {} }", SyntaxError)
+CheckStrictMode("var o = { set foo(eval) {} }", SyntaxError)

 // Property accessor parameter named 'arguments'.
-//CheckStrictMode("var o = { set foo(arguments) {} }", SyntaxError)
+CheckStrictMode("var o = { set foo(arguments) {} }", SyntaxError)

 // Duplicate function parameter name.
-//CheckStrictMode("function foo(a, b, c, d, b) {}", SyntaxError)
+CheckStrictMode("function foo(a, b, c, d, b) {}", SyntaxError)

 // catch(eval)
 CheckStrictMode("try{}catch(eval){};", SyntaxError)
@@ -103,10 +103,10 @@ CheckStrictMode("var eval;", SyntaxError)
 CheckStrictMode("var arguments;", SyntaxError)

 // Strict mode applies to the function in which the directive is used..
-//assertThrows('\
-//function foo(eval) {\
-//  "use strict";\
-//}', SyntaxError);
+assertThrows('\
+function foo(eval) {\
+  "use strict";\
+}', SyntaxError);

 // Strict mode doesn't affect the outer stop of strict code.
 function NotStrict(eval) {


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to