From c76a97159fd6ef9bcd33f1c49eed72a647970bd0 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Tue, 18 Mar 2014 12:34:02 +0000 Subject: [PATCH] Handlify callers of Object::GetElement. R=ishell@chromium.org Review URL: https://codereview.chromium.org/200363002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20028 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/builtins.cc | 36 ++++++++++++++++++++++++++++-------- src/factory.cc | 5 +++-- src/isolate.h | 1 - src/json-stringifier.h | 4 ++-- src/liveedit.cc | 5 +++-- src/log.cc | 22 +++++++++------------- src/log.h | 2 +- src/objects-inl.h | 8 ++++++-- src/objects.cc | 47 +++++++++++++++++++++++++++++------------------ src/objects.h | 9 ++++----- src/runtime.cc | 40 ++++++++++++++++++++++++---------------- test/cctest/test-api.cc | 8 ++++---- test/cctest/test-heap.cc | 18 +++++++++++------- 13 files changed, 124 insertions(+), 81 deletions(-) diff --git a/src/builtins.cc b/src/builtins.cc index 4e5fbd3..27efaea 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -507,6 +507,17 @@ BUILTIN(ArrayPush) { } +static Handle ElementsAccessorSetLengthWrapper( + Isolate* isolate, + ElementsAccessor* accessor, + Handle array, + int new_length) { + CALL_HEAP_FUNCTION(isolate, + accessor->SetLength(*array, Smi::FromInt(new_length)), + Object); +} + + BUILTIN(ArrayPop) { Heap* heap = isolate->heap(); Object* receiver = *args.receiver(); @@ -524,17 +535,26 @@ BUILTIN(ArrayPop) { ElementsAccessor* accessor = array->GetElementsAccessor(); int new_length = len - 1; - MaybeObject* maybe_result; if (accessor->HasElement(array, array, new_length, elms_obj)) { - maybe_result = accessor->Get(array, array, new_length, elms_obj); + MaybeObject* maybe_result = + accessor->Get(array, array, new_length, elms_obj); + if (maybe_result->IsFailure()) return maybe_result; + MaybeObject* maybe_failure = + accessor->SetLength(array, Smi::FromInt(new_length)); + if (maybe_failure->IsFailure()) return maybe_failure; + return maybe_result; } else { - maybe_result = array->GetPrototype()->GetElement(isolate, len - 1); + // TODO(yangguo): handlify all once ElementsAccessors are handlified. + HandleScope scope(isolate); + Handle proto(array->GetPrototype(), isolate); + Handle element = Object::GetElement(isolate, proto, len - 1); + RETURN_IF_EMPTY_HANDLE(isolate, element); + Handle array_handle(array, isolate); + RETURN_IF_EMPTY_HANDLE(isolate, + ElementsAccessorSetLengthWrapper( + isolate, accessor, array_handle, new_length)); + return *element; } - if (maybe_result->IsFailure()) return maybe_result; - MaybeObject* maybe_failure = - accessor->SetLength(array, Smi::FromInt(new_length)); - if (maybe_failure->IsFailure()) return maybe_failure; - return maybe_result; } diff --git a/src/factory.cc b/src/factory.cc index 7e66a56..64567a8 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -1131,8 +1131,9 @@ Handle Factory::EmergencyNewError(const char* message, *p++ = ' '; space--; if (space > 0) { - MaybeObject* maybe_arg = args->GetElement(isolate(), i); - Handle arg_str(reinterpret_cast(maybe_arg)); + Handle arg_str = Handle::cast( + Object::GetElement(isolate(), args, i)); + CHECK_NOT_EMPTY_HANDLE(isolate(), arg_str); SmartArrayPointer arg = arg_str->ToCString(); Vector v2(p, static_cast(space)); OS::StrNCpy(v2, arg.get(), space); diff --git a/src/isolate.h b/src/isolate.h index 69e1e4c..dfab99d 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -146,7 +146,6 @@ typedef ZoneList > ZoneObjectList; do { \ ASSERT(!(isolate)->has_pending_exception()); \ CHECK(!(call).is_null()); \ - CHECK(!(isolate)->has_pending_exception()); \ } while (false) #define RETURN_IF_EMPTY_HANDLE(isolate, call) \ diff --git a/src/json-stringifier.h b/src/json-stringifier.h index 4510c4b..175b9a1 100644 --- a/src/json-stringifier.h +++ b/src/json-stringifier.h @@ -655,7 +655,7 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSObject( isolate_); } else { property = GetProperty(isolate_, object, key); - if (property.is_null()) return EXCEPTION; + RETURN_IF_EMPTY_HANDLE_VALUE(isolate_, property, EXCEPTION); } Result result = SerializeProperty(property, comma, key); if (!comma && result == SUCCESS) comma = true; @@ -687,7 +687,7 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSObject( property = GetProperty(isolate_, object, key_handle); } } - if (property.is_null()) return EXCEPTION; + RETURN_IF_EMPTY_HANDLE_VALUE(isolate_, property, EXCEPTION); Result result = SerializeProperty(property, comma, key_handle); if (!comma && result == SUCCESS) comma = true; if (result >= EXCEPTION) return result; diff --git a/src/liveedit.cc b/src/liveedit.cc index 91f8150..aa906b2 100644 --- a/src/liveedit.cc +++ b/src/liveedit.cc @@ -1949,8 +1949,9 @@ static const char* DropActivationsInActiveThread( // Replace "blocked on active" with "replaced on active" status. for (int i = 0; i < array_len; i++) { - if (result->GetElement(result->GetIsolate(), i) == - Smi::FromInt(LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) { + Handle obj = Object::GetElement(isolate, result, i); + CHECK_NOT_EMPTY_HANDLE(isolate, obj); + if (*obj == Smi::FromInt(LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) { Handle replaced( Smi::FromInt(LiveEdit::FUNCTION_REPLACED_ON_ACTIVE_STACK), isolate); SetElementSloppy(result, i, replaced); diff --git a/src/log.cc b/src/log.cc index 76fef5c..e01692e 100644 --- a/src/log.cc +++ b/src/log.cc @@ -1198,37 +1198,33 @@ void Logger::RegExpCompileEvent(Handle regexp, bool in_cache) { void Logger::LogRuntime(Vector format, - JSArray* args) { + Handle args) { if (!log_->IsEnabled() || !FLAG_log_runtime) return; - HandleScope scope(isolate_); Log::MessageBuilder msg(log_); for (int i = 0; i < format.length(); i++) { char c = format[i]; if (c == '%' && i <= format.length() - 2) { i++; ASSERT('0' <= format[i] && format[i] <= '9'); - MaybeObject* maybe = args->GetElement(isolate_, format[i] - '0'); - Object* obj; - if (!maybe->ToObject(&obj)) { - msg.Append(""); - continue; - } + Handle obj = Object::GetElement(isolate_, args, format[i] - '0'); + // No exception expected when getting an element from an array literal. + CHECK_NOT_EMPTY_HANDLE(isolate_, obj); i++; switch (format[i]) { case 's': - msg.AppendDetailed(String::cast(obj), false); + msg.AppendDetailed(String::cast(*obj), false); break; case 'S': - msg.AppendDetailed(String::cast(obj), true); + msg.AppendDetailed(String::cast(*obj), true); break; case 'r': - Logger::LogRegExpSource(Handle(JSRegExp::cast(obj))); + Logger::LogRegExpSource(Handle::cast(obj)); break; case 'x': - msg.Append("0x%x", Smi::cast(obj)->value()); + msg.Append("0x%x", Smi::cast(*obj)->value()); break; case 'i': - msg.Append("%i", Smi::cast(obj)->value()); + msg.Append("%i", Smi::cast(*obj)->value()); break; default: UNREACHABLE(); diff --git a/src/log.h b/src/log.h index 09b6b77..c01aca2 100644 --- a/src/log.h +++ b/src/log.h @@ -349,7 +349,7 @@ class Logger { void RegExpCompileEvent(Handle regexp, bool in_cache); // Log an event reported from generated code - void LogRuntime(Vector format, JSArray* args); + void LogRuntime(Vector format, Handle args); bool is_logging() { return is_logging_; diff --git a/src/objects-inl.h b/src/objects-inl.h index 75ff839..8b4baf3 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1049,12 +1049,16 @@ bool Object::HasSpecificClassOf(String* name) { } -MaybeObject* Object::GetElement(Isolate* isolate, uint32_t index) { +Handle Object::GetElement(Isolate* isolate, + Handle object, + uint32_t index) { // GetElement can trigger a getter which can cause allocation. // This was not always the case. This ASSERT is here to catch // leftover incorrect uses. ASSERT(AllowHeapAllocation::IsAllowed()); - return GetElementWithReceiver(isolate, this, index); + CALL_HEAP_FUNCTION(isolate, + object->GetElementWithReceiver(isolate, *object, index), + Object); } diff --git a/src/objects.cc b/src/objects.cc index f656391..cb8489f 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -493,19 +493,11 @@ Handle Object::GetProperty(Handle object, // method (or somewhere else entirely). Needs more global clean-up. uint32_t index; Isolate* isolate = name->GetIsolate(); - if (name->AsArrayIndex(&index)) - return GetElement(isolate, object, index); + if (name->AsArrayIndex(&index)) return GetElement(isolate, object, index); CALL_HEAP_FUNCTION(isolate, object->GetProperty(*name), Object); } -Handle Object::GetElement(Isolate* isolate, - Handle object, - uint32_t index) { - CALL_HEAP_FUNCTION(isolate, object->GetElement(isolate, index), Object); -} - - MaybeObject* JSProxy::GetElementWithHandler(Object* receiver, uint32_t index) { String* name; @@ -4085,6 +4077,7 @@ Handle JSObject::SetPropertyForResult(Handle object, *name != isolate->heap()->hidden_string(); if (is_observed && lookup->IsDataProperty()) { old_value = Object::GetProperty(object, name); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); } // This is a real property that is not read-only, or it is a @@ -4130,6 +4123,7 @@ Handle JSObject::SetPropertyForResult(Handle object, object->LocalLookup(*name, &new_lookup, true); if (new_lookup.IsDataProperty()) { Handle new_value = Object::GetProperty(object, name); + CHECK_NOT_EMPTY_HANDLE(isolate, new_value); if (!new_value->SameValue(*old_value)) { EnqueueChangeRecord(object, "update", name, old_value); } @@ -4206,8 +4200,10 @@ Handle JSObject::SetLocalPropertyIgnoreAttributes( bool is_observed = object->map()->is_observed() && *name != isolate->heap()->hidden_string(); if (is_observed && lookup.IsProperty()) { - if (lookup.IsDataProperty()) old_value = - Object::GetProperty(object, name); + if (lookup.IsDataProperty()) { + old_value = Object::GetProperty(object, name); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); + } old_attributes = lookup.GetAttributes(); } @@ -4252,6 +4248,7 @@ Handle JSObject::SetLocalPropertyIgnoreAttributes( bool value_changed = false; if (new_lookup.IsDataProperty()) { Handle new_value = Object::GetProperty(object, name); + CHECK_NOT_EMPTY_HANDLE(isolate, new_value); value_changed = !old_value->SameValue(*new_value); } if (new_lookup.GetAttributes() != old_attributes) { @@ -5206,9 +5203,12 @@ Handle JSObject::DeleteElement(Handle object, if (object->map()->is_observed()) { should_enqueue_change_record = HasLocalElement(object, index); if (should_enqueue_change_record) { - old_value = object->GetLocalElementAccessorPair(index) != NULL - ? Handle::cast(factory->the_hole_value()) - : Object::GetElement(isolate, object, index); + if (object->GetLocalElementAccessorPair(index) != NULL) { + old_value = Handle::cast(factory->the_hole_value()); + } else { + old_value = Object::GetElement(isolate, object, index); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); + } } } @@ -5278,6 +5278,7 @@ Handle JSObject::DeleteProperty(Handle object, *name != isolate->heap()->hidden_string(); if (is_observed && lookup.IsDataProperty()) { old_value = Object::GetProperty(object, name); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); } Handle result; @@ -6376,6 +6377,7 @@ void JSObject::DefineAccessor(Handle object, preexists = HasLocalElement(object, index); if (preexists && object->GetLocalElementAccessorPair(index) == NULL) { old_value = Object::GetElement(isolate, object, index); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); } } else { LookupResult lookup(isolate); @@ -6383,6 +6385,7 @@ void JSObject::DefineAccessor(Handle object, preexists = lookup.IsProperty(); if (preexists && lookup.IsDataProperty()) { old_value = Object::GetProperty(object, name); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); } } } @@ -11374,9 +11377,14 @@ static bool GetOldValue(Isolate* isolate, JSReceiver::GetLocalElementAttribute(object, index); ASSERT(attributes != ABSENT); if (attributes == DONT_DELETE) return false; - old_values->Add(object->GetLocalElementAccessorPair(index) == NULL - ? Object::GetElement(isolate, object, index) - : Handle::cast(isolate->factory()->the_hole_value())); + Handle value; + if (object->GetLocalElementAccessorPair(index) != NULL) { + value = Handle::cast(isolate->factory()->the_hole_value()); + } else { + value = Object::GetElement(isolate, object, index); + CHECK_NOT_EMPTY_HANDLE(isolate, value); + } + old_values->Add(value); indices->Add(index); return true; } @@ -12606,8 +12614,10 @@ Handle JSObject::SetElement(Handle object, Handle new_length_handle; if (old_attributes != ABSENT) { - if (object->GetLocalElementAccessorPair(index) == NULL) + if (object->GetLocalElementAccessorPair(index) == NULL) { old_value = Object::GetElement(isolate, object, index); + CHECK_NOT_EMPTY_HANDLE(isolate, old_value); + } } else if (object->IsJSArray()) { // Store old array length in case adding an element grows the array. old_length_handle = handle(Handle::cast(object)->length(), @@ -12653,6 +12663,7 @@ Handle JSObject::SetElement(Handle object, EnqueueChangeRecord(object, "reconfigure", name, old_value); } else { Handle new_value = Object::GetElement(isolate, object, index); + CHECK_NOT_EMPTY_HANDLE(isolate, new_value); bool value_changed = !old_value->SameValue(*new_value); if (old_attributes != new_attributes) { if (!value_changed) old_value = isolate->factory()->the_hole_value(); diff --git a/src/objects.h b/src/objects.h index 2bec00d..f6ce77b 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1577,11 +1577,10 @@ class Object : public MaybeObject { MUST_USE_RESULT MaybeObject* GetPropertyWithDefinedGetter(Object* receiver, JSReceiver* getter); - static Handle GetElement(Isolate* isolate, - Handle object, - uint32_t index); - MUST_USE_RESULT inline MaybeObject* GetElement(Isolate* isolate, - uint32_t index); + static inline Handle GetElement(Isolate* isolate, + Handle object, + uint32_t index); + // For use when we know that no exception can be thrown. inline Object* GetElementNoExceptionThrown(Isolate* isolate, uint32_t index); MUST_USE_RESULT MaybeObject* GetElementWithReceiver(Isolate* isolate, diff --git a/src/runtime.cc b/src/runtime.cc index 6c5ec1c..916c993 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -4841,11 +4841,15 @@ MaybeObject* Runtime::GetElementOrCharAt(Isolate* isolate, if (!result->IsUndefined()) return *result; } + Handle result; if (object->IsString() || object->IsNumber() || object->IsBoolean()) { - return object->GetPrototype(isolate)->GetElement(isolate, index); + Handle proto(object->GetPrototype(isolate), isolate); + result = Object::GetElement(isolate, proto, index); + } else { + result = Object::GetElement(isolate, object, index); } - - return object->GetElement(isolate, index); + RETURN_IF_EMPTY_HANDLE(isolate, result); + return *result; } @@ -6023,7 +6027,11 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetArgumentsProperty) { if (index < n) { return frame->GetParameter(index); } else { - return isolate->initial_object_prototype()->GetElement(isolate, index); + Handle initial_prototype(isolate->initial_object_prototype()); + Handle result = + Object::GetElement(isolate, initial_prototype, index); + RETURN_IF_EMPTY_HANDLE(isolate, result); + return *result; } } @@ -8867,6 +8875,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_Apply) { for (int i = 0; i < argc; ++i) { argv[i] = Object::GetElement(isolate, arguments, offset + i); + RETURN_IF_EMPTY_HANDLE(isolate, argv[i]); } bool threw; @@ -13730,14 +13739,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetLanguageTagVariants) { Handle base = isolate->factory()->NewStringFromAscii(CStrVector("base")); for (unsigned int i = 0; i < length; ++i) { - MaybeObject* maybe_string = input->GetElement(isolate, i); - Object* locale_id; - if (!maybe_string->ToObject(&locale_id) || !locale_id->IsString()) { + Handle locale_id = Object::GetElement(isolate, input, i); + RETURN_IF_EMPTY_HANDLE(isolate, locale_id); + if (!locale_id->IsString()) { return isolate->Throw(isolate->heap()->illegal_argument_string()); } v8::String::Utf8Value utf8_locale_id( - v8::Utils::ToLocal(Handle(String::cast(locale_id)))); + v8::Utils::ToLocal(Handle::cast(locale_id))); UErrorCode error = U_ZERO_ERROR; @@ -14594,15 +14603,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ListNatives) { RUNTIME_FUNCTION(MaybeObject*, Runtime_Log) { - SealHandleScope shs(isolate); + HandleScope handle_scope(isolate); ASSERT(args.length() == 2); - CONVERT_ARG_CHECKED(String, format, 0); - CONVERT_ARG_CHECKED(JSArray, elms, 1); - DisallowHeapAllocation no_gc; - String::FlatContent format_content = format->GetFlatContent(); - RUNTIME_ASSERT(format_content.IsAscii()); - Vector chars = format_content.ToOneByteVector(); - isolate->logger()->LogRuntime(Vector::cast(chars), elms); + CONVERT_ARG_HANDLE_CHECKED(String, format, 0); + CONVERT_ARG_HANDLE_CHECKED(JSArray, elms, 1); + + SmartArrayPointer format_chars = format->ToCString(); + isolate->logger()->LogRuntime( + Vector(format_chars.get(), format->length()), elms); return isolate->heap()->undefined_value(); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 38b41d9..de73da5 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -15687,7 +15687,7 @@ static void CheckElementValue(i::Isolate* isolate, int expected, i::Handle obj, int offset) { - i::Object* element = obj->GetElement(isolate, offset)->ToObjectChecked(); + i::Object* element = *i::Object::GetElement(isolate, obj, offset); CHECK_EQ(expected, i::Smi::cast(element)->value()); } @@ -16321,7 +16321,7 @@ static void ObjectWithExternalArrayTestHelper( array_type == v8::kExternalFloat32Array) { CHECK_EQ(static_cast(i::OS::nan_value()), static_cast( - jsobj->GetElement(isolate, 7)->ToObjectChecked()->Number())); + i::Object::GetElement(isolate, jsobj, 7)->Number())); } else { CheckElementValue(isolate, 0, jsobj, 7); } @@ -16333,7 +16333,7 @@ static void ObjectWithExternalArrayTestHelper( CHECK_EQ(2, result->Int32Value()); CHECK_EQ(2, static_cast( - jsobj->GetElement(isolate, 6)->ToObjectChecked()->Number())); + i::Object::GetElement(isolate, jsobj, 6)->Number())); if (array_type != v8::kExternalFloat32Array && array_type != v8::kExternalFloat64Array) { @@ -16613,7 +16613,7 @@ static void ExternalArrayTestHelper(v8::ExternalArrayType array_type, kElementCount); CHECK_EQ(1, static_cast( - jsobj->GetElement(isolate, 1)->ToObjectChecked()->Number())); + i::Object::GetElement(isolate, jsobj, 1)->Number())); ObjectWithExternalArrayTestHelper( context.local(), obj, kElementCount, array_type, low, high); diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index 982977a..376c735 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -767,7 +767,7 @@ TEST(JSArray) { // array[length] = name. JSReceiver::SetElement(array, 0, name, NONE, SLOPPY); CHECK_EQ(Smi::FromInt(1), array->length()); - CHECK_EQ(array->GetElement(isolate, 0), *name); + CHECK_EQ(*i::Object::GetElement(isolate, array, 0), *name); // Set array length with larger than smi value. Handle length = @@ -784,8 +784,8 @@ TEST(JSArray) { uint32_t new_int_length = 0; CHECK(array->length()->ToArrayIndex(&new_int_length)); CHECK_EQ(static_cast(int_length), new_int_length - 1); - CHECK_EQ(array->GetElement(isolate, int_length), *name); - CHECK_EQ(array->GetElement(isolate, 0), *name); + CHECK_EQ(*i::Object::GetElement(isolate, array, int_length), *name); + CHECK_EQ(*i::Object::GetElement(isolate, array, 0), *name); } @@ -817,8 +817,10 @@ TEST(JSObjectCopy) { Handle clone = JSObject::Copy(obj); CHECK(!clone.is_identical_to(obj)); - CHECK_EQ(obj->GetElement(isolate, 0), clone->GetElement(isolate, 0)); - CHECK_EQ(obj->GetElement(isolate, 1), clone->GetElement(isolate, 1)); + CHECK_EQ(*i::Object::GetElement(isolate, obj, 0), + *i::Object::GetElement(isolate, clone, 0)); + CHECK_EQ(*i::Object::GetElement(isolate, obj, 1), + *i::Object::GetElement(isolate, clone, 1)); CHECK_EQ(obj->GetProperty(*first), clone->GetProperty(*first)); CHECK_EQ(obj->GetProperty(*second), clone->GetProperty(*second)); @@ -830,8 +832,10 @@ TEST(JSObjectCopy) { JSReceiver::SetElement(clone, 0, second, NONE, SLOPPY); JSReceiver::SetElement(clone, 1, first, NONE, SLOPPY); - CHECK_EQ(obj->GetElement(isolate, 1), clone->GetElement(isolate, 0)); - CHECK_EQ(obj->GetElement(isolate, 0), clone->GetElement(isolate, 1)); + CHECK_EQ(*i::Object::GetElement(isolate, obj, 1), + *i::Object::GetElement(isolate, clone, 0)); + CHECK_EQ(*i::Object::GetElement(isolate, obj, 0), + *i::Object::GetElement(isolate, clone, 1)); CHECK_EQ(obj->GetProperty(*second), clone->GetProperty(*first)); CHECK_EQ(obj->GetProperty(*first), clone->GetProperty(*second)); -- 2.7.4