From bac427fbf41ca6551bc2c662c83eb29d3d387ce3 Mon Sep 17 00:00:00 2001 From: Stefan Nitz Date: Sun, 26 Jul 2026 05:43:25 +0200 Subject: [PATCH 1/2] Free nodes_dependant and nodes_left_dependant setupNodesAndTransistors allocates these two arrays but destroyNodesAndTransistors never freed them, leaking 6904 bytes per chip instance. Harmless for cbmbasic, which builds one chip, but measure rebuilds chip state thousands of times via resetChip_test. Verified with valgrind over repeated initAndResetChip/destroyChip cycles: previously "definitely lost: 20,712 bytes in 6 blocks" for three instances, now "All heap blocks were freed -- no leaks are possible". measure's 256-opcode output is byte-identical. --- netlist_sim.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/netlist_sim.c b/netlist_sim.c index 62513e5..56d8c88 100644 --- a/netlist_sim.c +++ b/netlist_sim.c @@ -733,6 +733,8 @@ destroyNodesAndTransistors(state_t *state) free(state->nodes_c1c2s); free(state->nodes_c1c2offset); free(state->dependent_block); + free(state->nodes_dependant); + free(state->nodes_left_dependant); free(state->list1); free(state->list2); free(state->listout_bitmap); From 698338bfa35ba1eb410fe12db1c1e345628e6d24 Mon Sep 17 00:00:00 2001 From: Stefan Nitz Date: Sun, 26 Jul 2026 07:49:52 +0200 Subject: [PATCH 2/2] Narrow the dependant ranges to what was actually written The offsets into dependent_block are prefix sums over the gate counts, so they reserve one slot per gated transistor. add_nodes_dependant() then refuses to insert a node twice and uses fewer, but nothing ever narrowed the ranges to match. So nodes_dependant[i+1] and nodes_left_dependant[i+1] overshoot the entries belonging to node i, and changeNodeValue() reads past the real data into whatever the calloc left there - zero, i.e. node 0 - and queues it for recalculation. On the shipped netlist that is 20 slots across 6 nodes in nodes_dependant and 8 more across 5 nodes in nodes_left_dependant, 28 bogus reads in all. It is harmless only by luck: recalculating node 0 is a no-op, so the bogus work is invisible rather than wrong. Compact both lists once the fill is done. Entries only ever move towards the front, so copying forward in place is safe. measure's 256-opcode characterisation is byte-identical, cbmbasic still reaches READY. in 33155 half-cycles with the same end state, and valgrind reports no errors. --- netlist_sim.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/netlist_sim.c b/netlist_sim.c index 56d8c88..55ec96e 100644 --- a/netlist_sim.c +++ b/netlist_sim.c @@ -655,7 +655,8 @@ setupNodesAndTransistors(netlist_transdefs *transdefs, BOOL *node_is_pullup, nod /* Allocate the dependents block all at once */ state->dependent_block = calloc( block_dep_size, sizeof(*state->nodes_dependant) ); - /* Assign offsets from our block, using only counts needed */ + /* Assign offsets from our block, a slot per gated transistor; the fill + below uses fewer and the ranges are compacted to match afterwards. */ state->nodes_dependant = malloc((nodes+1) * sizeof(*state->nodes_dependant)); nodenum_t dep_index = 0; for (i = 0; i < state->nodes; i++) { @@ -665,7 +666,8 @@ setupNodesAndTransistors(netlist_transdefs *transdefs, BOOL *node_is_pullup, nod } state->nodes_dependant[state->nodes] = dep_index; /* fill the end entry, so we can calculate distances/counts */ - /* Assign offsets from our block, using only counts needed */ + /* Assign offsets from our block, a slot per gated transistor; the fill + below uses fewer and the ranges are compacted to match afterwards. */ state->nodes_left_dependant = malloc((nodes+1) * sizeof(*state->nodes_left_dependant)); for (i = 0; i < state->nodes; i++) { nodenum_t count = nodes_left_dep_count[i]; @@ -696,7 +698,35 @@ setupNodesAndTransistors(netlist_transdefs *transdefs, BOOL *node_is_pullup, nod } } } - + + /* Compact both lists so each node's range ends where the next one begins: + changeNodeValue() reads node i from nodes_dependant[i] up to + nodes_dependant[i+1], while the offsets above reserve a slot per gated + transistor and the fill inserts each dependant only once. Entries only + ever move towards the front, so copying forward in place is safe, and + both lists live in dependent_block, so the second pass carries on where + the first left off. */ + { + nodenum_t w = 0; + for (i = 0; i < state->nodes; i++) { + const nodenum_t r = state->nodes_dependant[i]; + const nodenum_t count = nodes_dep_count[i]; + state->nodes_dependant[i] = w; + for (nodenum_t k = 0; k < count; k++) + state->dependent_block[w++] = state->dependent_block[r + k]; + } + state->nodes_dependant[state->nodes] = w; + + for (i = 0; i < state->nodes; i++) { + const nodenum_t r = state->nodes_left_dependant[i]; + const nodenum_t count = nodes_left_dep_count[i]; + state->nodes_left_dependant[i] = w; + for (nodenum_t k = 0; k < count; k++) + state->dependent_block[w++] = state->dependent_block[r + k]; + } + state->nodes_left_dependant[state->nodes] = w; + } + /* these are unused after initialization */ free(nodes_dep_count); nodes_dep_count = NULL;