From 69004f73c2926f84f9a1f50905e348df718e69c9 Mon Sep 17 00:00:00 2001 From: Michael Denyer Date: Wed, 15 Jul 2026 00:32:58 +0100 Subject: [PATCH 1/2] tests: kv_alloc re-allocation regression test kv_alloc guards every KVState free with if(k->Lc) precisely so it can be called again on the same KVState (context resize, slot re-init). Exercise that path: allocate, touch the cache, allocate again at a larger size. Fails at this commit with a double-free (fixed in the next commit): malloc: *** error for object 0x7: pointer being freed was not allocated --- c/Makefile | 5 ++++- c/tests/test_kv_alloc.c | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 c/tests/test_kv_alloc.c diff --git a/c/Makefile b/c/Makefile index de8313c..1e0b1ea 100644 --- a/c/Makefile +++ b/c/Makefile @@ -146,7 +146,7 @@ else PYTHON ?= python3 endif CUDA_OBJ = -TEST_BINS = tests/test_json$(EXE) tests/test_st$(EXE) tests/test_tier$(EXE) tests/test_grammar$(EXE) tests/test_schema_gbnf$(EXE) tests/test_decode_batch$(EXE) tests/test_idot$(EXE) tests/test_i4_acc512$(EXE) tests/test_compat_direct$(EXE) +TEST_BINS = tests/test_json$(EXE) tests/test_st$(EXE) tests/test_tier$(EXE) tests/test_grammar$(EXE) tests/test_schema_gbnf$(EXE) tests/test_decode_batch$(EXE) tests/test_idot$(EXE) tests/test_kv_alloc$(EXE) tests/test_i4_acc512$(EXE) tests/test_compat_direct$(EXE) # Windows CUDA DLL path: host links the loader, NOT cudart. ifneq ($(IS_WIN),) @@ -260,6 +260,9 @@ tests/test_decode_batch$(EXE): tests/test_decode_batch.c decode_batch.h tests/test_idot$(EXE): tests/test_idot.c glm.c st.h json.h tok.h tok_unicode.h compat.h grammar.h tier.h $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS) +tests/test_kv_alloc$(EXE): tests/test_kv_alloc.c glm.c st.h json.h tok.h tok_unicode.h compat.h grammar.h tier.h + $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS) + tests/test_i4_acc512$(EXE): tests/test_i4_acc512.c $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS) diff --git a/c/tests/test_kv_alloc.c b/c/tests/test_kv_alloc.c new file mode 100644 index 0000000..40ec00d --- /dev/null +++ b/c/tests/test_kv_alloc.c @@ -0,0 +1,23 @@ +/* kv_alloc must survive re-allocation on the same KVState: every free path is + * guarded by if(k->Lc) precisely so callers (context resize, slot re-init) can + * call it again. A stale duplicate free block frees every Lc[i]/Rc[i] and both + * arrays twice on the second call -> allocator abort. No model file needed: + * the CPU path of kv_alloc only reads c->n_layers/kv_lora/qk_rope. */ +#define main coli_glm_main_unused +#include "../glm.c" +#undef main + +int main(void){ + static Model m; + m.c.n_layers=2; m.c.kv_lora=8; m.c.qk_rope=4; + m.kv=calloc(1,sizeof(KVState)); + kv_alloc(&m,16); + for(int i=0;i Date: Wed, 15 Jul 2026 00:33:45 +0100 Subject: [PATCH 2/2] glm: fix kv_alloc double-free on KV re-allocation (stale pre-Metal free block) kv_alloc had two consecutive if(k->Lc) free blocks. The first freed every k->Lc[i]/k->Rc[i] and both arrays without unregistering from Metal and without nulling k->Lc; the second then re-tested the dangling pointer, called coli_metal_unregister on freed pointers, and freed everything a second time. Safe only when k->Lc is NULL (first call); any re-allocation on the same KVState aborts in the allocator. The first block is the pre-Metal version of the free path: 3716e40 (Metal backend) replaced it with the Metal-aware block, and ec89136 (GPU resident pipeline) re-added it above during the merge. Delete it so the Metal-aware block is the only free path. Caught by tests/test_kv_alloc from the previous commit: before: malloc: *** error for object 0x7: pointer being freed was not allocated (exit 134) after: OK kv_alloc re-allocation (exit 0) --- c/glm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/c/glm.c b/c/glm.c index 8da3f55..6b3b140 100644 --- a/c/glm.c +++ b/c/glm.c @@ -3066,7 +3066,6 @@ static void kv_alloc(Model *m, int max_t){ m->kv_dev_valid[i]=0; } #endif - if(k->Lc){ for(int i=0;in_layers+1;i++){ free(k->Lc[i]); free(k->Rc[i]); } free(k->Lc); free(k->Rc); } if(k->Lc){ for(int i=0;in_layers+1;i++){ #ifdef COLI_METAL if(g_metal_enabled){ coli_metal_unregister(k->Lc[i]); coli_metal_unregister(k->Rc[i]); }