Stale-read detectors for the surviving pools: ASan poisoning + make jit-checked (#1665)

.github/workflows/ci.yml

@@ -395,6 +395,21 @@ jobs:
       - name: JIT vs interpreter differential
         run: bash tools/jit_diff.sh
 
+      # #1665 follow-up: the JIT-tier sections against `make jit-checked`, whose
+      # emitter calls a slot witness before every direct env-slot access and
+      # aborts with the read site on an out-of-range slot or a stale cached
+      # values base (#1661 was such a read). The plan line pins all six
+      # sections as executed, not merely selected.
+      - name: JIT-tier sections under make jit-checked
+        env:
+          EIGS_SUITE_SECTIONS: 82 100 129 130 0fd 0fe
+        run: |
+          set -o pipefail
+          make jit-checked
+          bash tests/run_all_tests.sh 2>&1 | tee "$RUNNER_TEMP/jit-checked.log"
+          grep -q 'PLAN: sections=82,100,129,130,0fd,0fe bearing=6 chunks=6$' "$RUNNER_TEMP/jit-checked.log"
+          grep -qE '^  RESULTS: [0-9]+/[0-9]+ passed, 0 failed, 0 skipped$' "$RUNNER_TEMP/jit-checked.log"
+
       - name: Require compiler guard activity
         if: always()
         run: bash tools/cc_guard.sh --report

Makefile

@@ -77,7 +77,7 @@ define AUX_REFRESH
 	done
 endef
 
-.PHONY: db-params-test all build slot-rehearsal server server-db full http net gfx zlib lib amalgamation tsan test test-changed precheck sandbox-intern-test install install-gfx clean coverage coverage-clean fuzz fuzz-run lsp lsp-asan dap jit-smoke embed-smoke embed-smoke-asan embed-smoke-asan-server embed-smoke-gfx embed-concurrent asan asan-server valgrind pgo poison freestanding-check freestanding-libc-diff asan-http asan-gfx tsan-server tsan-http nativefn-test arming-mt-test embed-roads print-% sigpipe-contract-test sigpipe-partial-test ui-sdl-input-gfx pool-off asan-pool-off pool-off-num pool-off-callenv
+.PHONY: db-params-test all build slot-rehearsal server server-db full http net gfx zlib lib amalgamation tsan test test-changed precheck sandbox-intern-test install install-gfx clean coverage coverage-clean fuzz fuzz-run lsp lsp-asan dap jit-smoke embed-smoke embed-smoke-asan embed-smoke-asan-server embed-smoke-gfx embed-concurrent asan asan-server valgrind pgo poison freestanding-check freestanding-libc-diff asan-http asan-gfx tsan-server tsan-http nativefn-test arming-mt-test embed-roads print-% sigpipe-contract-test sigpipe-partial-test ui-sdl-input-gfx pool-off asan-pool-off pool-off-num pool-off-callenv jit-checked
 
 # ---- Per-variant objdir engine (#740) -------------------------------------
 # The engine's rules are defined before `all`, so pin the default goal.
@@ -178,8 +178,16 @@ SRC_V_pool-off-callenv := $(HOSTED_SOURCES)
 FLAGS_pool-off-callenv := $(CFLAGS) $(DEFS_OFF) -DEIGENSCRIPT_EXT_GFX=1 -DEIGS_POOL_OFF_CALLENV $(VERDEF)
 LIBS_pool-off-callenv  := $(LDFLAGS) -ldl
 
+# #1665 follow-up: release build whose JIT emits a slot witness before every
+# direct env-slot access (GET_LOCAL/SET_LOCAL/LOCAL_DOT_*, the inline EnvIC
+# hit paths): a slot out of range of the env, or a stale cached values base,
+# aborts with the read site (jit.h EIGS_JIT_CHECKED). The default emitter is
+# unchanged; CI runs the JIT-tier sections against this build.
+SRC_V_jit-checked := $(HOSTED_SOURCES)
+FLAGS_jit-checked := $(CFLAGS) $(DEFS_OFF) -DEIGENSCRIPT_EXT_GFX=1 -DEIGS_JIT_CHECKED $(VERDEF)
+LIBS_jit-checked  := $(LDFLAGS) -ldl
 
-VARIANTS := release slot-rehearsal server server-db zlib asan asan-server tsan tsan-server valgrind poison pool-off asan-pool-off pool-off-num pool-off-callenv
+VARIANTS := release slot-rehearsal server server-db zlib asan asan-server tsan tsan-server valgrind poison pool-off asan-pool-off pool-off-num pool-off-callenv jit-checked
 
 # Objects depend on Makefile+VERSION so a flag or version-string change
 # rebuilds; header edits are covered by the generated .d files.
@@ -370,6 +378,16 @@ build/$(INTERN_OWNER_VARIANT)/test_intern_owners: tests/test_intern_owners.c $(I
 intern-owner-test: build/$(INTERN_OWNER_VARIANT)/test_intern_owners
 	@echo "Intern owner structural oracle built: $<"
 
+# #1665 follow-up: planted stale reads of the NUM freelist and the parked call
+# env, against the owning (ASan) variant; no CLI relink. Suite [0fg].
+POOL_POISON_VARIANT ?= asan
+POOL_POISON_OBJ := $(filter-out build/$(POOL_POISON_VARIANT)/main.o,$(OBJ_$(POOL_POISON_VARIANT)))
+build/$(POOL_POISON_VARIANT)/test_pool_poison: tests/test_pool_poison.c $(POOL_POISON_OBJ) $(wildcard $(SRC_DIR)/*.h) Makefile tools/werror_flags.txt
+	$(CC) $(FLAGS_$(POOL_POISON_VARIANT)) -I$(SRC_DIR) -o $@ $< $(POOL_POISON_OBJ) $(LIBS_$(POOL_POISON_VARIANT))
+.PHONY: pool-poison-test
+pool-poison-test: build/$(POOL_POISON_VARIANT)/test_pool_poison
+	@echo "Pool poison plants built: $<"
+
 # #1056: use the same variant as the CLI under test, without relinking it.
 ROAD_VARIANT ?= release
 EMBED_ROADS_OBJ := $(filter-out build/$(ROAD_VARIANT)/main.o,$(OBJ_$(ROAD_VARIANT)))
@@ -661,6 +679,12 @@ pool-off-callenv: build/pool-off-callenv/eigenscript
 	$(call RELINK,pool-off-callenv)
 	@echo "EigenScript $(VERSION) (pool-off C3: call-env recycling off) built. Binary: $(BINARY)"
 
+# JIT slot witnesses (see FLAGS_jit-checked). Run the JIT-tier sections:
+#   make jit-checked && cd tests && EIGS_SUITE_SECTIONS='82 100 129 130 0fd 0fe' bash run_all_tests.sh
+jit-checked: build/jit-checked/eigenscript
+	$(call RELINK,jit-checked)
+	@echo "EigenScript $(VERSION) (jit-checked: emitted slot witnesses) built. Binary: $(BINARY)"
+
 
 # Profile-guided optimization. Builds an instrumented binary, runs the
 # DMG cpu_instrs workload to collect branch/edge counters, then rebuilds

changes/added/1665-pool-stale-read-detectors.md

@@ -0,0 +1,8 @@
+- **Stale-read detectors for the two kept recycling pools (#1665).** ASan
+  builds now poison a NUM parked on the per-thread freelist (all but its link
+  word) and a call env parked on its chunk (its slot, name, count and hash
+  arrays and the Env struct, all but `parent`), so a read of a recycled
+  object reports `use-after-poison` instead of passing silently (#1661 was
+  such a read). `make jit-checked` builds a JIT that checks every emitted
+  direct env-slot access against the env and aborts with the read site; CI
+  runs the JIT-tier sections against it. Release builds are unchanged.

src/chunk.c

@@ -95,6 +95,7 @@ void chunk_decref(EigsChunk *chunk) {
     /* Stage 5i: release the parked call env (not captured, all slots
      * already null; the chunk holds its single ref — env_decref destroys
      * it and drops its owned parent ref). */
+    if (chunk->env_cache) eigs_env_park_unpoison(chunk->env_cache);
     env_decref(chunk->env_cache);
     free(chunk->lines);
     free(chunk->cols);

src/eigenscript.c

@@ -1547,12 +1547,12 @@ int observer_slot_from_trajectory(ObserverSlot *out, Value *dict) {
 void eigs_thread_drain_caches(EigsThread *th) {
     if (!th) return;
 
-    /* num_freelist: each entry's first sizeof(Value*) bytes of data are
-     * overlaid with the next-pointer. Save next, free node, advance. */
+    /* num_freelist: each entry's link word (EIGS_NUM_LINK, inside data) holds
+     * the next-pointer. Save next, free node, advance. */
     Value *vn = th->num_freelist;
     while (vn) {
         Value *next;
-        memcpy(&next, &vn->data, sizeof(Value *));
+        memcpy(&next, EIGS_NUM_LINK(vn), sizeof(Value *));
         free(vn);
         vn = next;
     }
@@ -1625,10 +1625,11 @@ void free_value(Value *v) {
 #ifndef EIGS_POOL_OFF_NUM
         /* Route freed NUMs to freelist for reuse by make_num */
         if (g_num_freelist_count < NUM_FREELIST_CAP) {
-            memcpy(&v->data, &g_num_freelist, sizeof(Value *));
+            memcpy(EIGS_NUM_LINK(v), &g_num_freelist, sizeof(Value *));
             g_num_freelist = v;
             g_num_freelist_count++;
             EIGS_VG_NOACCESS(&v->refcount, sizeof(v->refcount));  /* #297/#298 */
+            eigs_num_park_poison(v);   /* ASan: all but the link word */
             return;
         }
 #endif  /* EIGS_POOL_OFF_NUM (#1665): never park — a refcount-0 NUM is free()d so
@@ -1718,9 +1719,10 @@ Value* make_num(double n) {
 #ifndef EIGS_POOL_OFF_NUM
     if (g_num_freelist) {
         v = g_num_freelist;
-        memcpy(&g_num_freelist, &v->data, sizeof(Value *));
+        memcpy(&g_num_freelist, EIGS_NUM_LINK(v), sizeof(Value *));
         g_num_freelist_count--;
         EIGS_VG_DEFINED(&v->refcount, sizeof(v->refcount));  /* un-poison before reuse */
+        eigs_num_park_unpoison(v);
         memset(v, 0, sizeof(Value));
     } else
 #endif  /* EIGS_POOL_OFF_NUM (#1665): never satisfy a make_num from the freelist —
@@ -1747,10 +1749,11 @@ void recycle_intermediate(Value *v) {
         free(v);
         return;
     }
-    memcpy(&v->data, &g_num_freelist, sizeof(Value *));
+    memcpy(EIGS_NUM_LINK(v), &g_num_freelist, sizeof(Value *));
     g_num_freelist = v;
     g_num_freelist_count++;
     EIGS_VG_NOACCESS(&v->refcount, sizeof(v->refcount));  /* #297/#298 */
+    eigs_num_park_poison(v);   /* ASan: all but the link word */
 #endif
 }
 
@@ -4378,6 +4381,21 @@ static int gc_next_threshold(int live, int last_universe) {
     return (int)t;
 }
 
+#ifdef EIGS_ASAN_POOL_POISON
+/* Re-poison every parked call env the collector unpoisoned during discovery.
+ * Only SURVIVING chunks still own one (a garbage chunk's env_cache was cleared
+ * and freed with it), so after a completed collection only marked chunks are
+ * visited; after an aborted one the graph is unchanged and all are. */
+static void gc_repoison_parked(GcU *u, int survivors_only) {
+    for (int n = 0; n < u->count; n++) {
+        if (u->kind[n] != GC_KIND_CHUNK || (survivors_only && !u->mark[n]))
+            continue;
+        Env *parked = ((EigsChunk *)u->objs[n])->env_cache;
+        if (parked) eigs_env_park_poison(parked);
+    }
+}
+#endif
+
 static void gc_collect_impl(Value **seeds, int seed_count,
                             int include_captured_envs) {
     if (g_in_gc || g_vm_multithreaded) return;
@@ -4408,6 +4426,13 @@ static void gc_collect_impl(Value **seeds, int seed_count,
     for (int n = 0; n < u.count; n++) {
         GC_FOR_EACH_CHILD(&u, n, child, child_kind, {
             gcu_add(&u, child, child_kind);
+#ifdef EIGS_ASAN_POOL_POISON
+            /* A chunk's only Env edge is its parked env_cache: the collector
+             * reads it from here on, so lift the park poison (eigenscript.h);
+             * gc_repoison_parked restores it before returning. */
+            if (child_kind == GC_KIND_ENV && u.kind[n] == GC_KIND_CHUNK)
+                eigs_env_park_unpoison((Env *)child);
+#endif
             /* gcu_add may reallocate the arrays; retain indices, not pointers
              * into them. The target is now present even on a forward edge. */
             int ci = gcu_find(&u, child);
@@ -4440,6 +4465,9 @@ static void gc_collect_impl(Value **seeds, int seed_count,
         uint64_t discovery_work = 0;
         for (int n = 0; n < u.count; n++)
             discovery_work += gc_node_work(u.objs[n], u.kind[n]);
+#ifdef EIGS_ASAN_POOL_POISON
+        gc_repoison_parked(&u, 0);
+#endif
         free(stack);
         free(u.table); free(u.objs); free(u.kind);
         free(u.internal); free(u.pinned); free(u.mark);
@@ -4491,6 +4519,9 @@ static void gc_collect_impl(Value **seeds, int seed_count,
     if (eigs_env_flag("EIGS_GC_DEBUG"))
         fprintf(stderr, "[gc] universe %d, freed %d, live captured %d\n",
                 u.count, garbage, g_gc_captured_live);
+#ifdef EIGS_ASAN_POOL_POISON
+    gc_repoison_parked(&u, 1);
+#endif
 
     free(u.table); free(u.objs); free(u.kind);
     free(u.internal); free(u.pinned); free(u.mark);

src/eigenscript.h

@@ -486,6 +486,98 @@ _Static_assert(sizeof(((Value *)0)->data.str) == sizeof(((Value *)0)->data.strv.
 _Static_assert(sizeof(((Value *)0)->data.strv) <= sizeof(((Value *)0)->data.fn),
                "cached string length must fit in the union without growing it past `fn` on any pointer width — #1183");
 
+/* #1665 follow-up: ASan stale-read detectors for the two recycling pools the
+ * per-layer rule kept (the Value NUM freelist and call-env recycling). A
+ * recycled object stays "allocated" to ASan, so a read of a parked object was
+ * invisible (#1661 was one, tripped only by the JIT). Under AddressSanitizer
+ * the parked object is POISONED while it sits in the pool and unpoisoned when
+ * the pool hands it back out, so a stale read reports
+ * use-after-poison at the reading instruction. Compiled only when the
+ * compiler instruments for ASan; every other build expands these to nothing,
+ * so release objects are byte-identical.
+ *
+ * Parked NUM (free_value / recycle_intermediate -> make_num): the whole Value
+ * is poisoned EXCEPT the link word, the sizeof(Value *) bytes the freelist
+ * threads its next pointer through (EIGS_NUM_LINK). Only the pool's own push,
+ * pop (make_num) and drain (eigs_thread_drain_caches) touch those bytes. In
+ * every other build the link overlays the number itself (offset
+ * offsetof(Value, data)), which would leave a stale NUMBER read -- the
+ * commonest stale read of a recycled NUM -- undetectable; so under ASan the
+ * link moves one double further into the union, to bytes
+ * [offsetof(Value, data) + 8, offsetof(Value, data) + 16) (16..24 on LP64,
+ * the strv.len / list.count+capacity word, meaningless for a NUM). `type`,
+ * the number, `refcount`, the rest of the union and the tail flags are all
+ * poisoned, so a stale v->type check, number read, or incref/decref on a
+ * recycled NUM is reported. Release keeps the old offset: same bytes.
+ *
+ * Parked call env (vm_park_call_env -> vm_take_call_env, chunk->env_cache):
+ * values[], names[], assign_counts[] (capacity entries each), the three hash
+ * arrays (mask + 1 entries each) and the Env struct are poisoned, EXCEPT the
+ * `parent` field, which vm_take_call_env reads to decide whether the parked
+ * env matches the callee's closure. The pool's other legitimate readers --
+ * the cycle collector (it walks chunk -> env_cache as an owned edge) and
+ * chunk_free (it drops the parked ref) -- unpoison before they touch it. */
+#if defined(__SANITIZE_ADDRESS__)
+#  define EIGS_ASAN_POOL_POISON 1
+#elif defined(__has_feature)
+#  if __has_feature(address_sanitizer)
+#    define EIGS_ASAN_POOL_POISON 1
+#  endif
+#endif
+#ifdef EIGS_ASAN_POOL_POISON
+#  define EIGS_NUM_LINK_OFF (offsetof(Value, data) + sizeof(double))
+#else
+#  define EIGS_NUM_LINK_OFF offsetof(Value, data)
+#endif
+/* Address of a parked NUM's freelist link (see above). */
+#define EIGS_NUM_LINK(v) ((void *)((char *)(v) + EIGS_NUM_LINK_OFF))
+_Static_assert(EIGS_NUM_LINK_OFF + sizeof(Value *) <=
+               offsetof(Value, data) + sizeof(((Value *)0)->data),
+               "the NUM freelist link must lie inside the data union");
+#ifdef EIGS_ASAN_POOL_POISON
+#include <sanitizer/asan_interface.h>
+_Static_assert(EIGS_NUM_LINK_OFF % 8 == 0 && sizeof(Value) % 8 == 0,
+               "NUM park poisoning assumes 8-byte ASan granules around the link word");
+static inline void eigs_num_park_poison(Value *v) {
+    size_t link = EIGS_NUM_LINK_OFF, link_end = link + sizeof(Value *);
+    ASAN_POISON_MEMORY_REGION(v, link);
+    ASAN_POISON_MEMORY_REGION((char *)v + link_end, sizeof(Value) - link_end);
+}
+static inline void eigs_num_park_unpoison(Value *v) {
+    ASAN_UNPOISON_MEMORY_REGION(v, sizeof(Value));
+}
+_Static_assert(offsetof(Env, parent) % 8 == 0,
+               "call-env park poisoning keeps `parent` addressable on its own granule");
+static inline void eigs_env_park_mark(Env *e, int poison) {
+    /* Sizes are read from the struct, so poison it LAST and unpoison it
+     * FIRST. */
+    if (!poison) ASAN_UNPOISON_MEMORY_REGION(e, sizeof(Env));
+    size_t cap = e->capacity > 0 ? (size_t)e->capacity : 0;
+    size_t hcap = e->hash.hashes ? (size_t)e->hash.mask + 1 : 0;
+    void *blk[6] = { e->values, e->names, e->assign_counts,
+                     e->hash.hashes, e->hash.indices, e->hash.generations };
+    size_t len[6] = { cap * sizeof(EigsSlot), cap * sizeof(char *), cap * sizeof(int),
+                      hcap * sizeof(uint32_t), hcap * sizeof(int), hcap * sizeof(uint32_t) };
+    for (int i = 0; i < 6; i++) {
+        if (!blk[i] || !len[i]) continue;
+        if (poison) ASAN_POISON_MEMORY_REGION(blk[i], len[i]);
+        else        ASAN_UNPOISON_MEMORY_REGION(blk[i], len[i]);
+    }
+    if (poison) {
+        size_t p = offsetof(Env, parent), p_end = p + sizeof(Env *);
+        ASAN_POISON_MEMORY_REGION(e, p);
+        ASAN_POISON_MEMORY_REGION((char *)e + p_end, sizeof(Env) - p_end);
+    }
+}
+#  define eigs_env_park_poison(e)   eigs_env_park_mark((e), 1)
+#  define eigs_env_park_unpoison(e) eigs_env_park_mark((e), 0)
+#else
+#  define eigs_num_park_poison(v)   ((void)0)
+#  define eigs_num_park_unpoison(v) ((void)0)
+#  define eigs_env_park_poison(e)   ((void)0)
+#  define eigs_env_park_unpoison(e) ((void)0)
+#endif
+
 /* Install a VAL_STR / VAL_JSON_RAW payload. `s` is adopted (the Value frees
  * it) and `n` MUST equal strlen(s). This is the ONLY way to write the payload
  * — `v->data.str` is const, so the compiler refuses the alternative. */