From f5a2860534641c2a937ab7ce316f610c15bd1ee1 Mon Sep 17 00:00:00 2001
From: "mmaly@chromium.org"
 <mmaly@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Tue, 25 Jan 2011 17:21:45 +0000
Subject: [PATCH] Strict mode parameter validation.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6474 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/parser.cc               | 27 +++++++++++++++++++++++++--
 src/scanner-base.h          |  9 +++++++++
 src/scopes.h                | 11 +++++++++++
 test/mjsunit/strict-mode.js | 18 +++++++++---------
 4 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/src/parser.cc b/src/parser.cc
index a1ba9ca..de0391c 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3300,10 +3300,21 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
     //    '(' (Identifier)*[','] ')'
     Expect(Token::LPAREN, CHECK_OK);
     int start_pos = scanner().location().beg_pos;
+    Scanner::Location name_loc = Scanner::NoLocation();
+    Scanner::Location dupe_loc = Scanner::NoLocation();
 
     bool done = (peek() == Token::RPAREN);
     while (!done) {
       Handle<String> param_name = ParseIdentifier(CHECK_OK);
+
+      // Store locations for possible future error reports.
+      if (!name_loc.IsValid() && IsEvalOrArguments(param_name)) {
+        name_loc = scanner().location();
+      }
+      if (!dupe_loc.IsValid() && top_scope_->IsDeclared(param_name)) {
+        dupe_loc = scanner().location();
+      }
+
       Variable* parameter = top_scope_->DeclareLocal(param_name, Variable::VAR);
       top_scope_->AddParameter(parameter);
       num_parameters++;
@@ -3381,13 +3392,25 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
     if (temp_scope_->StrictMode()) {
       if (IsEvalOrArguments(name)) {
         int position = function_token_position != RelocInfo::kNoPosition
-                         ? function_token_position
-                         : (start_pos > 0 ? start_pos - 1 : start_pos);
+            ? function_token_position
+            : (start_pos > 0 ? start_pos - 1 : start_pos);
         ReportMessageAt(Scanner::Location(position, start_pos),
                         "strict_function_name", Vector<const char*>::empty());
         *ok = false;
         return NULL;
       }
+      if (name_loc.IsValid()) {
+        ReportMessageAt(name_loc, "strict_param_name",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
+      if (dupe_loc.IsValid()) {
+        ReportMessageAt(dupe_loc, "strict_param_dupe",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
       CheckOctalLiteral(start_pos, end_pos, CHECK_OK);
     }
 
diff --git a/src/scanner-base.h b/src/scanner-base.h
index a3e07d3..3d9800f 100644
--- a/src/scanner-base.h
+++ b/src/scanner-base.h
@@ -274,10 +274,19 @@ class Scanner {
   struct Location {
     Location(int b, int e) : beg_pos(b), end_pos(e) { }
     Location() : beg_pos(0), end_pos(0) { }
+
+    bool IsValid() const {
+      return beg_pos >= 0 && end_pos >= beg_pos;
+    }
+
     int beg_pos;
     int end_pos;
   };
 
+  static Location NoLocation() {
+    return Location(-1, -1);
+  }
+
   // Returns the location information for the current token
   // (the token returned by Next()).
   Location location() const { return current_.location; }
diff --git a/src/scopes.h b/src/scopes.h
index 09901ad..a9220eb 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
diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js
index 83a22cc..b065494 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) {
-- 
2.7.4