Revert "reworked set property and fixed corner cases of typed array set property"

This reverts commit 1b38f7b7a7709c3fe21f1adcf1d5de6b2e0e48f7.

Breaks tostring setter for some reason; we'll have to investigate...

1 files changed, 166 insertions, 110 deletions

diff --git a/lib/quickjs/quickjs.c b/lib/quickjs/quickjs.c
index 2dbc4914..3c6356c7 100644
--- a/lib/quickjs/quickjs.c
+++ b/lib/quickjs/quickjs.c
@@ -8414,14 +8414,119 @@ static void js_free_desc(JSContext *ctx, JSPropertyDescriptor *desc)
     JS_FreeValue(ctx, desc->value);
 }
 
+/* generic (and slower) version of JS_SetProperty() for
+ * Reflect.set(). 'obj' must be an object.  */
+static int JS_SetPropertyGeneric(JSContext *ctx,
+                                 JSValueConst obj, JSAtom prop,
+                                 JSValue val, JSValueConst this_obj,
+                                 int flags)
+{
+    int ret;
+    JSPropertyDescriptor desc;
+    JSValue obj1;
+    JSObject *p;
+    
+    obj1 = JS_DupValue(ctx, obj);
+    for(;;) {
+        p = JS_VALUE_GET_OBJ(obj1);
+        if (p->is_exotic) {
+            const JSClassExoticMethods *em = ctx->rt->class_array[p->class_id].exotic;
+            if (em && em->set_property) {
+                ret = em->set_property(ctx, obj1, prop,
+                                       val, this_obj, flags);
+                JS_FreeValue(ctx, obj1);
+                JS_FreeValue(ctx, val);
+                return ret;
+            }
+        }
+
+        ret = JS_GetOwnPropertyInternal(ctx, &desc, p, prop);
+        if (ret < 0) {
+            JS_FreeValue(ctx, obj1);
+            JS_FreeValue(ctx, val);
+            return ret;
+        }
+        if (ret) {
+            if (desc.flags & JS_PROP_GETSET) {
+                JSObject *setter;
+                if (JS_IsUndefined(desc.setter))
+                    setter = NULL;
+                else
+                    setter = JS_VALUE_GET_OBJ(desc.setter);
+                ret = call_setter(ctx, setter, this_obj, val, flags);
+                JS_FreeValue(ctx, desc.getter);
+                JS_FreeValue(ctx, desc.setter);
+                JS_FreeValue(ctx, obj1);
+                return ret;
+            } else {
+                JS_FreeValue(ctx, desc.value);
+                if (!(desc.flags & JS_PROP_WRITABLE)) {
+                    JS_FreeValue(ctx, obj1);
+                    goto read_only_error;
+                }
+            }
+            break;
+        }
+        /* Note: at this point 'obj1' cannot be a proxy. XXX: may have
+           to check recursion */
+        obj1 = JS_GetPrototypeFree(ctx, obj1);
+        if (JS_IsNull(obj1))
+            break;
+    }
+    JS_FreeValue(ctx, obj1);
+
+    if (!JS_IsObject(this_obj)) {
+        JS_FreeValue(ctx, val);
+        return JS_ThrowTypeErrorOrFalse(ctx, flags, "receiver is not an object");
+    }
+    
+    p = JS_VALUE_GET_OBJ(this_obj);
+
+    /* modify the property in this_obj if it already exists */
+    ret = JS_GetOwnPropertyInternal(ctx, &desc, p, prop);
+    if (ret < 0) {
+        JS_FreeValue(ctx, val);
+        return ret;
+    }
+    if (ret) {
+        if (desc.flags & JS_PROP_GETSET) {
+            JS_FreeValue(ctx, desc.getter);
+            JS_FreeValue(ctx, desc.setter);
+            JS_FreeValue(ctx, val);
+            return JS_ThrowTypeErrorOrFalse(ctx, flags, "setter is forbidden");
+        } else {
+            JS_FreeValue(ctx, desc.value);
+            if (!(desc.flags & JS_PROP_WRITABLE) ||
+                p->class_id == JS_CLASS_MODULE_NS) {
+            read_only_error:
+                JS_FreeValue(ctx, val);
+                return JS_ThrowTypeErrorReadOnly(ctx, flags, prop);
+            }
+        }
+        ret = JS_DefineProperty(ctx, this_obj, prop, val,
+                                JS_UNDEFINED, JS_UNDEFINED,
+                                JS_PROP_HAS_VALUE);
+        JS_FreeValue(ctx, val);
+        return ret;
+    }
+
+    ret = JS_CreateProperty(ctx, p, prop, val, JS_UNDEFINED, JS_UNDEFINED,
+                            flags |
+                            JS_PROP_HAS_VALUE |
+                            JS_PROP_HAS_ENUMERABLE |
+                            JS_PROP_HAS_WRITABLE |
+                            JS_PROP_HAS_CONFIGURABLE |
+                            JS_PROP_C_W_E);
+    JS_FreeValue(ctx, val);
+    return ret;
+}
+
 /* return -1 in case of exception or TRUE or FALSE. Warning: 'val' is
    freed by the function. 'flags' is a bitmask of JS_PROP_NO_ADD,
    JS_PROP_THROW or JS_PROP_THROW_STRICT. If JS_PROP_NO_ADD is set,
-   the new property is not added and an error is raised. 'this_obj' is
-   the receiver. If obj != this_obj, then obj must be an object
-   (Reflect.set case). */
-int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
-                           JSAtom prop, JSValue val, JSValueConst this_obj, int flags)
+   the new property is not added and an error is raised. */
+int JS_SetPropertyInternal(JSContext *ctx, JSValueConst this_obj,
+                           JSAtom prop, JSValue val, int flags)
 {
     JSObject *p, *p1;
     JSShapeProperty *prs;
@@ -8434,37 +8539,25 @@ int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
 #endif
     tag = JS_VALUE_GET_TAG(this_obj);
     if (unlikely(tag != JS_TAG_OBJECT)) {
-        if (JS_VALUE_GET_TAG(obj) == JS_TAG_OBJECT) {
+        switch(tag) {
+        case JS_TAG_NULL:
+            JS_FreeValue(ctx, val);
+            JS_ThrowTypeErrorAtom(ctx, "cannot set property '%s' of null", prop);
+            return -1;
+        case JS_TAG_UNDEFINED:
+            JS_FreeValue(ctx, val);
+            JS_ThrowTypeErrorAtom(ctx, "cannot set property '%s' of undefined", prop);
+            return -1;
+        default:
+            /* even on a primitive type we can have setters on the prototype */
             p = NULL;
-            p1 = JS_VALUE_GET_OBJ(obj);
+            p1 = JS_VALUE_GET_OBJ(JS_GetPrototypePrimitive(ctx, this_obj));
             goto prototype_lookup;
-        } else {
-            switch(tag) {
-            case JS_TAG_NULL:
-                JS_FreeValue(ctx, val);
-                JS_ThrowTypeErrorAtom(ctx, "cannot set property '%s' of null", prop);
-                return -1;
-            case JS_TAG_UNDEFINED:
-                JS_FreeValue(ctx, val);
-                JS_ThrowTypeErrorAtom(ctx, "cannot set property '%s' of undefined", prop);
-                return -1;
-            default:
-                /* even on a primitive type we can have setters on the prototype */
-                p = NULL;
-                p1 = JS_VALUE_GET_OBJ(JS_GetPrototypePrimitive(ctx, obj));
-                goto prototype_lookup;
-            }
         }
-    } else {
-        p = JS_VALUE_GET_OBJ(this_obj);
-        p1 = JS_VALUE_GET_OBJ(obj);
-        if (unlikely(p != p1))
-            goto retry2;
     }
-
-    /* fast path if obj == this_obj */
- retry:
-    prs = find_own_property(&pr, p1, prop);
+    p = JS_VALUE_GET_OBJ(this_obj);
+retry:
+    prs = find_own_property(&pr, p, prop);
     if (prs) {
         if (likely((prs->flags & (JS_PROP_TMASK | JS_PROP_WRITABLE |
                                   JS_PROP_LENGTH)) == JS_PROP_WRITABLE)) {
@@ -8496,7 +8589,8 @@ int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
             goto read_only_prop;
         }
     }
-    
+
+    p1 = p;
     for(;;) {
         if (p1->is_exotic) {
             if (p1->fast_array) {
@@ -8520,19 +8614,11 @@ int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
                             return -1;
                         }
                     typed_array_oob:
-                        /* must convert the argument even if out of bound access */
-                        if (p1->class_id == JS_CLASS_BIG_INT64_ARRAY ||
-                            p1->class_id == JS_CLASS_BIG_UINT64_ARRAY) {
-                            int64_t v;
-                            if (JS_ToBigInt64Free(ctx, &v, val))
-                                return -1;
-                        } else {
-                            val = JS_ToNumberFree(ctx, val);
-                            JS_FreeValue(ctx, val);
-                            if (JS_IsException(val))
-                                return -1;
-                        }
-                        return TRUE;
+                        val = JS_ToNumberFree(ctx, val);
+                        JS_FreeValue(ctx, val);
+                        if (JS_IsException(val))
+                            return -1;
+                        return JS_ThrowTypeErrorOrFalse(ctx, flags, "out-of-bound numeric index");
                     }
                 }
             } else {
@@ -8604,7 +8690,9 @@ int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
                     return -1;
                 goto retry2;
             } else if (!(prs->flags & JS_PROP_WRITABLE)) {
-                goto read_only_prop;
+            read_only_prop:
+                JS_FreeValue(ctx, val);
+                return JS_ThrowTypeErrorReadOnly(ctx, flags, prop);
             }
         }
     }
