From d281688ae571bc2e5a302ee32a503be32c4ae720 Mon Sep 17 00:00:00 2001 From: yangguo Date: Mon, 17 Aug 2015 04:02:31 -0700 Subject: [PATCH] Do not use js builtins object when constructing an error. R=cbruni@chromium.org Review URL: https://codereview.chromium.org/1295093002 Cr-Commit-Position: refs/heads/master@{#30189} --- src/bootstrapper.cc | 1 + src/contexts.h | 1 + src/factory.cc | 129 +++---------------------------- src/factory.h | 50 ++++-------- src/messages.cc | 30 ++++--- src/messages.h | 2 + src/messages.js | 10 +-- src/pending-compilation-error-handler.cc | 4 +- test/mjsunit/error-constructors.js | 9 ++- 9 files changed, 56 insertions(+), 180 deletions(-) diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 506b638..8e95163 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -1763,6 +1763,7 @@ void Bootstrapper::ImportNatives(Isolate* isolate, Handle container) { INSTALL_NATIVE(JSFunction, "SyntaxError", syntax_error_function); INSTALL_NATIVE(JSFunction, "TypeError", type_error_function); INSTALL_NATIVE(JSFunction, "URIError", uri_error_function); + INSTALL_NATIVE(JSFunction, "MakeError", make_error_function); INSTALL_NATIVE(Symbol, "promiseStatus", promise_status); INSTALL_NATIVE(Symbol, "promiseValue", promise_value); diff --git a/src/contexts.h b/src/contexts.h index ecc8f4b..9271b06 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -159,6 +159,7 @@ enum BindingFlags { V(SYNTAX_ERROR_FUNCTION_INDEX, JSFunction, syntax_error_function) \ V(TYPE_ERROR_FUNCTION_INDEX, JSFunction, type_error_function) \ V(URI_ERROR_FUNCTION_INDEX, JSFunction, uri_error_function) \ + V(MAKE_ERROR_FUNCTION_INDEX, JSFunction, make_error_function) \ V(PROMISE_STATUS_INDEX, Symbol, promise_status) \ V(PROMISE_VALUE_INDEX, Symbol, promise_value) \ V(PROMISE_CREATE_INDEX, JSFunction, promise_create) \ diff --git a/src/factory.cc b/src/factory.cc index de47c84..8d01918 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -1103,35 +1103,31 @@ Handle Factory::NewBool8x16(bool lanes[16], PretenureFlag pretenure) { } -Handle Factory::NewError(const char* maker, +Handle Factory::NewError(Handle constructor, MessageTemplate::Template template_index, Handle arg0, Handle arg1, Handle arg2) { HandleScope scope(isolate()); - Handle error_maker = InternalizeUtf8String(maker); if (isolate()->bootstrapper()->IsActive()) { - // If this exception is being thrown during bootstrapping, - // js_builtins_object is unavailable. We return the error maker - // name's string as the exception since we have nothing better - // to do. - return scope.CloseAndEscape(error_maker); + // During bootstrapping we cannot construct error objects. + return scope.CloseAndEscape(NewStringFromAsciiChecked( + MessageTemplate::TemplateString(template_index))); } - Handle fun_obj = Object::GetProperty(isolate()->js_builtins_object(), - error_maker).ToHandleChecked(); - Handle fun = Handle::cast(fun_obj); + Handle fun = isolate()->make_error_function(); Handle message_type(Smi::FromInt(template_index), isolate()); if (arg0.is_null()) arg0 = undefined_value(); if (arg1.is_null()) arg1 = undefined_value(); if (arg2.is_null()) arg2 = undefined_value(); - Handle argv[] = {message_type, arg0, arg1, arg2}; + Handle argv[] = {constructor, message_type, arg0, arg1, arg2}; // Invoke the JavaScript factory method. If an exception is thrown while // running the factory method, use the exception as the result. Handle result; MaybeHandle exception; - if (!Execution::TryCall(fun, isolate()->js_builtins_object(), arraysize(argv), - argv, &exception).ToHandle(&result)) { + if (!Execution::TryCall(fun, undefined_value(), arraysize(argv), argv, + &exception) + .ToHandle(&result)) { Handle exception_obj; if (exception.ToHandle(&exception_obj)) { result = exception_obj; @@ -1143,113 +1139,6 @@ Handle Factory::NewError(const char* maker, } -Handle Factory::NewError(MessageTemplate::Template template_index, - Handle arg0, Handle arg1, - Handle arg2) { - return NewError("MakeError", template_index, arg0, arg1, arg2); -} - - -Handle Factory::NewTypeError(MessageTemplate::Template template_index, - Handle arg0, Handle arg1, - Handle arg2) { - return NewError("MakeTypeError", template_index, arg0, arg1, arg2); -} - - -Handle Factory::NewSyntaxError(MessageTemplate::Template template_index, - Handle arg0, Handle arg1, - Handle arg2) { - return NewError("MakeSyntaxError", template_index, arg0, arg1, arg2); -} - - -Handle Factory::NewReferenceError( - MessageTemplate::Template template_index, Handle arg0, - Handle arg1, Handle arg2) { - return NewError("MakeReferenceError", template_index, arg0, arg1, arg2); -} - - -Handle Factory::NewRangeError(MessageTemplate::Template template_index, - Handle arg0, Handle arg1, - Handle arg2) { - return NewError("MakeRangeError", template_index, arg0, arg1, arg2); -} - - -Handle Factory::NewEvalError(MessageTemplate::Template template_index, - Handle arg0, Handle arg1, - Handle arg2) { - return NewError("MakeEvalError", template_index, arg0, arg1, arg2); -} - - -Handle Factory::EmergencyNewError(const char* message, - Handle args) { - const int kBufferSize = 1000; - char buffer[kBufferSize]; - size_t space = kBufferSize; - char* p = &buffer[0]; - - Vector v(buffer, kBufferSize); - StrNCpy(v, message, space); - space -= Min(space, strlen(message)); - p = &buffer[kBufferSize] - space; - - for (int i = 0; i < Smi::cast(args->length())->value(); i++) { - if (space > 0) { - *p++ = ' '; - space--; - if (space > 0) { - Handle arg_str = Handle::cast( - Object::GetElement(isolate(), args, i).ToHandleChecked()); - base::SmartArrayPointer arg = arg_str->ToCString(); - Vector v2(p, static_cast(space)); - StrNCpy(v2, arg.get(), space); - space -= Min(space, strlen(arg.get())); - p = &buffer[kBufferSize] - space; - } - } - } - if (space > 0) { - *p = '\0'; - } else { - buffer[kBufferSize - 1] = '\0'; - } - return NewStringFromUtf8(CStrVector(buffer), TENURED).ToHandleChecked(); -} - - -Handle Factory::NewError(const char* maker, const char* message, - Handle args) { - Handle make_str = InternalizeUtf8String(maker); - Handle fun_obj = Object::GetProperty( - isolate()->js_builtins_object(), make_str).ToHandleChecked(); - // If the builtins haven't been properly configured yet this error - // constructor may not have been defined. Bail out. - if (!fun_obj->IsJSFunction()) { - return EmergencyNewError(message, args); - } - Handle fun = Handle::cast(fun_obj); - Handle message_obj = InternalizeUtf8String(message); - Handle argv[] = { message_obj, args }; - - // Invoke the JavaScript factory method. If an exception is thrown while - // running the factory method, use the exception as the result. - Handle result; - MaybeHandle exception; - if (!Execution::TryCall(fun, undefined_value(), arraysize(argv), argv, - &exception) - .ToHandle(&result)) { - Handle exception_obj; - if (exception.ToHandle(&exception_obj)) return exception_obj; - return undefined_value(); - } - return result; -} - - Handle Factory::NewError(Handle constructor, Handle message) { Handle argv[] = { message }; diff --git a/src/factory.h b/src/factory.h index 96fa74d..57f537e 100644 --- a/src/factory.h +++ b/src/factory.h @@ -546,11 +546,6 @@ class Factory final { Handle CopyCode(Handle code, Vector reloc_info); // Interface for creating error objects. - - Handle NewError(const char* maker, const char* message, - Handle args); - Handle EmergencyNewError(const char* message, Handle args); - Handle NewError(Handle constructor, Handle message); @@ -558,41 +553,28 @@ class Factory final { return NewRangeError(MessageTemplate::kInvalidStringLength); } - Handle NewError(const char* maker, + Handle NewError(Handle constructor, MessageTemplate::Template template_index, Handle arg0 = Handle(), Handle arg1 = Handle(), Handle arg2 = Handle()); - Handle NewError(MessageTemplate::Template template_index, - Handle arg0 = Handle(), - Handle arg1 = Handle(), - Handle arg2 = Handle()); +#define DEFINE_ERROR(NAME, name) \ + Handle New##NAME(MessageTemplate::Template template_index, \ + Handle arg0 = Handle(), \ + Handle arg1 = Handle(), \ + Handle arg2 = Handle()) { \ + return NewError(isolate()->name##_function(), template_index, arg0, arg1, \ + arg2); \ + } - Handle NewTypeError(MessageTemplate::Template template_index, - Handle arg0 = Handle(), - Handle arg1 = Handle(), - Handle arg2 = Handle()); - - Handle NewSyntaxError(MessageTemplate::Template template_index, - Handle arg0 = Handle(), - Handle arg1 = Handle(), - Handle arg2 = Handle()); - - Handle NewReferenceError(MessageTemplate::Template template_index, - Handle arg0 = Handle(), - Handle arg1 = Handle(), - Handle arg2 = Handle()); - - Handle NewRangeError(MessageTemplate::Template template_index, - Handle arg0 = Handle(), - Handle arg1 = Handle(), - Handle arg2 = Handle()); - - Handle NewEvalError(MessageTemplate::Template template_index, - Handle arg0 = Handle(), - Handle arg1 = Handle(), - Handle arg2 = Handle()); + DEFINE_ERROR(Error, error) + DEFINE_ERROR(EvalError, eval_error) + DEFINE_ERROR(RangeError, range_error) + DEFINE_ERROR(ReferenceError, reference_error) + DEFINE_ERROR(SyntaxError, syntax_error) + DEFINE_ERROR(TypeError, type_error) +#undef DEFINE_ERROR Handle NumberToString(Handle number, bool check_number_string_cache = true); diff --git a/src/messages.cc b/src/messages.cc index c7a4476..4e0c45f 100644 --- a/src/messages.cc +++ b/src/messages.cc @@ -322,23 +322,29 @@ Handle MessageTemplate::FormatMessage(Isolate* isolate, } -MaybeHandle MessageTemplate::FormatMessage(int template_index, - Handle arg0, - Handle arg1, - Handle arg2) { - Isolate* isolate = arg0->GetIsolate(); - const char* template_string; +const char* MessageTemplate::TemplateString(int template_index) { switch (template_index) { -#define CASE(NAME, STRING) \ - case k##NAME: \ - template_string = STRING; \ - break; +#define CASE(NAME, STRING) \ + case k##NAME: \ + return STRING; MESSAGE_TEMPLATES(CASE) #undef CASE case kLastMessage: default: - isolate->ThrowIllegalOperation(); - return MaybeHandle(); + return NULL; + } +} + + +MaybeHandle MessageTemplate::FormatMessage(int template_index, + Handle arg0, + Handle arg1, + Handle arg2) { + Isolate* isolate = arg0->GetIsolate(); + const char* template_string = TemplateString(template_index); + if (template_string == NULL) { + isolate->ThrowIllegalOperation(); + return MaybeHandle(); } IncrementalStringBuilder builder(isolate); diff --git a/src/messages.h b/src/messages.h index 94a3f01..d92e5f7 100644 --- a/src/messages.h +++ b/src/messages.h @@ -413,6 +413,8 @@ class MessageTemplate { kLastMessage }; + static const char* TemplateString(int template_index); + static MaybeHandle FormatMessage(int template_index, Handle arg0, Handle arg1, diff --git a/src/messages.js b/src/messages.js index 5601db6..712dba4 100644 --- a/src/messages.js +++ b/src/messages.js @@ -172,7 +172,6 @@ function ToDetailString(obj) { function MakeGenericError(constructor, type, arg0, arg1, arg2) { - if (IS_UNDEFINED(arg0) && IS_STRING(type)) arg0 = []; var error = new constructor(FormatMessage(type, arg0, arg1, arg2)); error[$internalErrorSymbol] = true; return error; @@ -1014,18 +1013,10 @@ MakeError = function(type, arg0, arg1, arg2) { return MakeGenericError(GlobalError, type, arg0, arg1, arg2); } -MakeEvalError = function(type, arg0, arg1, arg2) { - return MakeGenericError(GlobalEvalError, type, arg0, arg1, arg2); -} - MakeRangeError = function(type, arg0, arg1, arg2) { return MakeGenericError(GlobalRangeError, type, arg0, arg1, arg2); } -MakeReferenceError = function(type, arg0, arg1, arg2) { - return MakeGenericError(GlobalReferenceError, type, arg0, arg1, arg2); -} - MakeSyntaxError = function(type, arg0, arg1, arg2) { return MakeGenericError(GlobalSyntaxError, type, arg0, arg1, arg2); } @@ -1067,6 +1058,7 @@ utils.ExportToRuntime(function(to) { to.GetStackTraceLine = GetStackTraceLine; to.NoSideEffectToString = NoSideEffectToString; to.ToDetailString = ToDetailString; + to.MakeError = MakeGenericError; }); }); diff --git a/src/pending-compilation-error-handler.cc b/src/pending-compilation-error-handler.cc index afb347e..f1f9a20 100644 --- a/src/pending-compilation-error-handler.cc +++ b/src/pending-compilation-error-handler.cc @@ -31,10 +31,10 @@ void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate, Handle error; switch (error_type_) { case kReferenceError: - error = factory->NewError("MakeReferenceError", message_, argument); + error = factory->NewReferenceError(message_, argument); break; case kSyntaxError: - error = factory->NewError("MakeSyntaxError", message_, argument); + error = factory->NewSyntaxError(message_, argument); break; default: UNREACHABLE(); diff --git a/test/mjsunit/error-constructors.js b/test/mjsunit/error-constructors.js index b18a3fe..1ada39d 100644 --- a/test/mjsunit/error-constructors.js +++ b/test/mjsunit/error-constructors.js @@ -25,8 +25,6 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// Flags: --allow-natives-syntax - // Check that message and name are not enumerable on Error objects. var desc = Object.getOwnPropertyDescriptor(Error.prototype, 'name'); assertFalse(desc['enumerable']); @@ -62,7 +60,12 @@ var e = new ReferenceError('123'); assertTrue(e.hasOwnProperty('message')); assertTrue(e.hasOwnProperty('stack')); -var e = %MakeReferenceError("my_test_error", [0, 1]); +try { + eval("var error = reference"); +} catch (error) { + e = error; +} + assertTrue(e.hasOwnProperty('stack')); // Check that intercepting property access from toString is prevented for -- 2.7.4