sequencer: reject a tag equal to max_sequencer_tags (one-past-the-end write)

[bwhitman]

The bug

sequences is a malloc'd array of max_sequences entries, so the last valid tag is max_sequences - 1. sequencer_add_event() guarded with:

if (tag > max_sequences) {

which lets tag == max_sequences through and writes a whole sequence_info_t — a pointer and two uint32_t — one element past the end of the allocation.

The message the check prints on rejection has always read "is greater than or eq max_sequences", so the intent was never in doubt; only the comparison was.

Verified as a real heap corruption, not a policy question: with the old comparison, tests/test_sequencer_bounds segfaults.

How you'd hit it

Not from the audio suite, which never sends a tag near the ceiling. Not from a well-behaved host either — an allocator that hands out tags below the limit can't produce one. It takes a single hand-written message:

amy.send(sequence="0,16,256")     # with the default max_sequencer_tags of 256

The change

• tag > max_sequences → tag < 0 || tag >= max_sequences.

  The negative half matters too: SEQUENCE_TAG arrives as an unsigned field and lands in an int32_t, so anything past INT32_MAX reads as negative and would have indexed backwards out of the array.

• tests/test_sequencer_bounds.c, added to CTESTS. It checks that tag 0 and tag max−1 are accepted, that max, max+1, a far-out tag and a negative-reading tag are all rejected, and that a completely full table still refuses the one past the end.

Testing

• make ctest — passes with the fix, segfaults without it
• make test — 122 tests pass, sequencer cases all at err=-100.0 dB

One thing worth flagging separately

CI (c-cpp.yml) runs make test but not make ctest, so neither this test nor the existing test_clock_wrap runs there. Worth wiring up, but it touches .github/workflows and needs workflow scope on the token, so I've left it out of this PR.

🤖 Generated with Claude Code

[github-actions]

🎛️ AMY HW CI (AMYboard bench)

Flashed this PR's AMY (LoadTestChord: 6-voice Juno patch=1, one held note every 2 s) onto the physical AMYboard and measured the smoothed render load as the chord grows — back-to-back with the same sketch built at the PR's merge base, so Δ is this PR's own cost.

✅ PASS — the bench ran the test to completion.

notes held	main @ 211ebfb	this PR	Δ
1	1014	1014	+0
2	1184	1179	-5
3	1765	1762	-3
4	1934	1930	-4
5	2562	2558	-4
6	2758	2753	-5

Full chord settled render μs: 2756 (was 2759, Δ -0.1%) (peak 2763, 39 samples)

⬇️ Artifacts: serial log · load trace · report

_{Self-hosted bench (amyboardci). FAIL means only that the test could not run — the load values are informational, with no threshold and no audio compare. See tools/arduino_loadsweep/.}

[bwhitman]

Obsolete after the sequencer rework on main: sequencer_add_event's int32_t tag / tag > max_sequences check is gone. The replacement, sequencer_add_wire(), keeps the tag as uint32_t and rejects with tag >= (uint32_t)max_sequences — that closes both halves of this bug: the off-by-one (tag == max would now land on anonymous slot 0, and the >= check refuses it before that), and the negative-reindex (a tag past INT32_MAX stays a huge unsigned value and fails the same unsigned compare, so it can never index backwards). Verified against main @ 187735a. The bounds regression test moves into the reworked #1017 alongside its active-list test.