Early draft of strict mode

src/parser.cc

 // Copyright 2010 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
@@ skipping 543 matching lines @@
     *with_nesting_level_variable_ = prev_level_;
   }
 
  private:
   Scope** scope_variable_;
   int* with_nesting_level_variable_;
   Scope* prev_scope_;
   int prev_level_;
 };
 
+// ----------------------------------------------------------------------------
+// Support for strict mode parser stack. The constructor saves the current
+// state and the destructor resets it back.
+class StrictMode BASE_EMBEDDED {
+ public:
+  StrictMode(bool* strict_mode_variable)
+    : strict_mode_variable_(strict_mode_variable),
+      previous_strict_mode_(*strict_mode_variable) {
+  }
+
+  ~StrictMode() {
+    *strict_mode_variable_ = previous_strict_mode_;
+  }
+
+  bool *const strict_mode_variable_;
+  const bool previous_strict_mode_;
+};
 
 // ----------------------------------------------------------------------------
 // The CHECK_OK macro is a convenient macro to enforce error
 // handling for functions that may fail (by returning !*ok).
 //
 // CAUTION: This macro appends extra statements after a call,
 // thus it must never be used where only a single statement
 // is correct (e.g. an if statement branch w/o braces)!
 
 #define CHECK_OK  ok);   \
@@ skipping 19 matching lines @@
       script_(script),
       scanner_(),
       top_scope_(NULL),
       with_nesting_level_(0),
       temp_scope_(NULL),
       target_stack_(NULL),
       allow_natives_syntax_(allow_natives_syntax),
       extension_(extension),
       pre_data_(pre_data),
       fni_(NULL),
-      stack_overflow_(false) {
+      stack_overflow_(false),
+      strict_mode_(false) {
   AstNode::ResetIds();
 }
 
 
 FunctionLiteral* Parser::ParseProgram(Handle<String> source,
                                       bool in_global_context) {
   CompilationZoneScope zone_scope(DONT_DELETE_ON_EXIT);
 
   HistogramTimerScope timer(&Counters::parse);
   Counters::total_parse_size.Increment(source->length());
@@ skipping 47 matching lines @@
           top_scope_,
           body,
           temp_scope.materialized_literal_count(),
           temp_scope.expected_property_count(),
           temp_scope.only_simple_this_property_assignments(),
           temp_scope.this_property_assignments(),
           0,
           0,
           source->length(),
           false,
-          temp_scope.ContainsLoops());
+          temp_scope.ContainsLoops(),
+          strict_mode_);
     } else if (stack_overflow_) {
       Top::StackOverflow();
     }
   }
 
   // Make sure the target stack is empty.
   ASSERT(target_stack_ == NULL);
 
   // If there was a syntax error we have to get rid of the AST
   // and it is not safe to do so before the scope has been deleted.
@@ skipping 369 matching lines @@
 
   // Allocate a target stack to use for this set of source
   // elements. This way, all scripts and functions get their own
   // target stack thus avoiding illegal breaks and continues across
   // functions.
   TargetScope scope(&this->target_stack_);
 
   ASSERT(processor != NULL);
   InitializationBlockFinder block_finder;
   ThisNamedPropertyAssigmentFinder this_property_assignment_finder;
