bigint add failures with aliasing

[kivikakk]

A Discord user came to us with a failing use of bigint which was just calculating the Fibonacci sequence.

It turns out it was miscalculating on the jump between one and two limbs. See attached a test that fails currently, but should succeed. It is not written very elegantly, but it's easy to verify that it should work.

It looks like the offending code is this optimisation in Mutable.add:

zig/lib/std/math/big/int.zig

Lines 318 to 324 in dce612a

	if (a.limbs.len == 1 and b.limbs.len == 1 and a.positive == b.positive) {
	if (!@addWithOverflow(Limb, a.limbs[0], b.limbs[0], &r.limbs[0])) {
	r.len = 1;
	r.positive = a.positive;
	return;
	}
	}

Note that, if @addWithOverflow does overflow, add continues on to perform Knuth's algorithm in ldadd, but it has also overwritten the contents of r.limbs[0] here. If r is aliased with one of its inputs, this causes the subsequent non-optimised calculation to fail.

I've confirmed the test passes if this is removed. We may want to do an alias check and skip the optimisation if overwriting r with garbage here will be a problem.

edit: I've added a suggested fix in case it's the right one, which skips this @addWithOverflow attempt if r.limbs.ptr is equal to a.limbs.ptr or b.limbs.ptr.

/cc @alexnask fyi

[kivikakk]

CI green (modulo FreeBSD CI outage), should be good now.

[kivikakk]

Our Discord friend has found another bug of the same class, so marking as a draft until that's fixed too. Seems to happen when we hit the 2¹⁹² limit, again only with aliasing.

[kivikakk]

zig/lib/std/math/big/int.zig

Lines 1699 to 1707 in 749c3f0

	/// r, a and b may be aliases.
	///
	/// Returns an error if memory could not be allocated.
	pub fn add(r: *Managed, a: Const, b: Const) Allocator.Error!void {
	try r.ensureCapacity(math.max(a.limbs.len, b.limbs.len) + 1);
	var m = r.toMutable();
	m.add(a, b);
	r.setMetadata(m.positive, m.len);
	}

If r is aliased with a or b and r.ensureCapacity needs to realloc:

zig/lib/std/math/big/int.zig

Lines 1472 to 1477 in 749c3f0

	pub fn ensureCapacity(self: *Managed, capacity: usize) !void {
	if (capacity <= self.limbs.len) {
	return;
	}
	self.limbs = try self.allocator.realloc(self.limbs, capacity);
	}

The memory pointed to by the alias is no longer valid.

[kivikakk]

Ref #6167 (comment).

[kivikakk]

CI green again. add is now a lot more correct. Other functions probably still need work.

[kivikakk]

Rebased and brought back up to date.