Revision: 2676
Author: [email protected]
Date: Thu Aug 13 04:16:12 2009
Log: Merge JSON fix to 1.2 branch.
Review URL: http://codereview.chromium.org/164470
http://code.google.com/p/v8/source/detail?r=2676

Modified:
  /branches/1.2/src/compiler.cc
  /branches/1.2/src/compiler.h
  /branches/1.2/src/runtime.cc
  /branches/1.2/src/version.cc
  /branches/1.2/test/mjsunit/json.js

=======================================
--- /branches/1.2/src/compiler.cc       Mon Jun 29 01:26:34 2009
+++ /branches/1.2/src/compiler.cc       Thu Aug 13 04:16:12 2009
@@ -85,7 +85,7 @@


  static bool IsValidJSON(FunctionLiteral* lit) {
-  if (!lit->body()->length() == 1)
+  if (lit->body()->length() != 1)
      return false;
    Statement* stmt = lit->body()->at(0);
    if (stmt->AsExpressionStatement() == NULL)
@@ -97,7 +97,7 @@

  static Handle<JSFunction> MakeFunction(bool is_global,
                                         bool is_eval,
-                                       bool is_json,
+                                       Compiler::ValidationState validate,
                                         Handle<Script> script,
                                         Handle<Context> context,
                                         v8::Extension* extension,
@@ -112,6 +112,7 @@
    script->set_context_data((*i::Top::global_context())->data());

  #ifdef ENABLE_DEBUGGER_SUPPORT
+  bool is_json = (validate == Compiler::VALIDATE_JSON);
    if (is_eval || is_json) {
      script->set_compilation_type(
          is_json ? Smi::FromInt(Script::COMPILATION_TYPE_JSON) :
@@ -145,7 +146,7 @@
    // 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)) {
+  if (validate == Compiler::VALIDATE_JSON && !IsValidJSON(lit)) {
      HandleScope scope;
      Handle<JSArray> args = Factory::NewJSArray(1);
      Handle<Object> source(script->source());
@@ -265,7 +266,7 @@
      // Compile the function and add it to the cache.
      result = MakeFunction(true,
                            false,
-                          false,
+                          DONT_VALIDATE_JSON,
                            script,
                            Handle<Context>::null(),
                            extension,
@@ -288,7 +289,11 @@
  Handle<JSFunction> Compiler::CompileEval(Handle<String> source,
                                           Handle<Context> context,
                                           bool is_global,
-                                         bool is_json) {
+                                         ValidationState validate) {
+  // Note that if validation is required then no path through this
+  // function is allowed to return a value without validating that
+  // the input is legal json.
+
    int source_length = source->length();
    Counters::total_eval_size.Increment(source_length);
    Counters::total_compile_size.Increment(source_length);
@@ -297,20 +302,26 @@
    VMState state(COMPILER);

    // Do a lookup in the compilation cache; if the entry is not there,
-  // invoke the compiler and add the result to the cache.
-  Handle<JSFunction> result =
-      CompilationCache::LookupEval(source, context, is_global);
+  // invoke the compiler and add the result to the cache.  If we're
+  // evaluating json we bypass the cache since we can't be sure a
+  // potential value in the cache has been validated.
+  Handle<JSFunction> result;
+  if (validate == DONT_VALIDATE_JSON)
+    result = CompilationCache::LookupEval(source, context, is_global);
+
    if (result.is_null()) {
      // Create a script object describing the script to be compiled.
      Handle<Script> script = Factory::NewScript(source);
      result = MakeFunction(is_global,
                            true,
-                          is_json,
+                          validate,
                            script,
                            context,
                            NULL,
                            NULL);
-    if (!result.is_null()) {
+    if (!result.is_null() && validate != VALIDATE_JSON) {
+      // For json it's unlikely that we'll ever see exactly the same
+      // string again so we don't use the compilation cache.
        CompilationCache::PutEval(source, context, is_global, result);
      }
    }
=======================================
--- /branches/1.2/src/compiler.h        Mon May 25 22:44:31 2009
+++ /branches/1.2/src/compiler.h        Thu Aug 13 04:16:12 2009
@@ -48,6 +48,8 @@

  class Compiler : public AllStatic {
   public:
+  enum ValidationState { VALIDATE_JSON, DONT_VALIDATE_JSON };
+
    // All routines return a JSFunction.
    // If an error occurs an exception is raised and
    // the return handle contains NULL.
@@ -63,7 +65,7 @@
    static Handle<JSFunction> CompileEval(Handle<String> source,
                                          Handle<Context> context,
                                          bool is_global,
-                                        bool is_json);
+                                        ValidationState validation);

    // Compile from function info (used for lazy compilation). Returns
    // true on success and false if the compilation resulted in a stack
=======================================
--- /branches/1.2/src/runtime.cc        Tue Jul 28 04:43:17 2009
+++ /branches/1.2/src/runtime.cc        Thu Aug 13 04:16:12 2009
@@ -4972,10 +4972,12 @@

    // Compile source string in the global context.
    Handle<Context> context(Top::context()->global_context());
+  Compiler::ValidationState validate = (is_json->IsTrue())
+    ? Compiler::VALIDATE_JSON : Compiler::DONT_VALIDATE_JSON;
    Handle<JSFunction> boilerplate = Compiler::CompileEval(source,
                                                           context,
                                                           true,
-                                                          
is_json->IsTrue());
+                                                         validate);
    if (boilerplate.is_null()) return Failure::Exception();
    Handle<JSFunction> fun =
        Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -4999,8 +5001,11 @@
    bool is_global = context->IsGlobalContext();

    // Compile source string in the current context.
-  Handle<JSFunction> boilerplate =
-      Compiler::CompileEval(source, context, is_global, false);
+  Handle<JSFunction> boilerplate = Compiler::CompileEval(
+      source,
+      context,
+      is_global,
+      Compiler::DONT_VALIDATE_JSON);
    if (boilerplate.is_null()) return Failure::Exception();
    Handle<JSFunction> fun =
      Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -7042,7 +7047,7 @@
        Compiler::CompileEval(function_source,
                              context,
                              context->IsGlobalContext(),
-                            false);
+                            Compiler::DONT_VALIDATE_JSON);
    if (boilerplate.is_null()) return Failure::Exception();
    Handle<JSFunction> compiled_function =
        Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -7110,7 +7115,7 @@
        Handle<JSFunction>(Compiler::CompileEval(source,
                                                 context,
                                                 true,
-                                               false));
+                                                
Compiler::DONT_VALIDATE_JSON));
    if (boilerplate.is_null()) return Failure::Exception();
    Handle<JSFunction> compiled_function =
        Handle<JSFunction>(Factory::NewFunctionFromBoilerplate(boilerplate,
=======================================
--- /branches/1.2/src/version.cc        Fri Aug  7 04:38:04 2009
+++ /branches/1.2/src/version.cc        Thu Aug 13 04:16:12 2009
@@ -35,7 +35,7 @@
  #define MAJOR_VERSION     1
  #define MINOR_VERSION     2
  #define BUILD_NUMBER      14
-#define PATCH_LEVEL       15
+#define PATCH_LEVEL       16
  #define CANDIDATE_VERSION false

  // Define SONAME to have the SCons build the put a specific SONAME into the
=======================================
--- /branches/1.2/test/mjsunit/json.js  Mon Apr 27 02:26:21 2009
+++ /branches/1.2/test/mjsunit/json.js  Thu Aug 13 04:16:12 2009
@@ -195,3 +195,13 @@

  assertEquals(undefined, JSON.stringify(undefined));
  assertEquals(undefined, JSON.stringify(function () { }));
+
+function checkIllegal(str) {
+  assertThrows(function () { JSON.parse(str); }, SyntaxError);
+}
+
+checkIllegal('1); throw "foo"; (1');
+
+var x = 0;
+eval("(1); x++; (1)");
+checkIllegal('1); x++; (1');

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

Reply via email to