+  bool directive_prologue = true; // Parsing directive prologue.
+
   while (peek() != end_token) {
+    if (!strict_mode_ && directive_prologue) {
+      if (peek() == Token::STRING) {

[Lasse Reichstein, 2011/01/13 07:18:40]

As you pointed out, this is too simple (and ECMAScript is rarely simple :).
The missing parts is checking that the "use strict" is not part of a larger
expression, and that it doesn't contain escapes. There are ways to do both
without rescanning or prescanning.

If an ExpressionStatement *starts* with a string token, and is itself a single
string, then there are no wrapping parentheses.
And if the string literal's start and end position are exactly strlen("use
strict") + 2 apart (content + quotes), then there are no escapes in the string.

The solution I have come up with is something like (from memory):

Statement* stat;
if (directive_prologue && peek() == Token::STRING) {
  var location = scanner_pos();
  stat = ParseStatement(NULL, CHECK);
  ExpressionStatement* expr_stat = stat->AsExpressionStatement();
  if (expr_stat != NULL) {
    Handle<Object> literal = expr_stat->handle();
    if (literal->IsString()) {
      // It's a directive prologue.
      Handle<String> prologue = Handle<String>::cast(literal);
      if (prologue.Equals(Heap::use_strict_symbol()) &&
          position.end_pos - position.beg_pos == 12) {
        // Literal location, including quotes, leaves
        // no room for escaped characters.
        temp_scope_->SetStrict();
      }
      continue;  // Don't add it to the AST.
    }
  }
  directive_prologue = false;
} else {
  directive_prologue = false;
  stat = ParseStatement(NULL, CHECK);
}
  
I'm not entirely satisfied with the flow yet :).

[Lasse Reichstein, 2011/01/13 07:44:07]

Argh, location should be:
  Scanner::Location location = scanner().peek_location();
and the literal test should be:
  Literal* literal = expr_stat->AsLiteral();
  if (literal != NULL && literal->handle()->IsString()) {
    Handle<String> prologue = Handle<String>::cast(literal->handle());
    ...

[Martin Maly, 2011/01/14 00:06:28]

Done. However, we still need to add the directives to the AST. Consider:

eval("Hello").length

[Lasse Reichstein, 2011/01/14 11:42:05]

I don't see how that example applies. The "Hello" String is a string literal.
When parsed by eval, it will be an identifier. In neither case will it be seen
as a Directive Prologue.

[Martin Maly, 2011/01/14 16:30:43]

Sorry, the example was incorrect

eval("'hello'").length

is what is affected by dropping expression statements that in the directive
prologue.

On 2011/01/14 11:42:05, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/01/17 10:17:49]

Ack, yes, now I see.

+        // "use strict" is an ascii literal so no need to check non-ascii ones.
+        if (scanner().is_next_literal_ascii() &&
+            Factory::use_strict()->IsAsciiEqualTo(
+                scanner().next_literal_ascii_string())) {
+          strict_mode_ = true;
+        }
+      } else {
+        // done with directive prologue
+        directive_prologue = false;
+      }
+    }
+
     Statement* stat = ParseStatement(NULL, CHECK_OK);
     if (stat == NULL || stat->IsEmpty()) continue;
     // We find and mark the initialization blocks on top level code only.
     // This is because the optimization prevents reuse of the map transitions,
     // so it should be used only for code that will only be run once.
     if (top_scope_->is_global_scope()) {
       block_finder.Update(stat);
     }
     // Find and mark all assignments to named properties in this (this.x =)
     if (top_scope_->is_function_scope()) {
@@ skipping 335 matching lines @@
 Block* Parser::ParseVariableStatement(bool* ok) {
   // VariableStatement ::
   //   VariableDeclarations ';'
 
   Expression* dummy;  // to satisfy the ParseVariableDeclarations() signature
   Block* result = ParseVariableDeclarations(true, &dummy, CHECK_OK);
   ExpectSemicolon(CHECK_OK);
   return result;
 }
 
+static bool IsEvalOrArguments(Handle<String> string) {
+  return string.is_identical_to(Factory::eval_symbol()) ||
+         string.is_identical_to(Factory::arguments_symbol());
+}
 
 // If the variable declaration declares exactly one non-const
 // variable, then *var is set to that variable. In all other cases,
 // *var is untouched; in particular, it is the caller's responsibility
 // to initialize it properly. This mechanism is used for the parsing
 // of 'for-in' loops.
 Block* Parser::ParseVariableDeclarations(bool accept_IN,
                                          Expression** var,
                                          bool* ok) {
   // VariableDeclarations ::
@@ skipping 28 matching lines @@
   VariableProxy* last_var = NULL;  // the last variable declared
   int nvars = 0;  // the number of variables declared
   do {
     if (fni_ != NULL) fni_->Enter();
 
     // Parse variable name.
     if (nvars > 0) Consume(Token::COMMA);
     Handle<String> name = ParseIdentifier(CHECK_OK);
     if (fni_ != NULL) fni_->PushVariableName(name);
 
+    // Strict mode variables may not be named eval or arguments
+    if (strict_mode_ && IsEvalOrArguments(name)) {
+      ReportMessage("strict_var_name", Vector<const char*>::empty());
+      *ok = false;
+      return NULL;
+    }
+
     // Declare variable.
     // Note that we *always* must treat the initial value via a separate init
     // assignment for variables and constants because the value must be assigned
     // when the variable is encountered in the source. But the variable/constant
     // is declared (and set to 'undefined') upon entering the function within
     // which the variable or constant is declared. Only function variables have
     // an initial value in the declaration (because they are initialized upon
     // entering the function).
     //
     // If we have a const declaration, in an inner scope, the proxy is always
@@ skipping 332 matching lines @@
   }
   return result;
 }
 
 
 Statement* Parser::ParseWithStatement(ZoneStringList* labels, bool* ok) {
   // WithStatement ::
   //   'with' '(' Expression ')' Statement
 
   Expect(Token::WITH, CHECK_OK);
+
+  if (strict_mode_) {
+    ReportMessage("strict_mode_with", Vector<const char*>::empty());
+    *ok = false;
+    return NULL;
+  }
+
   Expect(Token::LPAREN, CHECK_OK);
   Expression* expr = ParseExpression(true, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
 
   return WithHelper(expr, labels, false, CHECK_OK);
 }
 
 
 CaseClause* Parser::ParseCaseClause(bool* default_seen_ptr, bool* ok) {
   // CaseClause ::
@@ skipping 112 matching lines @@
   // the jump targets.
   ZoneList<BreakTarget*>* catch_target_list = new ZoneList<BreakTarget*>(0);
   TargetCollector catch_collector(catch_target_list);
   bool has_catch = false;
   if (tok == Token::CATCH) {
     has_catch = true;
     Consume(Token::CATCH);
 
     Expect(Token::LPAREN, CHECK_OK);
     Handle<String> name = ParseIdentifier(CHECK_OK);
+
+    if (strict_mode_ && IsEvalOrArguments(name)) {
+      ReportMessage("strict_catch_variable", Vector<const char*>::empty());
+      *ok = false;
+      return NULL;
+    }
+
     Expect(Token::RPAREN, CHECK_OK);
 
     if (peek() == Token::LBRACE) {
       // Allocate a temporary for holding the finally state while
       // executing the finally block.
       catch_var = top_scope_->NewTemporary(Factory::catch_var_symbol());
       Literal* name_literal = new Literal(name);
       VariableProxy* catch_var_use = new VariableProxy(catch_var);
       Expression* obj = new CatchExtensionObject(name_literal, catch_var_use);
       { Target target(&this->target_stack_, &catch_collector);
@@ skipping 1222 matching lines @@
 
   int num_parameters = 0;
   // Parse function body.
   { Scope* scope =
         NewScope(top_scope_, Scope::FUNCTION_SCOPE, inside_with());
     LexicalScope lexical_scope(&this->top_scope_, &this->with_nesting_level_,
                                scope);
     TemporaryScope temp_scope(&this->temp_scope_);
     top_scope_->SetScopeName(name);
 
+    // Strict mode stack
+    StrictMode strict(&strict_mode_);

[Lasse Reichstein, 2011/01/13 07:18:40]

As stated earlier, we define the TemporaryScope just above, which has the same
liveness and general meaning (collecting stuff to put into FunctionLiteral) as
the StrictMode here.

[Martin Maly, 2011/01/14 00:06:28]

Done.

+
     //  FormalParameterList ::
     //    '(' (Identifier)*[','] ')'
     Expect(Token::LPAREN, CHECK_OK);
     int start_pos = scanner().location().beg_pos;
     bool done = (peek() == Token::RPAREN);
     while (!done) {
       Handle<String> param_name = ParseIdentifier(CHECK_OK);
       top_scope_->AddParameter(top_scope_->DeclareLocal(param_name,
                                                         Variable::VAR));
       num_parameters++;
@@ skipping 57 matching lines @@
       materialized_literal_count = temp_scope.materialized_literal_count();
       expected_property_count = temp_scope.expected_property_count();
       only_simple_this_property_assignments =
           temp_scope.only_simple_this_property_assignments();
       this_property_assignments = temp_scope.this_property_assignments();
 
       Expect(Token::RBRACE, CHECK_OK);
       end_pos = scanner().location().end_pos;
     }
 
+    // Validate strict mode.
+    if (strict_mode_) {
+      int position = function_token_position != RelocInfo::kNoPosition
+                       ? function_token_position
+                       : (start_pos > 0 ? start_pos - 1 : start_pos);
+
+      if (IsEvalOrArguments(name)) {
+        ReportMessageAt(Scanner::Location(position, start_pos),
+                        "strict_function_name", Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
+      if (top_scope_->HasEvalOrArgumentsParameter()) {
+        ReportMessageAt(Scanner::Location(position, start_pos),

[Lasse Reichstein, 2011/01/13 07:18:40]

Could you store the position of the argument that is "eval" or "arguments"?
I.e., so that if thestored position is RelocInfo::kNoPosition, then there is
none, and otherwise it's the position of the offending argument.

[Martin Maly, 2011/01/14 00:06:28]

It is definitely possible. I was trying to avoid any unnecessary work that would
affect non-strict mode code. In this case, checking parameter names for
eval/arguments will have to happen regardless of strict mode because of:

function foo(eval, arguments) { // Seen not in strict mode but must ERR
  "use strict";
}

This check is relatively cheap so I will add it.

On 2011/01/13 07:18:40, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/01/14 11:42:05]

Yes, we need to either detect the problems when they are introduced (to know the
location) before we know that it's strict code, or be able to detect them
properly later.

I prefer just detecting them as soon as possible, and just remember the first
(or last, I don't care) problem that would cause strict mode to fail.
And I think it's much simpler to simply detect strict mode violations when they
occur, and then check at the end whether there was any, if the function is
strict.

[Martin Maly, 2011/01/14 16:30:43]

Sounds good.

On 2011/01/14 11:42:05, Lasse Reichstein wrote:

+                        "strict_param_name", Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
+      if (top_scope_->HasDuplicateParameterName()) {
+        ReportMessageAt(Scanner::Location(position, start_pos),
+                        "strict_param_dupe", Vector<const char*>::empty());

[Lasse Reichstein, 2011/01/13 07:18:40]

Also here, could you store the actual position of the (second instance of) the
offending variable name?

[Martin Maly, 2011/01/14 00:06:28]

I believe it can be done, also at a relatively small cost to the non-strict mode
parsing.

The only glitch is being able to allocate HashMap temporarily so that only
functions with >N parameters use hashmap for duplicate checks.

How can I allocate such temporary hash map inside V8 parser? I am hoping to put
it onto TemporaryScope as well and allocate it only when needed, and ideally
free it as soon as the arguments have been processed.

On 2011/01/13 07:18:40, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/01/14 11:42:05]

I think I would just drop the HashMap and do the simple, quadratic algorithm in
all cases.
We are comparing symbols, so it's a pretty damn fast check, and I doubt we'll
see any function with more than 10 formal parameters anyway. I.e., let's worry
about that only if it becomes a problem.

[Martin Maly, 2011/01/14 16:30:43]

Done.

+        *ok = false;
+        return NULL;
+      }

[Lasse Reichstein, 2011/01/13 07:18:40]

For later, I would also put the check for octal numbers here.
I imagine letting the scanner hold the position of the most recent octal escape
or octal number literal, and then testing at this point whether it lies between
start_pos and end_pos.
That will report the last octal number, not the first, but I think it's better
than checking all the time.

[Martin Maly, 2011/01/14 00:06:28]

Great idea. that will also handle the case where "use strict" is preceded by
another directive that does contain octal literal. Will do when I implement the
octal detection.

On 2011/01/13 07:18:40, Lasse Reichstein wrote:

+    }
+
     FunctionLiteral* function_literal =
         new FunctionLiteral(name,
                             top_scope_,
                             body,
                             materialized_literal_count,
                             expected_property_count,
                             only_simple_this_property_assignments,
                             this_property_assignments,
                             num_parameters,
                             start_pos,
                             end_pos,
                             function_name->length() > 0,
-                            temp_scope.ContainsLoops());
+                            temp_scope.ContainsLoops(),
+                            strict_mode_);
     function_literal->set_function_token_position(function_token_position);
 
     if (fni_ != NULL && !is_named) fni_->AddFunction(function_literal);
     return function_literal;
   }
 }
 
 
 Expression* Parser::ParseV8Intrinsic(bool* ok) {
   // CallRuntime ::
@@ skipping 1404 matching lines @@
       Handle<String> source = Handle<String>(String::cast(script->source()));
       result = parser.ParseProgram(source, info->is_global());
     }
   }
 
   info->SetFunction(result);
   return (result != NULL);
 }
 
 } }  // namespace v8::internal