From 4a909a7a62ebe9336366a8453351196e02d2aa5c Mon Sep 17 00:00:00 2001
From: "christian.plesner.hansen@gmail.com"
 <christian.plesner.hansen@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Fri, 24 Apr 2009 12:45:29 +0000
Subject: [PATCH] - When cloning maps to set the 'lazy loading' bit remember to
 clone   the properties as well.  This fixes some failing tests. - Moved json
 parsing into native code.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1789 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/ast.cc           | 21 +++++++++++++++++++++
 src/ast.h            |  7 +++++++
 src/bootstrapper.cc  |  4 ++--
 src/compiler.cc      | 37 +++++++++++++++++++++++++++++++++++--
 src/compiler.h       |  3 ++-
 src/debug-delay.js   |  2 +-
 src/factory.cc       |  6 +++---
 src/factory.h        |  2 +-
 src/handles.cc       |  2 +-
 src/json-delay.js    | 29 ++---------------------------
 src/objects.cc       | 18 +++++++++++-------
 src/objects.h        |  2 +-
 src/runtime.cc       | 21 +++++++++++++++------
 src/runtime.h        |  2 +-
 src/v8natives.js     |  6 +++---
 test/mjsunit/json.js |  8 +++++---
 16 files changed, 111 insertions(+), 59 deletions(-)

diff --git a/src/ast.cc b/src/ast.cc
index 1a6010a00..d19e3b3e0 100644
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -152,6 +152,27 @@ ObjectLiteral::Property::Property(bool is_getter, FunctionLiteral* value) {
 }
 
 
