From b51b5100b09ead3f101f75764b8e1178157f62d4 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 14 Dec 2023 11:12:55 +0100 Subject: [PATCH] Handle negative zero typed array indices correctly (#212) `ta["-0"] = 42` is a thing and not just any thing but a decidedly weird thing: it completes successful, sets no property, but still evaluates the value for side effects. --- quickjs-atom.h | 1 + quickjs.c | 36 ++++++++++++++++++++++++++++++------ test262_errors.txt | 4 ---- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/quickjs-atom.h b/quickjs-atom.h index 0215192..f54438d 100644 --- a/quickjs-atom.h +++ b/quickjs-atom.h @@ -77,6 +77,7 @@ DEF(await, "await") /* empty string */ DEF(empty_string, "") +DEF(negative_zero, "-0") /* identifiers */ DEF(length, "length") DEF(fileName, "fileName") diff --git a/quickjs.c b/quickjs.c index 7461263..80dbe7d 100644 --- a/quickjs.c +++ b/quickjs.c @@ -1070,6 +1070,11 @@ static void js_promise_resolve_function_finalizer(JSRuntime *rt, JSValue val); static void js_promise_resolve_function_mark(JSRuntime *rt, JSValue val, JS_MarkFunc *mark_func); +#define HINT_STRING 0 +#define HINT_NUMBER 1 +#define HINT_NONE 2 +#define HINT_FORCE_ORDINARY (1 << 4) // don't try Symbol.toPrimitive +static JSValue JS_ToPrimitiveFree(JSContext *ctx, JSValue val, int hint); static JSValue JS_ToStringFree(JSContext *ctx, JSValue val); static int JS_ToBoolFree(JSContext *ctx, JSValue val); static int JS_ToInt32Free(JSContext *ctx, int32_t *pres, JSValue val); @@ -8224,6 +8229,7 @@ static void js_free_desc(JSContext *ctx, JSPropertyDescriptor *desc) /* generic (and slower) version of JS_SetProperty() for * Reflect.set(). 'obj' must be an object. */ +// TODO(bnoordhuis) merge with JS_SetPropertyInternal2 static int JS_SetPropertyGeneric(JSContext *ctx, JSValue obj, JSAtom prop, JSValue val, JSValue this_obj, @@ -8289,6 +8295,18 @@ static int JS_SetPropertyGeneric(JSContext *ctx, } p = JS_VALUE_GET_OBJ(this_obj); + if (prop == JS_ATOM_negative_zero && + p->class_id >= JS_CLASS_UINT8C_ARRAY && + p->class_id <= JS_CLASS_FLOAT64_ARRAY) { + // per spec: CanonicalNumericIndexString("-0") evaluates to -0 and + // returns true but sets no property; side effects of evaluating + // the value should still be observable + val = JS_ToPrimitiveFree(ctx, val, HINT_NUMBER); + if (JS_IsException(val)) + return -1; + JS_FreeValue(ctx, val); + return TRUE; + } /* modify the property in this_obj if it already exists */ ret = JS_GetOwnPropertyInternal(ctx, &desc, p, prop); @@ -8363,6 +8381,18 @@ int JS_SetPropertyInternal2(JSContext *ctx, JSValue this_obj, } } p = JS_VALUE_GET_OBJ(this_obj); + if (prop == JS_ATOM_negative_zero && + p->class_id >= JS_CLASS_UINT8C_ARRAY && + p->class_id <= JS_CLASS_FLOAT64_ARRAY) { + // per spec: CanonicalNumericIndexString("-0") evaluates to -0 and + // returns true but sets no property; side effects of evaluating + // the value should still be observable + val = JS_ToPrimitiveFree(ctx, val, HINT_NUMBER); + if (JS_IsException(val)) + return -1; + JS_FreeValue(ctx, val); + return TRUE; + } retry: prs = find_own_property_ic(&pr, p, prop, &offset); if (prs) { @@ -9770,12 +9800,6 @@ void *JS_GetAnyOpaque(JSValue obj, JSClassID *class_id) return p->u.opaque; } -#define HINT_STRING 0 -#define HINT_NUMBER 1 -#define HINT_NONE 2 -/* don't try Symbol.toPrimitive */ -#define HINT_FORCE_ORDINARY (1 << 4) - static JSValue JS_ToPrimitiveFree(JSContext *ctx, JSValue val, int hint) { int i; diff --git a/test262_errors.txt b/test262_errors.txt index 79db9c0..dd216f8 100644 --- a/test262_errors.txt +++ b/test262_errors.txt @@ -13,16 +13,12 @@ test262/test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/tonumb test262/test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/tonumber-value-detached-buffer.js:42: strict mode: Test262Error: Reflect.defineProperty(ta, 0, {value: {valueOf() {$DETACHBUFFER(ta.buffer); return 42;}}} ) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/detached-buffer.js:34: TypeError: cannot convert bigint to number (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/detached-buffer.js:34: strict mode: TypeError: cannot convert bigint to number (Testing with BigInt64Array.) -test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-minus-zero.js:20: Test262Error: Reflect.set("new TA([42n])", "-0", 1n) must return true Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) -test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-minus-zero.js:20: strict mode: Test262Error: Reflect.set("new TA([42n])", "-0", 1n) must return true Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-not-integer.js:21: Test262Error: Reflect.set("new TA([42n])", "1.1", 1n) must return true Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-not-integer.js:21: strict mode: Test262Error: Reflect.set("new TA([42n])", "1.1", 1n) must return true Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-out-of-bounds.js:27: Test262Error: Reflect.set("new TA([42n])", "-1", 1n) must return false Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-out-of-bounds.js:27: strict mode: Test262Error: Reflect.set("new TA([42n])", "-1", 1n) must return false Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/tonumber-value-detached-buffer.js:24: Test262Error: Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/BigInt/tonumber-value-detached-buffer.js:24: strict mode: Test262Error: Expected SameValue(«false», «true») to be true (Testing with BigInt64Array.) -test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-minus-zero.js:22: Test262Error: Reflect.set(sample, "-0", 1) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.) -test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-minus-zero.js:22: strict mode: Test262Error: Reflect.set(sample, "-0", 1) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-not-integer.js:22: Test262Error: Reflect.set(sample, "1.1", 1) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-not-integer.js:22: strict mode: Test262Error: Reflect.set(sample, "1.1", 1) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.) test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds.js:22: Test262Error: Reflect.set(sample, "-1", 1) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.)