From 99ac84e8ebdbb861cccab8d1576cc8fee85244cd Mon Sep 17 00:00:00 2001 From: jmh530 Date: Tue, 20 Oct 2026 18:00:52 -0400 Subject: [PATCH 1/3] Fix pairwise Summator merges and self-aliasing Preserve the counter maintained by put when merging source partial sums. Adjusting it afterward to count the source's original inputs breaks the destination tree invariant and causes a buffer overrun on the 65th repeated merge of a two-value accumulator. Copy the source for self-addition and self-subtraction so inserting partials cannot overwrite values still needed by the merge. Ordinary merges retain the existing source traversal without a copy. Add float, double, and real regressions for repeated addition and subtraction, varied and empty batches, interleaved scalar insertion, source preservation, tree invariants, and self-merges. Fixes #494. AI-Assisted-By: Codex (GPT-6 Astra Medium) --- source/mir/math/sum.d | 88 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/source/mir/math/sum.d b/source/mir/math/sum.d index c74f106e..9b1e8ab2 100644 --- a/source/mir/math/sum.d +++ b/source/mir/math/sum.d @@ -1430,10 +1430,17 @@ public: else static if (summation == Summation.pairwise) { + // Keep the source stable when insertion also modifies rhs. + if (rhs.partials[] is partials[]) + { + auto source = this; + this += source; + return; + } + // Each source partial is one new leaf in this tree. put updates + // counter to match that tree; rhs.counter counts a different tree. foreach_reverse (e; rhs.partials[0 .. rhs.index]) put(e); - counter -= rhs.index; - counter += rhs.counter; } else static if (summation == Summation.naive) @@ -1527,10 +1534,16 @@ public: else static if (summation == Summation.pairwise) { + // Keep the source stable when insertion also modifies rhs. + if (rhs.partials[] is partials[]) + { + auto source = this; + this -= source; + return; + } + // Count the inserted partials, not the source's original inputs. foreach_reverse (e; rhs.partials[0 .. rhs.index]) put(-e); - counter -= rhs.index; - counter += rhs.counter; } else static if (summation == Summation.naive) @@ -1655,6 +1668,73 @@ public: } } +// Merging source partials must preserve the destination's binary-tree state. +version(mir_test) +@safe pure nothrow @nogc unittest +{ + import core.bitop: popcnt; + + foreach (T; AliasSeq!(float, double, real)) + foreach (op; AliasSeq!("+", "-")) + { + alias S = Summator!(T, Summation.pairwise); + S batch, total; + batch.put(1); + batch.put(1); + foreach (i; 0 .. 130) + { + total.opOpAssign!op(batch); + const expected = cast(T) (2 * (i + 1)); + assert(total.sum == (op == "+" ? expected : -expected)); + assert(total.index == popcnt(total.counter)); + } + assert(batch.sum == 2); + + // Empty, odd-sized and power-of-two batches, with scalar insertions + // between merges. These small integers are exact in all tested types. + total = S.init; + T expected = 0; + foreach (i; 0 .. 1024) + { + batch = S.init; + T batchSum = 0; + foreach (j; 0 .. i % 33) + { + const value = cast(T) (cast(int) (j % 7) - 3); + batch.put(value); + batchSum += value; + } + const sourceCounter = batch.counter; + const sourceIndex = batch.index; + total.opOpAssign!op(batch); + expected += op == "+" ? batchSum : -batchSum; + assert(total.sum == expected); + assert(total.index == popcnt(total.counter)); + assert(batch.sum == batchSum); + assert(batch.counter == sourceCounter && batch.index == sourceIndex); + total.put(2); + expected += 2; + assert(total.sum == expected); + assert(total.index == popcnt(total.counter)); + } + + // A self-merge must read a stable source even when put collapses + // several occupied tree levels during the first insertion. + foreach (length; 0 .. 34) + { + S self; + foreach (j; 0 .. length) + self.put(cast(T) (j + 1)); + const before = self.sum; + self.opOpAssign!op(self); + assert(self.sum == (op == "+" ? 2 * before : 0)); + assert(self.index == popcnt(self.counter)); + self.put(3); + assert(self.sum == (op == "+" ? 2 * before + 3 : 3)); + } + } +} + version(mir_test) unittest { From b60588e960cd742c8417b8049f80880b01e5c311 Mon Sep 17 00:00:00 2001 From: jmh530 Date: Tue, 20 Oct 2026 20:11:28 -0400 Subject: [PATCH 2/3] Cover pairwise Summator underflow after two merges Add a regression using a seven-element source merged twice into a distinct accumulator. The original bookkeeping causes a partial-buffer index underflow on the second merge. Cover addition and subtraction for float, double, and real, checking both accumulated results and preservation of the source sum. AI-Assisted-By: Codex (GPT-6 Astra Medium) --- source/mir/math/sum.d | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/source/mir/math/sum.d b/source/mir/math/sum.d index 9b1e8ab2..70f6b0b2 100644 --- a/source/mir/math/sum.d +++ b/source/mir/math/sum.d @@ -1668,6 +1668,25 @@ public: } } +// A seven-element source previously caused index underflow on the second merge. +version(mir_test) +@safe pure nothrow @nogc unittest +{ + foreach (T; AliasSeq!(float, double, real)) + foreach (op; AliasSeq!("+", "-")) + { + Summator!(T, Summation.pairwise) source, total; + foreach (i; 0 .. 7) + source.put(1); + + total.opOpAssign!op(source); + assert(total.sum == (op == "+" ? 7 : -7)); + total.opOpAssign!op(source); + assert(total.sum == (op == "+" ? 14 : -14)); + assert(source.sum == 7); + } +} + // Merging source partials must preserve the destination's binary-tree state. version(mir_test) @safe pure nothrow @nogc unittest From 90238ee305e59e642bba94c63eb5ef35429581bd Mon Sep 17 00:00:00 2001 From: jmh530 Date: Tue, 20 Oct 2026 20:26:44 -0400 Subject: [PATCH 3/3] Avoid snapshots for single-partial pairwise Summator merges Insert a source's sole partial directly for addition and subtraction, bypassing the alias check and self-merge snapshot. The argument is read before put modifies the destination, so this is safe for self-merges. Retain destination counter bookkeeping and snapshot protection for multi-partial self-merges, without changing the arithmetic order. AI-Assisted-By: Codex (GPT-6 Astra Medium) --- source/mir/math/sum.d | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/source/mir/math/sum.d b/source/mir/math/sum.d index 70f6b0b2..ab6bce1a 100644 --- a/source/mir/math/sum.d +++ b/source/mir/math/sum.d @@ -1430,6 +1430,13 @@ public: else static if (summation == Summation.pairwise) { + // A single partial is read before put can modify it, so even + // a self-merge needs no snapshot. + if (rhs.index == 1) + { + put(rhs.partials[0]); + return; + } // Keep the source stable when insertion also modifies rhs. if (rhs.partials[] is partials[]) { @@ -1534,6 +1541,13 @@ public: else static if (summation == Summation.pairwise) { + // A single partial is read before put can modify it, so even + // a self-merge needs no snapshot. + if (rhs.index == 1) + { + put(-rhs.partials[0]); + return; + } // Keep the source stable when insertion also modifies rhs. if (rhs.partials[] is partials[]) {