Follow-up to #3428, which I closed saying I had not measured how widespread that shape was.
The audit, and how much of it my own matchers got wrong
Of 4692 emitted functions, 67 take a slice parameter and 59 index into one. Classifying what bounds the index took three matcher attempts, and each earlier answer was wrong in a way worth recording:
- "34 write without bounding by
.len()" — true and useless. Most of those bound by an explicit length parameter, which is the only thing that can work in C.
- "3 have a length parameter and do not use it as the bound" — two of the three were false positives.
ternary_shift_right starts its loop at len - 1; my regex looked for while (... len ...) and missed it.
- "24 are bounded by a compile-time constant" — inflated.
TRIT_NEG, TRIT_POS and TRIT_ZERO are values, not bounds, and matching "a declared constant appears in the body" swept in byte_to_trits, which I had already hand-verified as correctly bounded.
Hand-verified, ten functions read by eye: two were matcher false positives, and eight genuinely bound by something other than the buffer. I did not verify all 59, so the class size is somewhere between those eight and the matchers 24, and I am not going to publish a single number for it.
The four bounds actually in use
| bound |
example |
safe in C? |
the buffers own .len() |
pipeline_run after #3429 |
no — C has no length |
| an explicit length parameter |
byte_to_trits(byte, trits, len), mac_parallel_multiply(..., count) |
yes |
| a compile-time constant |
hash_insert bounded by TABLE_SIZE, mem_store by MEM_SIZE, set_insert by SET_MAX |
only if the caller matches it |
| nothing |
buffer_write |
no |
cache_kv deserves its own line: it writes cache_k[position * EMBED_DIM + i] with i < EMBED_DIM, so the index grows with position, and nothing bounds position.
The one that is unambiguous, and is fixed here
fn buffer_write(buf_in: []u8, size: usize, head: usize, data: u8) -> bool {
const new_head = (head + 1) % size;
if (new_head == 0 && head == size - 1) { return false; }
buf_in[head] = data;
head is never compared to size. The evidence that this is an oversight rather than a convention is inside the same file: the sibling buffer_read does guard its index. And its guard is tail == size, which catches exactly one-past-the-end and lets every larger index through — so both are corrected to >=.
The bound uses size, not buf_in.len, deliberately. A []T loses its length at the C ABI, so a .len-based guard exists in Rust and Zig and is absent from C. Using the parameter puts the identical check in all four:
bool buffer_write(uint8_t* buf_in, size_t size, size_t head, uint8_t data) {
if ((head >= size)) { return false; }
pub fn buffer_write(buf_in: &mut [u8], size: usize, head: usize, data: u8) -> bool {
if (head >= size) { return false; }
Left for a decision
The constant-bounded set — hash_insert, hash_remove, mem_store, mem_store_block, set_insert, cache_kv, and the two gla_* — is a convention with an unstated precondition: "your buffer must be at least CONST long". It is coherent, and nothing enforces it in any backend. Changing it means adding length parameters, which changes the ABI.
Follow-up to #3428, which I closed saying I had not measured how widespread that shape was.
The audit, and how much of it my own matchers got wrong
Of 4692 emitted functions, 67 take a slice parameter and 59 index into one. Classifying what bounds the index took three matcher attempts, and each earlier answer was wrong in a way worth recording:
.len()" — true and useless. Most of those bound by an explicit length parameter, which is the only thing that can work in C.ternary_shift_rightstarts its loop atlen - 1; my regex looked forwhile (... len ...)and missed it.TRIT_NEG,TRIT_POSandTRIT_ZEROare values, not bounds, and matching "a declared constant appears in the body" swept inbyte_to_trits, which I had already hand-verified as correctly bounded.Hand-verified, ten functions read by eye: two were matcher false positives, and eight genuinely bound by something other than the buffer. I did not verify all 59, so the class size is somewhere between those eight and the matchers 24, and I am not going to publish a single number for it.
The four bounds actually in use
.len()pipeline_runafter #3429byte_to_trits(byte, trits, len),mac_parallel_multiply(..., count)hash_insertbounded byTABLE_SIZE,mem_storebyMEM_SIZE,set_insertbySET_MAXbuffer_writecache_kvdeserves its own line: it writescache_k[position * EMBED_DIM + i]withi < EMBED_DIM, so the index grows withposition, and nothing boundsposition.The one that is unambiguous, and is fixed here
headis never compared tosize. The evidence that this is an oversight rather than a convention is inside the same file: the siblingbuffer_readdoes guard its index. And its guard istail == size, which catches exactly one-past-the-end and lets every larger index through — so both are corrected to>=.The bound uses
size, notbuf_in.len, deliberately. A[]Tloses its length at the C ABI, so a.len-based guard exists in Rust and Zig and is absent from C. Using the parameter puts the identical check in all four:Left for a decision
The constant-bounded set —
hash_insert,hash_remove,mem_store,mem_store_block,set_insert,cache_kv, and the twogla_*— is a convention with an unstated precondition: "your buffer must be at least CONST long". It is coherent, and nothing enforces it in any backend. Changing it means adding length parameters, which changes the ABI.