+bool ObjectLiteral::IsValidJSON() {
+  int length = properties()->length();
+  for (int i = 0; i < length; i++) {
+    Property* prop = properties()->at(i);
+    if (!prop->value()->IsValidJSON())
+      return false;
+  }
+  return true;
+}
+
+
+bool ArrayLiteral::IsValidJSON() {
+  int length = values()->length();
+  for (int i = 0; i < length; i++) {
+    if (!values()->at(i)->IsValidJSON())
+      return false;
+  }
+  return true;
+}
+
+
 void TargetCollector::AddTarget(BreakTarget* target) {
   // Add the label to the collector, but discard duplicates.
   int length = targets_->length();
diff --git a/src/ast.h b/src/ast.h
index b496816f7..6a2f67105 100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -155,6 +155,7 @@ class Expression: public Node {
  public:
   virtual Expression* AsExpression()  { return this; }
 
+  virtual bool IsValidJSON() { return false; }
   virtual bool IsValidLeftHandSide() { return false; }
 
   // Mark the expression as being compiled as an expression
@@ -625,6 +626,8 @@ class Literal: public Expression {
     return handle_.is_identical_to(other->handle_);
   }
 
+  virtual bool IsValidJSON() { return true; }
+
   // Identity testers.
   bool IsNull() const { return handle_.is_identical_to(Factory::null_value()); }
   bool IsTrue() const { return handle_.is_identical_to(Factory::true_value()); }
@@ -653,6 +656,8 @@ class MaterializedLiteral: public Expression {
   // constants and simple object and array literals.
   bool is_simple() const { return is_simple_; }
 
+  virtual bool IsValidJSON() { return true; }
+
   int depth() const { return depth_; }
 
  private:
@@ -704,6 +709,7 @@ class ObjectLiteral: public MaterializedLiteral {
 
   virtual ObjectLiteral* AsObjectLiteral() { return this; }
   virtual void Accept(AstVisitor* v);
+  virtual bool IsValidJSON();
 
   Handle<FixedArray> constant_properties() const {
     return constant_properties_;
@@ -751,6 +757,7 @@ class ArrayLiteral: public MaterializedLiteral {
 
   virtual void Accept(AstVisitor* v);
   virtual ArrayLiteral* AsArrayLiteral() { return this; }
+  virtual bool IsValidJSON();
 
   Handle<FixedArray> literals() const { return literals_; }
   ZoneList<Expression*>* values() const { return values_; }
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index 31d8c63ea..c43420761 100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -530,7 +530,7 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
     global_context()->function_instance_map()->set_prototype(*empty_function);
 
     // Allocate the function map first and then patch the prototype later
-    Handle<Map> empty_fm = Factory::CopyMap(fm);
+    Handle<Map> empty_fm = Factory::CopyMapDropDescriptors(fm);
     empty_fm->set_instance_descriptors(*function_map_descriptors);
     empty_fm->set_prototype(global_context()->object_function()->prototype());
     empty_function->set_map(*empty_fm);
@@ -1433,7 +1433,7 @@ void Genesis::MakeFunctionInstancePrototypeWritable() {
 
   Handle<DescriptorArray> function_map_descriptors =
       ComputeFunctionInstanceDescriptor(false, true);
-  Handle<Map> fm = Factory::CopyMap(Top::function_map());
+  Handle<Map> fm = Factory::CopyMapDropDescriptors(Top::function_map());
   fm->set_instance_descriptors(*function_map_descriptors);
   Top::context()->global_context()->set_function_map(*fm);
 }
diff --git a/src/compiler.cc b/src/compiler.cc
index 62e838e6c..c16b93888 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -80,8 +80,20 @@ static Handle<Code> MakeCode(FunctionLiteral* literal,
 }
 
 
+static bool IsValidJSON(FunctionLiteral* lit) {
+  if (!lit->body()->length() == 1)
+    return false;
+  Statement* stmt = lit->body()->at(0);
+  if (stmt->AsExpressionStatement() == NULL)
+    return false;
+  Expression *expr = stmt->AsExpressionStatement()->expression();
+  return expr->IsValidJSON();
+}
+
+
 static Handle<JSFunction> MakeFunction(bool is_global,
                                        bool is_eval,
+                                       bool is_json,
                                        Handle<Script> script,
                                        Handle<Context> context,
                                        v8::Extension* extension,
@@ -109,6 +121,19 @@ static Handle<JSFunction> MakeFunction(bool is_global,
     return Handle<JSFunction>::null();
   }
 
+  // When parsing JSON we do an ordinary parse and then afterwards
+  // check the AST to ensure it was well-formed.  If not we give a
+  // syntax error.
+  if (is_json && !IsValidJSON(lit)) {
+    HandleScope scope;
+    Handle<JSArray> args = Factory::NewJSArray(1);
+    Handle<Object> source(script->source());
+    SetElement(args, 0, source);
+    Handle<Object> result = Factory::NewSyntaxError("invalid_json", args);
+    Top::Throw(*result, NULL);
+    return Handle<JSFunction>::null();
+  }
+
   // Measure how long it takes to do the compilation; only take the
   // rest of the function into account to avoid overlap with the
   // parsing statistics.
@@ -214,6 +239,7 @@ Handle<JSFunction> Compiler::Compile(Handle<String> source,
 
     // Compile the function and add it to the cache.
     result = MakeFunction(true,
+                          false,
                           false,
                           script,
                           Handle<Context>::null(),
@@ -237,7 +263,8 @@ Handle<JSFunction> Compiler::Compile(Handle<String> source,
 Handle<JSFunction> Compiler::CompileEval(Handle<String> source,
                                          Handle<Context> context,
                                          int line_offset,
-                                         bool is_global) {
+                                         bool is_global,
+                                         bool is_json) {
   int source_length = source->length();
   Counters::total_eval_size.Increment(source_length);
   Counters::total_compile_size.Increment(source_length);
@@ -256,7 +283,13 @@ Handle<JSFunction> Compiler::CompileEval(Handle<String> source,
     // Create a script object describing the script to be compiled.
     Handle<Script> script = Factory::NewScript(source);
     script->set_line_offset(Smi::FromInt(line_offset));
-    result = MakeFunction(is_global, true, script, context, NULL, NULL);
+    result = MakeFunction(is_global,
+                          true,
+                          is_json,
+                          script,
+                          context,
+                          NULL,
+                          NULL);
     if (!result.is_null()) {
       CompilationCache::PutEval(source, context, entry, result);
     }
diff --git a/src/compiler.h b/src/compiler.h
index 4f589b7ee..8abe130d8 100644
--- a/src/compiler.h
+++ b/src/compiler.h
@@ -60,7 +60,8 @@ class Compiler : public AllStatic {
   static Handle<JSFunction> CompileEval(Handle<String> source,
                                         Handle<Context> context,
                                         int line_offset,
-                                        bool is_global);
+                                        bool is_global,
+                                        bool is_json);
 
   // Compile from function info (used for lazy compilation). Returns
   // true on success and false if the compilation resulted in a stack
diff --git a/src/debug-delay.js b/src/debug-delay.js
index 6cb5e7f57..961f304ed 100644
--- a/src/debug-delay.js
+++ b/src/debug-delay.js
@@ -1133,7 +1133,7 @@ DebugCommandProcessor.prototype.processDebugJSONRequest = function(json_request)
   try {
     try {
       // Convert the JSON string to an object.
-      request = %CompileString('(' + json_request + ')', 0)();
+      request = %CompileString('(' + json_request + ')', 0, false)();
 
       // Create an initial response.
       response = this.createResponse(request);
diff --git a/src/factory.cc b/src/factory.cc
index c849ab71e..c6b1d1715 100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -208,14 +208,14 @@ Handle<JSObject> Factory::NewFunctionPrototype(Handle<JSFunction> function) {
 }
 
 
-Handle<Map> Factory::CopyMap(Handle<Map> src) {
-  CALL_HEAP_FUNCTION(src->Copy(), Map);
+Handle<Map> Factory::CopyMapDropDescriptors(Handle<Map> src) {
+  CALL_HEAP_FUNCTION(src->CopyDropDescriptors(), Map);
 }
 
 
 Handle<Map> Factory::CopyMap(Handle<Map> src,
                              int extra_inobject_properties) {
-  Handle<Map> copy = CopyMap(src);
+  Handle<Map> copy = CopyMapDropDescriptors(src);
   // Check that we do not overflow the instance size when adding the
   // extra inobject properties.
   int instance_size_delta = extra_inobject_properties * kPointerSize;
diff --git a/src/factory.h b/src/factory.h
index 54f2089f7..6ac2706ec 100644
--- a/src/factory.h
+++ b/src/factory.h
@@ -153,7 +153,7 @@ class Factory : public AllStatic {
 
   static Handle<JSObject> NewFunctionPrototype(Handle<JSFunction> function);
 
-  static Handle<Map> CopyMap(Handle<Map> map);
+  static Handle<Map> CopyMapDropDescriptors(Handle<Map> map);
 
   // Copy the map adding more inobject properties if possible without
   // overflowing the instance size.
diff --git a/src/handles.cc b/src/handles.cc
index 6cd500613..773483d60 100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -700,7 +700,7 @@ void SetupLazy(Handle<JSObject> obj,
   arr->set(2, *function_context);  // Set function context to this
   arr->set(3, obj->map()->constructor());  // Remember the constructor
   Handle<Map> old_map(obj->map());
-  Handle<Map> new_map = Factory::CopyMap(old_map);
+  Handle<Map> new_map = Factory::CopyMapDropTransitions(old_map);
   obj->set_map(*new_map);
   new_map->set_needs_loading(true);
   // Store the lazy loading info in the constructor field.  We'll
diff --git a/src/json-delay.js b/src/json-delay.js
index 3640d7cd8..90150c6f1 100644
--- a/src/json-delay.js
+++ b/src/json-delay.js
@@ -27,35 +27,10 @@
 
 var $JSON = global.JSON;
 
-function IsValidJSON(s) {
-  // All empty whitespace is not valid.
-  if (/^\s*$/.test(s))
-    return false;
- 
-  // This is taken from http://www.json.org/json2.js which is released to the
-  // public domain.
-
-  var backslashesRe = /\\["\\\/bfnrtu]/g;
-  var simpleValuesRe =
-      /"[^"\\\n\r\x00-\x1f\x7f-\x9f]*"|true|false|null|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?/g;
-  var openBracketsRe = /(?:^|:|,)(?:[\s]*\[)+/g;
-  var remainderRe = /^[\],:{}\s]*$/;
-
-  return remainderRe.test(s.replace(backslashesRe, '@').
-      replace(simpleValuesRe, ']').
-      replace(openBracketsRe, ''));  
-}
-
 function ParseJSONUnfiltered(text) {
   var s = $String(text);
-  if (IsValidJSON(s)) {
-    try {
-      return global.eval('(' + s + ')');
-    } catch (e) {
-      // ignore exceptions
-    }
-  }
-  throw MakeSyntaxError('invalid_json', [s]);
+  var f = %CompileString("(" + text + ")", -1, true);
+  return f();
 }
 
 function Revive(holder, name, reviver) {
diff --git a/src/objects.cc b/src/objects.cc
index 9471ecdaf..55fc971ed 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1158,7 +1158,7 @@ Object* JSObject::AddFastProperty(String* name,
          (index - map()->inobject_properties()) < properties()->length() ||
          map()->unused_property_fields() == 0);
   // Allocate a new map for the object.
-  Object* r = map()->Copy();
+  Object* r = map()->CopyDropDescriptors();
   if (r->IsFailure()) return r;
   Map* new_map = Map::cast(r);
   if (allow_map_transition) {
@@ -1203,7 +1203,7 @@ Object* JSObject::AddConstantFunctionProperty(String* name,
   if (new_descriptors->IsFailure()) return new_descriptors;
 
   // Allocate a new map for the object.
-  Object* new_map = map()->Copy();
+  Object* new_map = map()->CopyDropDescriptors();
   if (new_map->IsFailure()) return new_map;
 
   DescriptorArray* descriptors = DescriptorArray::cast(new_descriptors);
@@ -1361,7 +1361,7 @@ Object* JSObject::ConvertDescriptorToField(String* name,
       DescriptorArray::cast(descriptors_unchecked);
 
   // Make a new map for the object.
-  Object* new_map_unchecked = map()->Copy();
+  Object* new_map_unchecked = map()->CopyDropDescriptors();
   if (new_map_unchecked->IsFailure()) return new_map_unchecked;
   Map* new_map = Map::cast(new_map_unchecked);
   new_map->set_instance_descriptors(new_descriptors);
@@ -2032,7 +2032,7 @@ Object* JSObject::NormalizeProperties(PropertyNormalizationMode mode) {
   dictionary->SetNextEnumerationIndex(index);
 
   // Allocate new map.
-  obj = map()->Copy();
+  obj = map()->CopyDropDescriptors();
   if (obj->IsFailure()) return obj;
   Map* new_map = Map::cast(obj);
 
@@ -2704,12 +2704,16 @@ Object* JSObject::SlowReverseLookup(Object* value) {
 }
 
 
-Object* Map::Copy() {
+Object* Map::CopyDropDescriptors() {
   Object* result = Heap::AllocateMap(instance_type(), instance_size());
   if (result->IsFailure()) return result;
   Map::cast(result)->set_prototype(prototype());
   Map::cast(result)->set_constructor(constructor());
   // Don't copy descriptors, so map transitions always remain a forest.
+  // If we retained the same descriptors we would have two maps
+  // pointing to the same transition which is bad because the garbage
+  // collector relies on being able to reverse pointers from transitions
+  // to maps.  If properties need to be retained use CopyDropTransitions.
   Map::cast(result)->set_instance_descriptors(Heap::empty_descriptor_array());
   // Please note instance_type and instance_size are set when allocated.
   Map::cast(result)->set_inobject_properties(inobject_properties());
@@ -2722,7 +2726,7 @@ Object* Map::Copy() {
 
 
 Object* Map::CopyDropTransitions() {
-  Object* new_map = Copy();
+  Object* new_map = CopyDropDescriptors();
   if (new_map->IsFailure()) return new_map;
   Object* descriptors = instance_descriptors()->RemoveTransitions();
   if (descriptors->IsFailure()) return descriptors;
@@ -7154,7 +7158,7 @@ Object* Dictionary::TransformPropertiesToFastFor(JSObject* obj,
 
   descriptors->Sort();
   // Allocate new map.
-  Object* new_map = obj->map()->Copy();
+  Object* new_map = obj->map()->CopyDropDescriptors();
   if (new_map->IsFailure()) return new_map;
 
   // Transform the object.
diff --git a/src/objects.h b/src/objects.h
index d17aeeae6..8e765917a 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2495,7 +2495,7 @@ class Map: public HeapObject {
   DECL_ACCESSORS(code_cache, FixedArray)
 
   // Returns a copy of the map.
-  Object* Copy();
+  Object* CopyDropDescriptors();
 
   // Returns a copy of the map, with all transitions dropped from the
   // instance descriptors.
diff --git a/src/runtime.cc b/src/runtime.cc
index 2b350a574..a1e23b4aa 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4824,14 +4824,18 @@ static Object* Runtime_GlobalReceiver(Arguments args) {
 
 static Object* Runtime_CompileString(Arguments args) {
   HandleScope scope;
-  ASSERT(args.length() == 2);
+  ASSERT_EQ(3, args.length());
   CONVERT_ARG_CHECKED(String, source, 0);
   CONVERT_ARG_CHECKED(Smi, line_offset, 1);
+  CONVERT_ARG_CHECKED(Oddball, is_json, 2)
 
   // Compile source string in the global context.
   Handle<Context> context(Top::context()->global_context());
-  Handle<JSFunction> boilerplate =
-      Compiler::CompileEval(source, context, line_offset->value(), true);
+  Handle<JSFunction> boilerplate = Compiler::CompileEval(source,
+                                                         context,
+                                                         line_offset->value(),
+                                                         true,
+                                                         is_json->IsTrue());
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> fun =
       Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -4856,7 +4860,7 @@ static Object* CompileDirectEval(Handle<String> source) {
 
   // Compile source string in the current context.
   Handle<JSFunction> boilerplate =
-      Compiler::CompileEval(source, context, 0, is_global);
+      Compiler::CompileEval(source, context, 0, is_global, false);
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> fun =
     Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -6479,7 +6483,8 @@ static Object* Runtime_DebugEvaluate(Arguments args) {
       Compiler::CompileEval(function_source,
                             context,
                             0,
-                            context->IsGlobalContext());
+                            context->IsGlobalContext(),
+                            false);
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> compiled_function =
       Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -6537,7 +6542,11 @@ static Object* Runtime_DebugEvaluateGlobal(Arguments args) {
 
   // Compile the source to be evaluated.
   Handle<JSFunction> boilerplate =
-      Handle<JSFunction>(Compiler::CompileEval(source, context, 0, true));
+      Handle<JSFunction>(Compiler::CompileEval(source,
+                                               context,
+                                               0,
+                                               true,
+                                               false));
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> compiled_function =
       Handle<JSFunction>(Factory::NewFunctionFromBoilerplate(boilerplate,
diff --git a/src/runtime.h b/src/runtime.h
index 6bd19f65c..b29ce70c6 100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -194,7 +194,7 @@ namespace v8 { namespace internal {
   F(NumberIsFinite, 1) \
   \
   /* Globals */ \
-  F(CompileString, 2) \
+  F(CompileString, 3) \
   F(GlobalPrint, 1) \
   \
   /* Eval */ \
diff --git a/src/v8natives.js b/src/v8natives.js
index 9ed655847..29a24b476 100644
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -110,7 +110,7 @@ function GlobalEval(x) {
                          'be the global object from which eval originated');
   }
   
-  var f = %CompileString(x, 0);
+  var f = %CompileString(x, 0, false);
   if (!IS_FUNCTION(f)) return f;
 
   return f.call(this);
@@ -121,7 +121,7 @@ function GlobalEval(x) {
 function GlobalExecScript(expr, lang) {
   // NOTE: We don't care about the character casing.
   if (!lang || /javascript/i.test(lang)) {
-    var f = %CompileString(ToString(expr), 0);
+    var f = %CompileString(ToString(expr), 0, false);
     f.call(%GlobalReceiver(global));
   }
   return null;
@@ -540,7 +540,7 @@ function NewFunction(arg1) {  // length == 1
 
   // The call to SetNewFunctionAttributes will ensure the prototype
   // property of the resulting function is enumerable (ECMA262, 15.3.5.2).
-  var f = %CompileString(source, -1)();
+  var f = %CompileString(source, -1, false)();
   %FunctionSetName(f, "anonymous");
   return %SetNewFunctionAttributes(f);
 }
diff --git a/test/mjsunit/json.js b/test/mjsunit/json.js
index ce7d0781f..47582643c 100644
--- a/test/mjsunit/json.js
+++ b/test/mjsunit/json.js
@@ -136,9 +136,11 @@ function TestInvalid(str) {
   assertThrows(function () { JSON.parse(str); }, SyntaxError);
 }
 
-TestInvalid('"abc\x00def"');
-TestInvalid('"abc\x10def"');
-TestInvalid('"abc\x1fdef"');
+TestInvalid('abcdef');
+TestInvalid('isNaN()');
+TestInvalid('{"x": [1, 2, deepObject]}');
+TestInvalid('[1, [2, [deepObject], 3], 4]');
+TestInvalid('function () { return 0; }');
 
 TestInvalid("[1, 2");
 TestInvalid('{"x": 3');
-- 
2.34.1