Commit 8b18c5e6 authored by Patrick Thier's avatar Patrick Thier Committed by V8 LUCI CQ

Reland "Improve error messages for property access on null/undefined"

This is a reland of 24c626c1

Original change's description:
> Improve error messages for property access on null/undefined
>
> Only print the property name when accessing null/undefined if we can
> convert it to a string without causing side effects.
> If we can't, omit the property name in the error message.
> This should avoid confusion when the key is an object with toString().
> E.g. undefined[{toString:()=>'a'}] doesn't print 'read property [object
> Object]' anymore, which was misleading since the property accessed would
> be 'a', but we can't evaluate the key without side effects.
>
> Bug: v8:11365
> Change-Id: If82d1adb42561d4851e2bd2ca297a1c71738aee8
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2960211
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Commit-Queue: Patrick Thier <pthier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#75250}

Bug: v8:11365
Change-Id: Ie2312337f4f1915faa31528a728d90833d80dbd1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2979599Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75571}
parent d42ae802
......@@ -129,8 +129,12 @@ namespace internal {
T(NonObjectAssertOption, "The 'assert' option must be an object") \
T(NonObjectInInstanceOfCheck, \
"Right-hand side of 'instanceof' is not an object") \
T(NonObjectPropertyLoad, "Cannot read property '%' of %") \
T(NonObjectPropertyStore, "Cannot set property '%' of %") \
T(NonObjectPropertyLoad, "Cannot read properties of %") \
T(NonObjectPropertyLoadWithProperty, \
"Cannot read properties of % (reading '%')") \
T(NonObjectPropertyStore, "Cannot set properties of %") \
T(NonObjectPropertyStoreWithProperty, \
"Cannot set properties of % (setting '%')") \
T(NonObjectImportArgument, \
"The second argument to import() must be an object") \
T(NonStringImportAssertionValue, "Import assertion value must be a string") \
......
......@@ -896,6 +896,9 @@ Object ErrorUtils::ThrowLoadFromNullOrUndefined(Isolate* isolate,
if (key.ToHandle(&key_handle)) {
if (key_handle->IsString()) {
maybe_property_name = Handle<String>::cast(key_handle);
} else {
maybe_property_name =
Object::NoSideEffectsToMaybeString(isolate, key_handle);
}
}
......@@ -969,14 +972,16 @@ Object ErrorUtils::ThrowLoadFromNullOrUndefined(Isolate* isolate,
}
} else {
Handle<Object> key_handle;
if (!key.ToHandle(&key_handle)) {
key_handle = ReadOnlyRoots(isolate).undefined_value_handle();
}
if (*key_handle == ReadOnlyRoots(isolate).iterator_symbol()) {
if (!key.ToHandle(&key_handle) ||
!maybe_property_name.ToHandle(&property_name)) {
error = isolate->factory()->NewTypeError(
MessageTemplate::kNonObjectPropertyLoad, object);
} else if (*key_handle == ReadOnlyRoots(isolate).iterator_symbol()) {
error = NewIteratorError(isolate, object);
} else {
error = isolate->factory()->NewTypeError(
MessageTemplate::kNonObjectPropertyLoad, key_handle, object);
MessageTemplate::kNonObjectPropertyLoadWithProperty, object,
property_name);
}
}
......
......@@ -1726,7 +1726,8 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> name,
SetCache(name, StoreHandler::StoreSlow(isolate()));
TraceIC("StoreIC", name);
}
return TypeError(MessageTemplate::kNonObjectPropertyStore, object, name);
return TypeError(MessageTemplate::kNonObjectPropertyStoreWithProperty, name,
object);
}
JSObject::MakePrototypesFast(object, kStartAtPrototype, isolate());
......
......@@ -463,8 +463,8 @@ Handle<String> NoSideEffectsErrorToString(Isolate* isolate,
} // namespace
// static
Handle<String> Object::NoSideEffectsToString(Isolate* isolate,
Handle<Object> input) {
MaybeHandle<String> Object::NoSideEffectsToMaybeString(Isolate* isolate,
Handle<Object> input) {
DisallowJavascriptExecution no_js(isolate);
if (input->IsString() || input->IsNumber() || input->IsOddball()) {
......@@ -561,6 +561,20 @@ Handle<String> Object::NoSideEffectsToString(Isolate* isolate,
}
}
}
return MaybeHandle<String>(kNullMaybeHandle);
}
// static
Handle<String> Object::NoSideEffectsToString(Isolate* isolate,
Handle<Object> input) {
DisallowJavascriptExecution no_js(isolate);
// Try to convert input to a meaningful string.
MaybeHandle<String> maybe_string = NoSideEffectsToMaybeString(isolate, input);
Handle<String> string_handle;
if (maybe_string.ToHandle(&string_handle)) {
return string_handle;
}
// At this point, input is either none of the above or a JSReceiver.
......
......@@ -410,6 +410,9 @@ class Object : public TaggedImpl<HeapObjectReferenceType::STRONG, Address> {
V8_WARN_UNUSED_RESULT static inline MaybeHandle<String> ToString(
Isolate* isolate, Handle<Object> input);
V8_EXPORT_PRIVATE static MaybeHandle<String> NoSideEffectsToMaybeString(
Isolate* isolate, Handle<Object> input);
V8_EXPORT_PRIVATE static Handle<String> NoSideEffectsToString(
Isolate* isolate, Handle<Object> input);
......
......@@ -675,11 +675,12 @@ MaybeHandle<JSReceiver> GetSuperHolder(Isolate* isolate,
PrototypeIterator iter(isolate, home_object);
Handle<Object> proto = PrototypeIterator::GetCurrent(iter);
if (!proto->IsJSReceiver()) {
MessageTemplate message = mode == SuperMode::kLoad
? MessageTemplate::kNonObjectPropertyLoad
: MessageTemplate::kNonObjectPropertyStore;
MessageTemplate message =
mode == SuperMode::kLoad
? MessageTemplate::kNonObjectPropertyLoadWithProperty
: MessageTemplate::kNonObjectPropertyStoreWithProperty;
Handle<Name> name = key->GetName(isolate);
THROW_NEW_ERROR(isolate, NewTypeError(message, name, proto), JSReceiver);
THROW_NEW_ERROR(isolate, NewTypeError(message, proto, name), JSReceiver);
}
return Handle<JSReceiver>::cast(proto);
}
......
......@@ -525,10 +525,21 @@ MaybeHandle<Object> Runtime::SetObjectProperty(
Handle<Object> value, StoreOrigin store_origin,
Maybe<ShouldThrow> should_throw) {
if (object->IsNullOrUndefined(isolate)) {
THROW_NEW_ERROR(
isolate,
NewTypeError(MessageTemplate::kNonObjectPropertyStore, key, object),
Object);
MaybeHandle<String> maybe_property =
Object::NoSideEffectsToMaybeString(isolate, key);
Handle<String> property_name;
if (maybe_property.ToHandle(&property_name)) {
THROW_NEW_ERROR(
isolate,
NewTypeError(MessageTemplate::kNonObjectPropertyStoreWithProperty,
object, property_name),
Object);
} else {
THROW_NEW_ERROR(
isolate,
NewTypeError(MessageTemplate::kNonObjectPropertyStore, object),
Object);
}
}
// Check if the given key is an array index.
......
......@@ -83,7 +83,7 @@ bytecodes: [
B(Mov), R(this), R(0),
B(Mov), R(context), R(2),
/* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 53 S> */ B(Wide), B(LdaSmi), I16(279),
/* 53 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star3),
B(LdaConstant), U8(0),
B(Star4),
......@@ -114,7 +114,7 @@ bytecodes: [
B(Mov), R(this), R(0),
B(Mov), R(context), R(2),
/* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 46 S> */ B(Wide), B(LdaSmi), I16(278),
/* 46 S> */ B(Wide), B(LdaSmi), I16(280),
B(Star3),
B(LdaConstant), U8(0),
B(Star4),
......@@ -145,7 +145,7 @@ bytecodes: [
B(Mov), R(this), R(0),
B(Mov), R(context), R(2),
/* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 53 S> */ B(Wide), B(LdaSmi), I16(279),
/* 53 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star3),
B(LdaConstant), U8(0),
B(Star4),
......@@ -176,7 +176,7 @@ bytecodes: [
B(Mov), R(this), R(0),
B(Mov), R(context), R(2),
/* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 46 S> */ B(Wide), B(LdaSmi), I16(278),
/* 46 S> */ B(Wide), B(LdaSmi), I16(280),
B(Star4),
B(LdaConstant), U8(0),
B(Star5),
......
......@@ -56,7 +56,7 @@ bytecodes: [
B(Mov), R(this), R(0),
B(Mov), R(context), R(2),
/* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 49 S> */ B(Wide), B(LdaSmi), I16(277),
/* 49 S> */ B(Wide), B(LdaSmi), I16(279),
B(Star3),
B(LdaConstant), U8(0),
B(Star4),
......@@ -88,7 +88,7 @@ bytecodes: [
B(Mov), R(this), R(0),
B(Mov), R(context), R(2),
/* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 49 S> */ B(Wide), B(LdaSmi), I16(277),
/* 49 S> */ B(Wide), B(LdaSmi), I16(279),
B(Star3),
B(LdaConstant), U8(0),
B(Star4),
......
......@@ -24,7 +24,7 @@ bytecodes: [
B(TestReferenceEqual), R(this),
B(Mov), R(this), R(1),
B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(275),
B(Wide), B(LdaSmi), I16(277),
B(Star2),
B(LdaConstant), U8(0),
B(Star3),
......@@ -55,7 +55,7 @@ frame size: 2
parameter count: 1
bytecode array length: 14
bytecodes: [
/* 56 S> */ B(Wide), B(LdaSmi), I16(277),
/* 56 S> */ B(Wide), B(LdaSmi), I16(279),
B(Star0),
B(LdaConstant), U8(0),
B(Star1),
......@@ -82,7 +82,7 @@ frame size: 2
parameter count: 1
bytecode array length: 14
bytecodes: [
/* 56 S> */ B(Wide), B(LdaSmi), I16(277),
/* 56 S> */ B(Wide), B(LdaSmi), I16(279),
B(Star0),
B(LdaConstant), U8(0),
B(Star1),
......@@ -121,7 +121,7 @@ bytecodes: [
B(TestReferenceEqual), R(this),
B(Mov), R(this), R(0),
B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(275),
B(Wide), B(LdaSmi), I16(277),
B(Star2),
B(LdaConstant), U8(0),
B(Star3),
......@@ -143,7 +143,7 @@ bytecodes: [
B(TestReferenceEqual), R(this),
B(Mov), R(this), R(1),
B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(276),
B(Wide), B(LdaSmi), I16(278),
B(Star3),
B(LdaConstant), U8(0),
B(Star4),
......@@ -158,7 +158,7 @@ bytecodes: [
B(TestReferenceEqual), R(this),
B(Mov), R(this), R(0),
B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(275),
B(Wide), B(LdaSmi), I16(277),
B(Star2),
B(LdaConstant), U8(0),
B(Star3),
......@@ -188,7 +188,7 @@ frame size: 2
parameter count: 1
bytecode array length: 14
bytecodes: [
/* 60 S> */ B(Wide), B(LdaSmi), I16(279),
/* 60 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star0),
B(LdaConstant), U8(0),
B(Star1),
......@@ -214,7 +214,7 @@ frame size: 2
parameter count: 1
bytecode array length: 14
bytecodes: [
/* 53 S> */ B(Wide), B(LdaSmi), I16(278),
/* 53 S> */ B(Wide), B(LdaSmi), I16(280),
B(Star0),
B(LdaConstant), U8(0),
B(Star1),
......@@ -240,7 +240,7 @@ frame size: 2
parameter count: 1
bytecode array length: 14
bytecodes: [
/* 60 S> */ B(Wide), B(LdaSmi), I16(279),
/* 60 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star0),
B(LdaConstant), U8(0),
B(Star1),
......@@ -266,7 +266,7 @@ frame size: 3
parameter count: 1
bytecode array length: 14
bytecodes: [
/* 46 S> */ B(Wide), B(LdaSmi), I16(278),
/* 46 S> */ B(Wide), B(LdaSmi), I16(280),
B(Star1),
B(LdaConstant), U8(0),
B(Star2),
......
......@@ -27,7 +27,7 @@ class A { constructor () { this.a = 239; } }
class B extends A {
constructor () {
debugger;
assertTrue(result.indexOf("Cannot read property 'a' of undefined") >= 0);
assertTrue(result.indexOf("Cannot read properties of undefined (reading 'a')") >= 0);
super();
debugger;
assertEquals(239, result);
......
*%(basename)s:5: TypeError: Cannot destructure 'undefined' as it is undefined.
*%(basename)s:5: TypeError: Cannot destructure property '1' of 'undefined' as it is undefined.
var { 1: x } = undefined;
^
TypeError: Cannot destructure 'undefined' as it is undefined.
^
TypeError: Cannot destructure property '1' of 'undefined' as it is undefined.
at *%(basename)s:5:10
......@@ -25,9 +25,9 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*%(basename)s:31: TypeError: Cannot read property 'x' of undefined
*%(basename)s:31: TypeError: Cannot read properties of undefined (reading 'x')
undefined.x
^
TypeError: Cannot read property 'x' of undefined
TypeError: Cannot read properties of undefined (reading 'x')
at *%(basename)s:31:11
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var x = undefined;
x[{toString:()=>'a'}];
# Copyright 2021 the V8 project authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
*%(basename)s:6: TypeError: Cannot read properties of undefined
x[{toString:()=>'a'}];
^
TypeError: Cannot read properties of undefined
at *%(basename)s:6:2
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var x = undefined;
x[4711];
# Copyright 2021 the V8 project authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
*%(basename)s:6: TypeError: Cannot read properties of undefined (reading '4711')
x[4711];
^
TypeError: Cannot read properties of undefined (reading '4711')
at *%(basename)s:6:2
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var x = undefined;
x['a'];
# Copyright 2021 the V8 project authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
*%(basename)s:6: TypeError: Cannot read properties of undefined (reading 'a')
x['a'];
^
TypeError: Cannot read properties of undefined (reading 'a')
at *%(basename)s:6:2
......@@ -26,22 +26,22 @@ This test checks that deletion of properties works properly with getters and set
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS b1.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS a2.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS b3.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS a4.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS b5.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS a6.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS b7.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS a8.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o1.b.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o1.a.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o3.b.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o4.a.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o5.b.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o6.a.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o7.b.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS o8.a.property threw exception TypeError: Cannot read property 'property' of undefined.
PASS b1.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS a2.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS b3.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS a4.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS b5.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS a6.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS b7.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS a8.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o1.b.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o1.a.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o3.b.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o4.a.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o5.b.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o6.a.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o7.b.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS o8.a.property threw exception TypeError: Cannot read properties of undefined (reading 'property').
PASS successfullyParsed is true
TEST COMPLETE
......
......@@ -216,7 +216,7 @@ PASS baz(i) is 66
PASS baz(i) is 66
PASS baz(i) is 66
PASS baz(i) is 66
Caught exception: TypeError: Cannot read property 'g' of null
Caught exception: TypeError: Cannot read properties of null (reading 'g')
PASS baz(i) is "ERROR"
PASS baz(i) is 66
PASS baz(i) is 66
......
......@@ -160,7 +160,7 @@ PASS argumentsVarUndefined() is '[object Arguments]'
PASS argumentCalleeInException() is argumentCalleeInException
PASS shadowedArgumentsApply([true]) is true
PASS shadowedArgumentsLength([]) is 0
PASS shadowedArgumentsLength() threw exception TypeError: Cannot read property 'length' of undefined.
PASS shadowedArgumentsLength() threw exception TypeError: Cannot read properties of undefined (reading 'length').
PASS shadowedArgumentsCallee([]) is undefined.
PASS shadowedArgumentsIndex([true]) is true
PASS descriptor.value is "one"
......
......@@ -42,7 +42,7 @@ PASS Array.prototype.every.call(undefined, toString) threw exception TypeError:
PASS Array.prototype.forEach.call(undefined, toString) threw exception TypeError: Array.prototype.forEach called on null or undefined.
PASS Array.prototype.some.call(undefined, toString) threw exception TypeError: Array.prototype.some called on null or undefined.
PASS Array.prototype.indexOf.call(undefined, 0) threw exception TypeError: Array.prototype.indexOf called on null or undefined.
PASS Array.prototype.indlastIndexOfexOf.call(undefined, 0) threw exception TypeError: Cannot read property 'call' of undefined.
PASS Array.prototype.indlastIndexOfexOf.call(undefined, 0) threw exception TypeError: Cannot read properties of undefined (reading 'call').
PASS Array.prototype.filter.call(undefined, toString) threw exception TypeError: Array.prototype.filter called on null or undefined.
PASS Array.prototype.reduce.call(undefined, toString) threw exception TypeError: Array.prototype.reduce called on null or undefined.
PASS Array.prototype.reduceRight.call(undefined, toString) threw exception TypeError: Array.prototype.reduceRight called on null or undefined.
......
......@@ -35,14 +35,14 @@ PASS testLineContinuation.call(undefined) === undefined is false
PASS testEscapeSequence.call(undefined) === undefined is false
PASS testThis.call('a string') is 'a string'
PASS testThisDotAccess.call('a string') is 'a string'.length
PASS testThisDotAccess.call(null) threw exception TypeError: Cannot read property 'length' of null.
PASS testThisDotAccess.call(undefined) threw exception TypeError: Cannot read property 'length' of undefined.
PASS testThisDotAccess.call(null) threw exception TypeError: Cannot read properties of null (reading 'length').
PASS testThisDotAccess.call(undefined) threw exception TypeError: Cannot read properties of undefined (reading 'length').
PASS testThisDotAccess.call(true) is undefined.
PASS testThisDotAccess.call(false) is undefined.
PASS testThisDotAccess.call(1) is undefined.
PASS testThisBracketAccess.call('a string', 'length') is 'a string'.length
PASS testThisBracketAccess.call(null, 'length') threw exception TypeError: Cannot read property 'length' of null.
PASS testThisBracketAccess.call(undefined, 'length') threw exception TypeError: Cannot read property 'length' of undefined.
PASS testThisBracketAccess.call(null, 'length') threw exception TypeError: Cannot read properties of null (reading 'length').
PASS testThisBracketAccess.call(undefined, 'length') threw exception TypeError: Cannot read properties of undefined (reading 'length').
PASS testThisBracketAccess.call(true, 'length') is undefined.
PASS testThisBracketAccess.call(false, 'length') is undefined.
PASS testThisBracketAccess.call(1, 'length') is undefined.
......
......@@ -26,8 +26,8 @@ Tests for Date.toISOString
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS Date.toISOString.call({}) threw exception TypeError: Cannot read property 'call' of undefined.
PASS Date.toISOString.call(0) threw exception TypeError: Cannot read property 'call' of undefined.
PASS Date.toISOString.call({}) threw exception TypeError: Cannot read properties of undefined (reading 'call').
PASS Date.toISOString.call(0) threw exception TypeError: Cannot read properties of undefined (reading 'call').
PASS new Date(-400).toISOString() is '1969-12-31T23:59:59.600Z'
PASS new Date(0).toISOString() is '1970-01-01T00:00:00.000Z'
PASS new Date('1 January 1500 UTC').toISOString() is '1500-01-01T00:00:00.000Z'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment