Strict mode object property validation.

Review URL: http://codereview.chromium.org/6335010/

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

src/messages.js

@@ -233,6 +233,9 @@ function FormatMessage(message) {
       strict_var_name:              "Variable name may not be eval or arguments in strict mode",
       strict_function_name:         "Function name may not be eval or arguments in strict mode",
       strict_octal_literal:         "Octal literals are not allowed in strict mode.",
+      strict_duplicate_property:    "Duplicate data property in object literal not allowed in strict mode",
+      accessor_data_property:       "Object literal may not have data and accessor property with the same name",
+      accessor_get_set:             "Object literal may not have multiple get/set accessors with the same name",
     };
   }
   var format = kMessages[message.type];

src/parser.cc

@@ -3016,6 +3016,126 @@ Handle<Object> Parser::GetBoilerplateValue(Expression* expression) {
   return Factory::undefined_value();
 }
 
+// Defined in ast.cc
+bool IsEqualString(void* first, void* second);
+bool IsEqualSmi(void* first, void* second);
+
+
+// Validation per 11.1.5 Object Initialiser
+class ObjectLiteralPropertyChecker {
+ public:
+  ObjectLiteralPropertyChecker(Parser* parser, bool strict) :
+    props(&IsEqualString),
+    elems(&IsEqualSmi),
+    parser_(parser),
+    strict_(strict) {
+  }
+
+  void CheckProperty(
+    ObjectLiteral::Property* property,
+    Scanner::Location loc,
+    bool* ok);
+
+ private:
+  enum PropertyKind {
+    kGetAccessor = 0x01,
+    kSetAccessor = 0x02,
+    kAccessor = kGetAccessor | kSetAccessor,
+    kData = 0x04
+  };
+
+  static intptr_t GetPropertyKind(ObjectLiteral::Property* property) {
+    switch (property->kind()) {
+      case ObjectLiteral::Property::GETTER:
+        return kGetAccessor;
+      case ObjectLiteral::Property::SETTER:
+        return kSetAccessor;
+      default:
+        return kData;
+    }
+  }
+
+  HashMap props;
+  HashMap elems;
+  Parser* parser_;
+  bool strict_;
+};
+
+
+void ObjectLiteralPropertyChecker::CheckProperty(
+    ObjectLiteral::Property* property,
+    Scanner::Location loc,
+    bool* ok) {
+
+  ASSERT(property != NULL);
+
+  Literal *lit = property->key();
+  Handle<Object> handle = lit->handle();
+
+  uint32_t hash;
+  HashMap* map;
+  void* key;
+  Smi* smi_key_location;
+
+  if (handle->IsSymbol()) {
+    Handle<String> name(String::cast(*handle));
+    if (name->AsArrayIndex(&hash)) {
+      smi_key_location = Smi::FromInt(hash);
+      key = &smi_key_location;
+      map = &elems;
+    } else {
+      key = handle.location();
+      hash = name->Hash();
+      map = &props;
+    }
+  } else if (handle->ToArrayIndex(&hash)) {
+    key = handle.location();
+    map = &elems;
+  } else {
+    ASSERT(handle->IsNumber());
+    double num = handle->Number();
+    char arr[100];
+    Vector<char> buffer(arr, ARRAY_SIZE(arr));
+    const char* str = DoubleToCString(num, buffer);
+    Handle<String> name = Factory::NewStringFromAscii(CStrVector(str));
+    key = name.location();
+    hash = name->Hash();
+    map = &props;
+  }
+
+  // Lookup property previously defined, if any.
+  HashMap::Entry* entry = map->Lookup(key, hash, true);
+  intptr_t prev = reinterpret_cast<intptr_t> (entry->value);
+  intptr_t curr = GetPropertyKind(property);
+
+  // Duplicate data properties are illegal in strict mode.
+  if (strict_ && (curr & prev & kData) != 0) {
+    parser_->ReportMessageAt(loc, "strict_duplicate_property",
+                             Vector<const char*>::empty());
+    *ok = false;
+    return;
+  }
+  // Data property conflicting with an accessor.
+  if (((curr & kData) && (prev & kAccessor)) ||
+      ((prev & kData) && (curr & kAccessor))) {
+    parser_->ReportMessageAt(loc, "accessor_data_property",
+                             Vector<const char*>::empty());
+    *ok = false;
+    return;
+  }
+  // Two accessors of the same type conflicting
+  if ((curr & prev & kAccessor) != 0) {
+    parser_->ReportMessageAt(loc, "accessor_get_set",
+                             Vector<const char*>::empty());
+    *ok = false;
+    return;
+  }
+
+  // Update map
+  entry->value = reinterpret_cast<void*> (prev | curr);
+  *ok = true;
+}
+
 
 void Parser::BuildObjectLiteralConstantProperties(
     ZoneList<ObjectLiteral::Property*>* properties,
@@ -3121,12 +3241,20 @@ Expression* Parser::ParseObjectLiteral(bool* ok) {
       new ZoneList<ObjectLiteral::Property*>(4);
   int number_of_boilerplate_properties = 0;
 
+  ObjectLiteralPropertyChecker checker(this, temp_scope_->StrictMode());
+
   Expect(Token::LBRACE, CHECK_OK);
+  Scanner::Location loc = scanner().location();
+
   while (peek() != Token::RBRACE) {
     if (fni_ != NULL) fni_->Enter();
 
     Literal* key = NULL;
     Token::Value next = peek();
+
+    // Location of the property name token
+    Scanner::Location loc = scanner().peek_location();
+
     switch (next) {
       case Token::IDENTIFIER: {
         bool is_getter = false;
@@ -3136,11 +3264,15 @@ Expression* Parser::ParseObjectLiteral(bool* ok) {
         if (fni_ != NULL) fni_->PushLiteralName(id);
 
         if ((is_getter || is_setter) && peek() != Token::COLON) {
+            // Update loc to point to the identifier
+            loc = scanner().peek_location();
             ObjectLiteral::Property* property =
                 ParseObjectLiteralGetSet(is_getter, CHECK_OK);
             if (IsBoilerplateProperty(property)) {
               number_of_boilerplate_properties++;
             }
+            // Validate the property.
+            checker.CheckProperty(property, loc, CHECK_OK);
             properties->Add(property);
             if (peek() != Token::RBRACE) Expect(Token::COMMA, CHECK_OK);
 
@@ -3197,6 +3329,8 @@ Expression* Parser::ParseObjectLiteral(bool* ok) {
 
     // Count CONSTANT or COMPUTED properties to maintain the enumeration order.
     if (IsBoilerplateProperty(property)) number_of_boilerplate_properties++;
+    // Validate the property
+    checker.CheckProperty(property, loc, CHECK_OK);
     properties->Add(property);
 
     // TODO(1240767): Consider allowing trailing comma.
@@ -3208,6 +3342,7 @@ Expression* Parser::ParseObjectLiteral(bool* ok) {
     }
   }
   Expect(Token::RBRACE, CHECK_OK);
+
   // Computation of literal_index must happen before pre parse bailout.
   int literal_index = temp_scope_->NextMaterializedLiteralIndex();
 

test/mjsunit/strict-mode.js

@@ -129,3 +129,52 @@ assertThrows('\
     "octal\\032directive";\
     "use strict";\
   }', SyntaxError);
+
+// Duplicate data properties.
+CheckStrictMode("var x = { dupe : 1, nondupe: 3, dupe : 2 };", SyntaxError)
+CheckStrictMode("var x = { '1234' : 1, '2345' : 2, '1234' : 3 };", SyntaxError)
+CheckStrictMode("var x = { '1234' : 1, '2345' : 2, 1234 : 3 };", SyntaxError)
+CheckStrictMode("var x = { 3.14 : 1, 2.71 : 2, 3.14 : 3 };", SyntaxError)
+CheckStrictMode("var x = { 3.14 : 1, '3.14' : 2 };", SyntaxError)
+CheckStrictMode("var x = { 123: 1, 123.00000000000000000000000000000000000000000000000000000000000000000001 : 2 }", SyntaxError)
+
+// Non-conflicting data properties.
+function StrictModeNonDuplicate() {
+  "use strict";
+  var x = { 123 : 1, "0123" : 2 };
+  var x = { 123: 1, '123.00000000000000000000000000000000000000000000000000000000000000000001' : 2 }
+}
+
+//CheckStrictMode("", SyntaxError)
+
+// Two getters (non-strict)
+assertThrows("var x = { get foo() { }, get foo() { } };", SyntaxError)
+assertThrows("var x = { get foo(){}, get 'foo'(){}};", SyntaxError)
+assertThrows("var x = { get 12(){}, get '12'(){}};", SyntaxError)
+
+// Two setters (non-strict)
+assertThrows("var x = { set foo(v) { }, set foo(v) { } };", SyntaxError)
+assertThrows("var x = { set foo(v) { }, set 'foo'(v) { } };", SyntaxError)
+assertThrows("var x = { set 13(v) { }, set '13'(v) { } };", SyntaxError)
+
+// Setter and data (non-strict)
+assertThrows("var x = { foo: 'data', set foo(v) { } };", SyntaxError)
+assertThrows("var x = { set foo(v) { }, foo: 'data' };", SyntaxError)
+assertThrows("var x = { foo: 'data', set 'foo'(v) { } };", SyntaxError)
+assertThrows("var x = { set foo(v) { }, 'foo': 'data' };", SyntaxError)
+assertThrows("var x = { 'foo': 'data', set foo(v) { } };", SyntaxError)
+assertThrows("var x = { set 'foo'(v) { }, foo: 'data' };", SyntaxError)
+assertThrows("var x = { 'foo': 'data', set 'foo'(v) { } };", SyntaxError)
+assertThrows("var x = { set 'foo'(v) { }, 'foo': 'data' };", SyntaxError)
+assertThrows("var x = { 12: 1, set '12'(v){}};", SyntaxError);
+assertThrows("var x = { 12: 1, set 12(v){}};", SyntaxError);
+assertThrows("var x = { '12': 1, set '12'(v){}};", SyntaxError);
+assertThrows("var x = { '12': 1, set 12(v){}};", SyntaxError);
+
+// Getter and data (non-strict)
+assertThrows("var x = { foo: 'data', get foo() { } };", SyntaxError)
+assertThrows("var x = { get foo() { }, foo: 'data' };", SyntaxError)
+assertThrows("var x = { 'foo': 'data', get foo() { } };", SyntaxError)
+assertThrows("var x = { get 'foo'() { }, 'foo': 'data' };", SyntaxError)
+assertThrows("var x = { '12': 1, get '12'(){}};", SyntaxError);
+assertThrows("var x = { '12': 1, get 12(){}};", SyntaxError);