Fix use-after-free when freeing a WeakMap with symbol values

quickjs.c

@@ -53868,12 +53868,20 @@ static void js_map_finalizer(JSRuntime *rt, JSValueConst val)
     if (s) {
         /* if the object is deleted we are sure that no iterator is
            using it */
+        if (s->is_weak) {
+            /* first pass to remove the weak references, so that freeing
+               a value (e.g. a symbol that is also a key of this map)
+               cannot reset and free records we have yet to visit */
+            list_for_each(el, &s->records) {
+                mr = list_entry(el, JSMapRecord, link);
+                if (!mr->empty)
+                    delete_map_weak_ref(rt, mr);
+            }
+        }
         list_for_each_safe(el, el1, &s->records) {
             mr = list_entry(el, JSMapRecord, link);
             if (!mr->empty) {
-                if (s->is_weak)
-                    delete_map_weak_ref(rt, mr);
-                else
+                if (!s->is_weak)
                     JS_FreeValueRT(rt, mr->key);
                 JS_FreeValueRT(rt, mr->value);
             }

tests/bug1835.js

@@ -0,0 +1,36 @@
+// https://github.com/quickjs-ng/quickjs/issues/1835
+// Freeing a WeakMap whose values are symbols that are also its keys crashed
+// in the WeakMap finalizer. Run under ASAN.
+import { assert } from "./assert.js";
+
+function f() {
+    var o = {};
+    var m = new WeakMap(), b = Symbol("b");
+    m.set(o, b);
+    m.set(b, 1);
+    assert(m.get(o), b);
+    assert(m.get(b), 1);
+    b = undefined;
+    m = undefined;
+}
+f();
+
+function g() {
+    var o = {};
+    var m = new WeakMap(), a = Symbol("a"), b = Symbol("b"), c = Symbol("c");
+    m.set(o, a);
+    m.set(a, b);
+    m.set(b, c);
+    m.set(c, 1);
+    a = b = c = undefined;
+    m = undefined;
+}
+g();
+
+// freed when the runtime is freed
+const m = new WeakMap();
+const a = Symbol("a"), b = Symbol("b");
+m.set(a, b);
+m.set(b, a);
+assert(m.get(a), b);
+assert(m.get(b), a);