Avoid callbacks to user code during error formatting in a couple of

other situations.

Do not use overwritten Object.prototype.hasOwnProperty and
Array.prototype.pop. Do not use split and join in the error formatting
implementation. They are too big to control and their generality is
not needed.

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

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

src/array.js

@@ -171,8 +171,9 @@ function Join(array, length, separator, convert) {
     }
     return %StringBuilderConcat(elements, length2, '');    
   } finally {
-    // Make sure to pop the visited array no matter what happens.
-    if (is_array) visited_arrays.pop();
+    // Make sure to remove the last element of the visited array no
+    // matter what happens.
+    if (is_array) visited_arrays.length = visited_arrays.length - 1;
   }
 }
 

src/messages.js

@@ -53,20 +53,22 @@ var kAddMessageAccessorsMarker = { };
 var kMessages = 0;
 
 var kReplacementMarkers =
-    [ "%0", "%1", "%2", "%3", "%4", "%5", "%6", "%7", "%8", "%9", "%10" ];
+    [ "%0", "%1", "%2", "%3" ]
 
 function FormatString(format, args) {
-  var result = format;
-  for (var i = 0; i < args.length; i++) {
-    var str;
-    try {
-      str = ToDetailString(args[i]);
-    } catch (e) {
-      str = "#<error>";
+  var result = "";
+  var arg_num = 0;
+  for (var i = 0; i < format.length; i++) {
+    var str = format[i];
+    for (arg_num = 0; arg_num < kReplacementMarkers.length; arg_num++) {
+      if (format[i] !== kReplacementMarkers[arg_num]) continue;
+      try {
+        str = ToDetailString(args[arg_num]);
+      } catch (e) {
+        str = "#<error>";
+      }
     }
-    var replacement_marker = kReplacementMarkers[i];
-    var split = %_CallFunction(result, replacement_marker, StringSplit);
-    result = %_CallFunction(split, str, ArrayJoin);
+    result += str;
   }
   return result;
 }
@@ -143,86 +145,86 @@ function FormatMessage(message) {
   if (kMessages === 0) {
     kMessages = {
       // Error
-      cyclic_proto:                 "Cyclic __proto__ value",
+      cyclic_proto:                 ["Cyclic __proto__ value"],
       // TypeError
-      unexpected_token:             "Unexpected token %0",
-      unexpected_token_number:      "Unexpected number",
-      unexpected_token_string:      "Unexpected string",
-      unexpected_token_identifier:  "Unexpected identifier",
-      unexpected_eos:               "Unexpected end of input",
-      malformed_regexp:             "Invalid regular expression: /%0/: %1",
-      unterminated_regexp:          "Invalid regular expression: missing /",
-      regexp_flags:                 "Cannot supply flags when constructing one RegExp from another",
-      incompatible_method_receiver: "Method %0 called on incompatible receiver %1",
-      invalid_lhs_in_assignment:    "Invalid left-hand side in assignment",
-      invalid_lhs_in_for_in:        "Invalid left-hand side in for-in",
-      invalid_lhs_in_postfix_op:    "Invalid left-hand side expression in postfix operation",
-      invalid_lhs_in_prefix_op:     "Invalid left-hand side expression in prefix operation",
-      multiple_defaults_in_switch:  "More than one default clause in switch statement",
-      newline_after_throw:          "Illegal newline after throw",
-      redeclaration:                "%0 '%1' has already been declared",
-      no_catch_or_finally:          "Missing catch or finally after try",
-      unknown_label:                "Undefined label '%0'",
-      uncaught_exception:           "Uncaught %0",
-      stack_trace:                  "Stack Trace:\n%0",
-      called_non_callable:          "%0 is not a function",
-      undefined_method:             "Object %1 has no method '%0'",
-      property_not_function:        "Property '%0' of object %1 is not a function",
-      cannot_convert_to_primitive:  "Cannot convert object to primitive value",
-      not_constructor:              "%0 is not a constructor",
-      not_defined:                  "%0 is not defined",
-      non_object_property_load:     "Cannot read property '%0' of %1",
-      non_object_property_store:    "Cannot set property '%0' of %1",
-      non_object_property_call:     "Cannot call method '%0' of %1",
-      with_expression:              "%0 has no properties",
-      illegal_invocation:           "Illegal invocation",
-      no_setter_in_callback:        "Cannot set property %0 of %1 which has only a getter",
-      apply_non_function:           "Function.prototype.apply was called on %0, which is a %1 and not a function",
-      apply_wrong_args:             "Function.prototype.apply: Arguments list has wrong type",
-      invalid_in_operator_use:      "Cannot use 'in' operator to search for '%0' in %1",
-      instanceof_function_expected: "Expecting a function in instanceof check, but got %0",
-      instanceof_nonobject_proto:   "Function has non-object prototype '%0' in instanceof check",
-      null_to_object:               "Cannot convert null to object",
-      reduce_no_initial:            "Reduce of empty array with no initial value",
-      getter_must_be_callable:      "Getter must be a function: %0",
-      setter_must_be_callable:      "Setter must be a function: %0",
-      value_and_accessor:           "Invalid property.  A property cannot both have accessors and be writable or have a value: %0",
-      proto_object_or_null:         "Object prototype may only be an Object or null",
-      property_desc_object:         "Property description must be an object: %0",
-      redefine_disallowed:          "Cannot redefine property: %0",
-      define_disallowed:            "Cannot define property, object is not extensible: %0",
+      unexpected_token:             ["Unexpected token ", "%0"],
+      unexpected_token_number:      ["Unexpected number"],
+      unexpected_token_string:      ["Unexpected string"],
+      unexpected_token_identifier:  ["Unexpected identifier"],
+      unexpected_eos:               ["Unexpected end of input"],
+      malformed_regexp:             ["Invalid regular expression: /", "%0", "/: ", "%1"],
+      unterminated_regexp:          ["Invalid regular expression: missing /"],
+      regexp_flags:                 ["Cannot supply flags when constructing one RegExp from another"],
+      incompatible_method_receiver: ["Method ", "%0", " called on incompatible receiver ", "%1"],
+      invalid_lhs_in_assignment:    ["Invalid left-hand side in assignment"],
+      invalid_lhs_in_for_in:        ["Invalid left-hand side in for-in"],
+      invalid_lhs_in_postfix_op:    ["Invalid left-hand side expression in postfix operation"],
+      invalid_lhs_in_prefix_op:     ["Invalid left-hand side expression in prefix operation"],
+      multiple_defaults_in_switch:  ["More than one default clause in switch statement"],
+      newline_after_throw:          ["Illegal newline after throw"],
+      redeclaration:                ["%0", " '", "%1", "' has already been declared"],
+      no_catch_or_finally:          ["Missing catch or finally after try"],
+      unknown_label:                ["Undefined label '", "%0", "'"],
+      uncaught_exception:           ["Uncaught ", "%0"],
+      stack_trace:                  ["Stack Trace:\n", "%0"],
+      called_non_callable:          ["%0", " is not a function"],
+      undefined_method:             ["Object ", "%1", " has no method '", "%0", "'"],
+      property_not_function:        ["Property '", "%0", "' of object ", "%1", " is not a function"],
+      cannot_convert_to_primitive:  ["Cannot convert object to primitive value"],
+      not_constructor:              ["%0", " is not a constructor"],
+      not_defined:                  ["%0", " is not defined"],
+      non_object_property_load:     ["Cannot read property '", "%0", "' of ", "%1"],
+      non_object_property_store:    ["Cannot set property '", "%0", "' of ", "%1"],
+      non_object_property_call:     ["Cannot call method '", "%0", "' of ", "%1"],
+      with_expression:              ["%0", " has no properties"],
+      illegal_invocation:           ["Illegal invocation"],
+      no_setter_in_callback:        ["Cannot set property ", "%0", " of ", "%1", " which has only a getter"],
+      apply_non_function:           ["Function.prototype.apply was called on ", "%0", ", which is a ", "%1", " and not a function"],
+      apply_wrong_args:             ["Function.prototype.apply: Arguments list has wrong type"],
+      invalid_in_operator_use:      ["Cannot use 'in' operator to search for '", "%0", "' in ", "%1"],
+      instanceof_function_expected: ["Expecting a function in instanceof check, but got ", "%0"],
+      instanceof_nonobject_proto:   ["Function has non-object prototype '", "%0", "' in instanceof check"],
+      null_to_object:               ["Cannot convert null to object"],
+      reduce_no_initial:            ["Reduce of empty array with no initial value"],
+      getter_must_be_callable:      ["Getter must be a function: ", "%0"],
+      setter_must_be_callable:      ["Setter must be a function: ", "%0"],
+      value_and_accessor:           ["Invalid property.  A property cannot both have accessors and be writable or have a value: ", "%0"],
+      proto_object_or_null:         ["Object prototype may only be an Object or null"],
+      property_desc_object:         ["Property description must be an object: ", "%0"],
+      redefine_disallowed:          ["Cannot redefine property: ", "%0"],
+      define_disallowed:            ["Cannot define property, object is not extensible: ", "%0"],
       // RangeError
-      invalid_array_length:         "Invalid array length",
-      stack_overflow:               "Maximum call stack size exceeded",
+      invalid_array_length:         ["Invalid array length"],
+      stack_overflow:               ["Maximum call stack size exceeded"],
       // SyntaxError
-      unable_to_parse:              "Parse error",
-      duplicate_regexp_flag:        "Duplicate RegExp flag %0",
-      invalid_regexp:               "Invalid RegExp pattern /%0/",
-      illegal_break:                "Illegal break statement",
-      illegal_continue:             "Illegal continue statement",
-      illegal_return:               "Illegal return statement",
-      error_loading_debugger:       "Error loading debugger",
-      no_input_to_regexp:           "No input to %0",
-      invalid_json:                 "String '%0' is not valid JSON",
-      circular_structure:           "Converting circular structure to JSON",
-      obj_ctor_property_non_object: "Object.%0 called on non-object",
-      array_indexof_not_defined:    "Array.getIndexOf: Argument undefined",
-      object_not_extensible:        "Can't add property %0, object is not extensible",
-      illegal_access:               "Illegal access",
-      invalid_preparser_data:       "Invalid preparser data for function %0",
-      strict_mode_with:             "Strict mode code may not include a with statement",
-      strict_catch_variable:        "Catch variable may not be eval or arguments in strict mode",
-      strict_param_name:            "Parameter name eval or arguments is not allowed in strict mode",
-      strict_param_dupe:            "Strict mode function may not have duplicate parameter names",
-      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",
-      strict_lhs_eval_assignment:   "Assignment to eval or arguments is not allowed in strict mode",
-      strict_lhs_postfix:           "Postfix increment/decrement may not have eval or arguments operand in strict mode",
-      strict_lhs_prefix:            "Prefix increment/decrement may not have eval or arguments operand in strict mode",
+      unable_to_parse:              ["Parse error"],
+      duplicate_regexp_flag:        ["Duplicate RegExp flag ", "%0"],
+      invalid_regexp:               ["Invalid RegExp pattern /", "%0", "/"],
+      illegal_break:                ["Illegal break statement"],
+      illegal_continue:             ["Illegal continue statement"],
+      illegal_return:               ["Illegal return statement"],
+      error_loading_debugger:       ["Error loading debugger"],
+      no_input_to_regexp:           ["No input to ", "%0"],
+      invalid_json:                 ["String '", "%0", "' is not valid JSON"],
+      circular_structure:           ["Converting circular structure to JSON"],
+      obj_ctor_property_non_object: ["Object.", "%0", " called on non-object"],
+      array_indexof_not_defined:    ["Array.getIndexOf: Argument undefined"],
+      object_not_extensible:        ["Can't add property ", "%0", ", object is not extensible"],
+      illegal_access:               ["Illegal access"],
+      invalid_preparser_data:       ["Invalid preparser data for function ", "%0"],
+      strict_mode_with:             ["Strict mode code may not include a with statement"],
+      strict_catch_variable:        ["Catch variable may not be eval or arguments in strict mode"],
+      strict_param_name:            ["Parameter name eval or arguments is not allowed in strict mode"],
+      strict_param_dupe:            ["Strict mode function may not have duplicate parameter names"],
+      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"],
+      strict_lhs_eval_assignment:   ["Assignment to eval or arguments is not allowed in strict mode"],
+      strict_lhs_postfix:           ["Postfix increment/decrement may not have eval or arguments operand in strict mode"],
+      strict_lhs_prefix:            ["Prefix increment/decrement may not have eval or arguments operand in strict mode"],
     };
   }
   var format = kMessages[message.type];
@@ -1038,14 +1040,16 @@ function errorToStringDetectCycle() {
   if (!%PushIfAbsent(visited_errors, this)) throw cyclic_error_marker;
   try {
     var type = this.type;
-    if (type && !this.hasOwnProperty("message")) {
+    if (type && !%_CallFunction(this, "message", ObjectHasOwnProperty)) {
       var formatted = FormatMessage({ type: type, args: this.arguments });
       return this.name + ": " + formatted;
     }
-    var message = this.hasOwnProperty("message") ? (": " + this.message) : "";
+    var message = %_CallFunction(this, "message", ObjectHasOwnProperty)
+        ? (": " + this.message)
+        : "";
     return this.name + message;
   } finally {
-    visited_errors.pop();
+    visited_errors.length = visited_errors.length - 1;
   }
 }
 

src/mirror-debugger.js

@@ -1084,9 +1084,9 @@ ErrorMirror.prototype.toText = function() {
   // Use the same text representation as in messages.js.
   var text;
   try {
-    str = builtins.ToDetailString(this.value_);
+    str = %_CallFunction(this.value_, builtins.errorToString);
   } catch (e) {
-    str = '#<an Error>';
+    str = '#<Error>';
   }
   return str;
 }

test/cctest/test-api.cc

@@ -2369,13 +2369,25 @@ static void check_reference_error_message(
 }
 
 
-// Test that overwritten toString methods are not invoked on uncaught
-// exception formatting. However, they are invoked when performing
-// normal error string conversions.
+static v8::Handle<Value> Fail(const v8::Arguments& args) {
+  ApiTestFuzzer::Fuzz();
+  CHECK(false);
+  return v8::Undefined();
+}
+
+
+// Test that overwritten methods are not invoked on uncaught exception
+// formatting. However, they are invoked when performing normal error
+// string conversions.
 TEST(APIThrowMessageOverwrittenToString) {
   v8::HandleScope scope;
   v8::V8::AddMessageListener(check_reference_error_message);
-  LocalContext context;
+  Local<ObjectTemplate> templ = ObjectTemplate::New();
+  templ->Set(v8_str("fail"), v8::FunctionTemplate::New(Fail));
+  LocalContext context(NULL, templ);
+  CompileRun("Array.prototype.pop = fail;");
+  CompileRun("Object.prototype.hasOwnProperty = fail;");
+  CompileRun("Object.prototype.toString = function f() { return 'Yikes'; }");
   CompileRun("Number.prototype.toString = function f() { return 'Yikes'; }");
   CompileRun("String.prototype.toString = function f() { return 'Yikes'; }");
   CompileRun("ReferenceError.prototype.toString ="

test/mjsunit/mirror-error.js

@@ -76,7 +76,7 @@ function testErrorMirror(e) {
     }
     assertTrue(found_message, 'Property message not found');
   }
-  
+
   // Check the formatted text (regress 1231579).
   assertEquals(fromJSON.text, e.toString(), 'toString');
 }