@@ -8625,57 +8713,17 @@ int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
         return JS_ThrowTypeErrorOrFalse(ctx, flags, "object is not extensible");
     }
 
-    if (likely(p == JS_VALUE_GET_OBJ(obj))) {
-        if (p->is_exotic) {
-            if (p->class_id == JS_CLASS_ARRAY && p->fast_array &&
-                __JS_AtomIsTaggedInt(prop)) {
-                uint32_t idx = __JS_AtomToUInt32(prop);
-                if (idx == p->u.array.count) {
-                    /* fast case */
-                    return add_fast_array_element(ctx, p, val, flags);
-                } else {
-                    goto generic_create_prop;
-                }
+    if (p->is_exotic) {
+        if (p->class_id == JS_CLASS_ARRAY && p->fast_array &&
+            __JS_AtomIsTaggedInt(prop)) {
+            uint32_t idx = __JS_AtomToUInt32(prop);
+            if (idx == p->u.array.count) {
+                /* fast case */
+                return add_fast_array_element(ctx, p, val, flags);
             } else {
                 goto generic_create_prop;
             }
         } else {
-            pr = add_property(ctx, p, prop, JS_PROP_C_W_E);
-            if (unlikely(!pr)) {
-                JS_FreeValue(ctx, val);
-                return -1;
-            }
-            pr->u.value = val;
-            return TRUE;
-        }
-    } else {
-        /* generic case: modify the property in this_obj if it already exists */
-        ret = JS_GetOwnPropertyInternal(ctx, &desc, p, prop);
-        if (ret < 0) {
-            JS_FreeValue(ctx, val);
-            return ret;
-        }
-        if (ret) {
-            if (desc.flags & JS_PROP_GETSET) {
-                JS_FreeValue(ctx, desc.getter);
-                JS_FreeValue(ctx, desc.setter);
-                JS_FreeValue(ctx, val);
-                return JS_ThrowTypeErrorOrFalse(ctx, flags, "setter is forbidden");
-            } else {
-                JS_FreeValue(ctx, desc.value);
-                if (!(desc.flags & JS_PROP_WRITABLE) ||
-                    p->class_id == JS_CLASS_MODULE_NS) {
-                read_only_prop:
-                    JS_FreeValue(ctx, val);
-                    return JS_ThrowTypeErrorReadOnly(ctx, flags, prop);
-                }
-            }
-            ret = JS_DefineProperty(ctx, this_obj, prop, val,
-                                    JS_UNDEFINED, JS_UNDEFINED,
-                                    JS_PROP_HAS_VALUE);
-            JS_FreeValue(ctx, val);
-            return ret;
-        } else {
         generic_create_prop:
             ret = JS_CreateProperty(ctx, p, prop, val, JS_UNDEFINED, JS_UNDEFINED,
                                     flags |
@@ -8688,6 +8736,14 @@ int JS_SetPropertyInternal(JSContext *ctx, JSValueConst obj,
             return ret;
         }
     }
+
+    pr = add_property(ctx, p, prop, JS_PROP_C_W_E);
+    if (unlikely(!pr)) {
+        JS_FreeValue(ctx, val);
+        return -1;
+    }
+    pr->u.value = val;
+    return TRUE;
 }
 
 /* flags can be JS_PROP_THROW or JS_PROP_THROW_STRICT */
@@ -8796,7 +8852,7 @@ static int JS_SetPropertyValue(JSContext *ctx, JSValueConst this_obj,
                 return -1;
             if (unlikely(idx >= (uint32_t)p->u.array.count)) {
             ta_out_of_bound:
-                return TRUE;
+                return JS_ThrowTypeErrorOrFalse(ctx, flags, "out-of-bound numeric index");
             }
             p->u.array.u.double_ptr[idx] = d;
             break;
@@ -8814,7 +8870,7 @@ static int JS_SetPropertyValue(JSContext *ctx, JSValueConst this_obj,
             JS_FreeValue(ctx, val);
             return -1;
         }
-        ret = JS_SetPropertyInternal(ctx, this_obj, atom, val, this_obj, flags);
+        ret = JS_SetPropertyInternal(ctx, this_obj, atom, val, flags);
         JS_FreeAtom(ctx, atom);
         return ret;
     }
@@ -8854,7 +8910,7 @@ int JS_SetPropertyStr(JSContext *ctx, JSValueConst this_obj,
     JSAtom atom;
     int ret;
     atom = JS_NewAtom(ctx, prop);
-    ret = JS_SetPropertyInternal(ctx, this_obj, atom, val, this_obj, JS_PROP_THROW);
+    ret = JS_SetPropertyInternal(ctx, this_obj, atom, val, JS_PROP_THROW);
     JS_FreeAtom(ctx, atom);
     return ret;
 }
@@ -9311,7 +9367,7 @@ int JS_DefineProperty(JSContext *ctx, JSValueConst this_obj,
             }
             idx = __JS_AtomToUInt32(prop);
             /* if the typed array is detached, p->u.array.count = 0 */
-            if (idx >= p->u.array.count) {
+            if (idx >= typed_array_get_length(ctx, p)) {
             typed_array_oob:
                 return JS_ThrowTypeErrorOrFalse(ctx, flags, "out-of-bound index in typed array");
             }
@@ -9705,7 +9761,7 @@ static int JS_SetGlobalVar(JSContext *ctx, JSAtom prop, JSValue val,
     flags = JS_PROP_THROW_STRICT;
     if (is_strict_mode(ctx)) 
         flags |= JS_PROP_NO_ADD;
-    return JS_SetPropertyInternal(ctx, ctx->global_obj, prop, val, ctx->global_obj, flags);
+    return JS_SetPropertyInternal(ctx, ctx->global_obj, prop, val, flags);
 }
 
 /* return -1, FALSE or TRUE. return FALSE if not configurable or
@@ -14914,7 +14970,7 @@ static JSValue build_for_in_iterator(JSContext *ctx, JSValue obj)
                                    JS_GPN_STRING_MASK | JS_GPN_ENUM_ONLY))
             goto fail;
         for(i = 0; i < tab_atom_count; i++) {
-            JS_SetPropertyInternal(ctx, enum_obj, tab_atom[i].atom, JS_NULL, enum_obj, 0);
+            JS_SetPropertyInternal(ctx, enum_obj, tab_atom[i].atom, JS_NULL, 0);
         }
         js_free_prop_enum(ctx, tab_atom, tab_atom_count);
     }
@@ -17334,7 +17390,7 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
                 atom = get_u32(pc);
                 pc += 4;
 
-                ret = JS_SetPropertyInternal(ctx, sp[-2], atom, sp[-1], sp[-2],
+                ret = JS_SetPropertyInternal(ctx, sp[-2], atom, sp[-1],
                                              JS_PROP_THROW_STRICT);
                 JS_FreeValue(ctx, sp[-2]);
                 sp -= 2;
@@ -17633,8 +17689,8 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
                 atom = JS_ValueToAtom(ctx, sp[-2]);
                 if (unlikely(atom == JS_ATOM_NULL))
                     goto exception;
-                ret = JS_SetPropertyInternal(ctx, sp[-3], atom, sp[-1], sp[-4],
-                                             JS_PROP_THROW_STRICT);
+                ret = JS_SetPropertyGeneric(ctx, sp[-3], atom, sp[-1], sp[-4],
+                                            JS_PROP_THROW_STRICT);
                 JS_FreeAtom(ctx, atom);
                 JS_FreeValue(ctx, sp[-4]);
                 JS_FreeValue(ctx, sp[-3]);
@@ -18313,7 +18369,7 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
                         break;
                     case OP_with_put_var:
                         /* XXX: check if strict mode */
-                        ret = JS_SetPropertyInternal(ctx, obj, atom, sp[-2], obj,
+                        ret = JS_SetPropertyInternal(ctx, obj, atom, sp[-2],
                                                      JS_PROP_THROW_STRICT);
                         JS_FreeValue(ctx, sp[-1]);
                         sp -= 2;
@@ -44298,8 +44354,8 @@ static JSValue js_reflect_set(JSContext *ctx, JSValueConst this_val,
     atom = JS_ValueToAtom(ctx, prop);
     if (unlikely(atom == JS_ATOM_NULL))
         return JS_EXCEPTION;
-    ret = JS_SetPropertyInternal(ctx, obj, atom,
-                                 JS_DupValue(ctx, val), receiver, 0);
+    ret = JS_SetPropertyGeneric(ctx, obj, atom,
+                                JS_DupValue(ctx, val), receiver, 0);
     JS_FreeAtom(ctx, atom);
     if (ret < 0)
         return JS_EXCEPTION;
@@ -44646,9 +44702,9 @@ static int js_proxy_set(JSContext *ctx, JSValueConst obj, JSAtom atom,
     if (!s)
         return -1;
     if (JS_IsUndefined(method)) {
-        return JS_SetPropertyInternal(ctx, s->target, atom,
-                                      JS_DupValue(ctx, value), receiver,
-                                      flags);
+        return JS_SetPropertyGeneric(ctx, s->target, atom,
+                                     JS_DupValue(ctx, value), receiver,
+                                     flags);
     }
     atom_val = JS_AtomToValue(ctx, atom);
     if (JS_IsException(atom_val)) {
@@ -51712,7 +51768,7 @@ static JSValue js_typed_array_at(JSContext *ctx, JSValueConst this_val,
         idx = len + idx;
     if (idx < 0 || idx >= len)
         return JS_UNDEFINED;
-    return JS_GetPropertyInt64(ctx, this_val, idx);
+    return JS_GetPropertyUint32(ctx, this_val, idx);
 }
 
 static JSValue js_typed_array_set(JSContext *ctx,