errdefer coverage

This commit is contained in:
Emil Lerch 2026-07-28 07:06:01 -07:00
parent 6794f34b12
commit 7ce56b46fd
Signed by: lobo
GPG key ID: A7B62D657EF764F8
4 changed files with 358 additions and 50 deletions

View file

@ -577,6 +577,41 @@ the demotion policy at the denominator cap, rational-to-decimal rendering and
rounding, and a regression test per bug in 2.7.1 - including one asserting
`9007199254740993` round-trips, which nothing currently guards.
##### Allocation-failure testing
`errdefer` cleanup paths are not reachable by ordinary tests, and covering them
is worthless on its own: the line exists to *prevent a leak*, so executing it
proves nothing unless the test also asserts that nothing leaked.
`std.testing.FailingAllocator` fails after exactly N allocations. Sweeping N
across an operation's whole allocation sequence, with `testing.allocator`
underneath to detect leaks at test end, turns those paths into real assertions.
(`std.mem.Allocator.failing` fails on the *first* allocation, which only covers
entry-point OOM, not the interesting mid-construction failures.)
This is not a coverage exercise; the sweep immediately found three genuine bugs
in `rational.zig` that ordinary tests could never reach:
1. **A double free.** `initOwned` took its numerator and denominator *by value*
and freed them via `errdefer` on its own failure, while every caller ALSO had
an `errdefer` for the same values. Any allocation failure inside normalization
freed them twice, which segfaulted.
2. **A dangling copy.** Because the parts were passed by value, and normalization
can reallocate limbs, a caller's `errdefer` could free a stale pointer even
without the double free.
Fixed by renaming to `finish` and taking both parts **by pointer**, with the
contract that ownership transfers only on success: on failure the caller's
`errdefer` sees live, current values and frees them exactly once.
3. **Two struct-literal leaks.** `initInt`, `initZero` and `initRatio` built
`.{ .num = try initSet(...), .den = try initSet(...) }` in a single literal.
An `errdefer` cannot cover a value that does not exist yet, so a failure on
the second allocation orphaned the first. Fixed by building the parts as
separate statements, each with its own `errdefer`. `floor` and `factorial`
were also missing an `errdefer` on their denominator.
The remaining uncovered lines in these modules are `unreachable` arms guarded
upstream, and diagnostic paths inside the tests themselves.
---
## 3. Struct Layout Engine

View file

@ -167,9 +167,17 @@ behavioral change (rationale in design.md 2.7.9):
expected value shifts with optimize mode verifies nothing, so the `mod` test now
asserts the values the definition requires instead of deriving them from `@mod`.
Worth reporting upstream.
- Allocation-failure sweeps added using `std.testing.FailingAllocator`
(`fail_index` = fail after N allocations) over `testing.allocator`, so leaks are
detected rather than the cleanup lines merely being executed. These found three
real bugs unreachable by ordinary tests: a double free (`initOwned` freed values
its callers also freed), a potential dangling copy (parts passed by value while
normalization can reallocate limbs), and struct-literal leaks in `initInt` /
`initZero` / `initRatio` where an `errdefer` cannot cover a value that does not
exist yet. `floor` and `factorial` were also missing a denominator `errdefer`.
See design.md 2.7.10. Uncovered lines in the numeric modules went 25 -> 8.
#### 2.0c: `evalString` and the display path move to `Number`
- Mechanical signature churn through evaluator/formatter/CLI/TUI tests. This is
#### 2.0c: `evalString` and the display path move to `Number`- Mechanical signature churn through evaluator/formatter/CLI/TUI tests. This is
the commit where `main.zig`'s output assertions get reviewed.
- Lift the limits this removes: `formatter.is_integer`'s `< 2^53` gate and
`evaluator.factorial`'s `x > 170` rejection (also the source of the misleading

View file

@ -858,3 +858,112 @@ test "max and min with an inexact operand return that operand as-is" {
try testing.expect(!hi.isExact());
try testing.expectEqual(@as(f64, 2.0), hi.toFloat(alloc));
}
// -- Allocation-failure safety --
//
// Mirrors the sweep in rational.zig: fail the Nth allocation for every N, and
// let testing.allocator's leak detection verify that partially-built values are
// released. This is what actually validates the cleanup paths; merely executing
// them proves nothing.
fn oomSweep(comptime body: fn (Allocator) anyerror!void) !void {
var fail_index: usize = 0;
while (fail_index < 512) : (fail_index += 1) {
var failing = std.testing.FailingAllocator.init(alloc, .{ .fail_index = fail_index });
if (body(failing.allocator())) |_| {
return;
} else |err| {
if (err != error.OutOfMemory) return err;
}
}
return error.OomSweepNeverCompleted;
}
fn bodyExactArithmetic(a: Allocator) anyerror!void {
var x = try Number.parse(a, "0.1");
defer x.deinit();
var y = try Number.parse(a, "0.2");
defer y.deinit();
var sum = try Number.add(a, x, y);
defer sum.deinit();
var diff = try Number.sub(a, x, y);
defer diff.deinit();
var prod = try Number.mul(a, x, y);
defer prod.deinit();
var quot = try Number.div(a, x, y);
defer quot.deinit();
var neg = try Number.negate(a, x);
defer neg.deinit();
var magnitude = try Number.abs(a, neg);
defer magnitude.deinit();
_ = try Number.order(a, x, y);
}
fn bodyPowSqrtFactorial(a: Allocator) anyerror!void {
var base = try Number.fromInt(a, 4);
defer base.deinit();
var exp = try Number.fromInt(a, 3);
defer exp.deinit();
var p = try Number.pow(a, base, exp);
defer p.deinit();
var r = try Number.sqrt(a, base);
defer r.deinit();
if (try Number.factorial(a, exp)) |f| {
var value = f;
value.deinit();
}
}
fn bodyRoundingAndSelection(a: Allocator) anyerror!void {
var x = try Number.parse(a, "-3.75");
defer x.deinit();
var y = try Number.fromInt(a, 2);
defer y.deinit();
var f = try Number.floor(a, x);
defer f.deinit();
var c = try Number.ceil(a, x);
defer c.deinit();
var rounded = try Number.round(a, x);
defer rounded.deinit();
var m = try Number.mod(a, x, y);
defer m.deinit();
var hi = try Number.max(a, x, y);
defer hi.deinit();
var lo = try Number.min(a, x, y);
defer lo.deinit();
var copy = try x.clone();
defer copy.deinit();
}
fn bodyRendering(a: Allocator) anyerror!void {
var x = try Number.parse(a, "0.1");
defer x.deinit();
var three = try Number.fromInt(a, 3);
defer three.deinit();
var third = try Number.div(a, x, three);
defer third.deinit();
const d = try third.toDecimalString(a, 12);
a.free(d.text);
if (try third.toFractionString(a)) |frac| a.free(frac);
_ = third.toFloat(a);
}
test "OOM safety: exact arithmetic" {
try oomSweep(bodyExactArithmetic);
}
test "OOM safety: pow, sqrt and factorial" {
try oomSweep(bodyPowSqrtFactorial);
}
test "OOM safety: rounding, mod, max/min and clone" {
try oomSweep(bodyRoundingAndSelection);
}
test "OOM safety: rendering" {
try oomSweep(bodyRendering);
}

View file

@ -39,39 +39,44 @@ pub const Rational = struct {
/// Zero (`0/1`).
pub fn initZero(allocator: Allocator) Error!Rational {
return .{
.num = try Managed.initSet(allocator, 0),
.den = try Managed.initSet(allocator, 1),
};
return initInt(allocator, 0);
}
/// An integer value (`value/1`).
pub fn initInt(allocator: Allocator, value: anytype) Error!Rational {
return .{
.num = try Managed.initSet(allocator, value),
.den = try Managed.initSet(allocator, 1),
};
// Built separately rather than inside a struct literal: if the second
// allocation fails, the first must still be released.
var num = try Managed.initSet(allocator, value);
errdefer num.deinit();
const den = try Managed.initSet(allocator, 1);
return .{ .num = num, .den = den };
}
/// A ratio, reduced on construction. Errors if `den` is zero.
pub fn initRatio(allocator: Allocator, num: anytype, den: anytype) Error!Rational {
var result: Rational = .{
.num = try Managed.initSet(allocator, num),
.den = try Managed.initSet(allocator, den),
};
errdefer result.deinit();
if (result.den.eqlZero()) return Error.DivisionByZero;
try result.normalize(allocator);
return result;
// Built separately rather than inside a struct literal: if the second
// allocation fails, the first must still be released, and an errdefer
// cannot cover a value that does not exist yet.
var n = try Managed.initSet(allocator, num);
errdefer n.deinit();
var d = try Managed.initSet(allocator, den);
errdefer d.deinit();
return finish(allocator, &n, &d);
}
/// Take ownership of two `Managed` values, normalizing them.
fn initOwned(allocator: Allocator, num: Managed, den: Managed) Error!Rational {
var result: Rational = .{ .num = num, .den = den };
errdefer result.deinit();
if (result.den.eqlZero()) return Error.DivisionByZero;
try result.normalize(allocator);
return result;
/// Assemble a reduced `Rational` from a numerator and denominator.
///
/// Ownership transfers ONLY on success. Both parts are taken by pointer and
/// normalized in place, so on failure the caller's `errdefer` still sees
/// live, current values and frees them exactly once.
///
/// Taking them by value would be unsound twice over: the caller's errdefer
/// and this function would both free them (a double free), and normalization
/// can reallocate limbs, which would leave the caller's copy dangling.
fn finish(allocator: Allocator, num: *Managed, den: *Managed) Error!Rational {
if (den.eqlZero()) return Error.DivisionByZero;
try normalizeParts(allocator, num, den);
return .{ .num = num.*, .den = den.* };
}
pub fn deinit(self: *Rational) void {
@ -86,23 +91,23 @@ pub const Rational = struct {
return .{ .num = num, .den = den };
}
/// Reduce by the GCD and force the sign onto the numerator.
fn normalize(self: *Rational, allocator: Allocator) Error!void {
if (self.num.eqlZero()) {
try self.den.set(1);
self.num.setSign(true);
/// Reduce by the GCD and force the sign onto the numerator, in place.
fn normalizeParts(allocator: Allocator, num: *Managed, den: *Managed) Error!void {
if (num.eqlZero()) {
try den.set(1);
num.setSign(true);
return;
}
// Move the sign to the numerator.
if (!self.den.isPositive()) {
self.num.negate();
self.den.negate();
if (!den.isPositive()) {
num.negate();
den.negate();
}
var g = try Managed.init(allocator);
defer g.deinit();
try g.gcd(&self.num, &self.den);
try g.gcd(num, den);
// Nothing to do when already coprime.
if (g.toConst().orderAgainstScalar(1) == .eq) return;
@ -112,10 +117,10 @@ pub const Rational = struct {
var r = try Managed.init(allocator);
defer r.deinit();
try q.divFloor(&r, &self.num, &g);
try self.num.copy(q.toConst());
try q.divFloor(&r, &self.den, &g);
try self.den.copy(q.toConst());
try q.divFloor(&r, num, &g);
try num.copy(q.toConst());
try q.divFloor(&r, den, &g);
try den.copy(q.toConst());
}
// -- Parsing --
@ -201,7 +206,7 @@ pub const Rational = struct {
}
if (negative) num.negate();
return initOwned(allocator, num, den);
return finish(allocator, &num, &den);
}
// -- Predicates --
@ -285,7 +290,7 @@ pub const Rational = struct {
errdefer den.deinit();
try den.mul(&a.den, &b.den);
return initOwned(allocator, num, den);
return finish(allocator, &num, &den);
}
pub fn sub(allocator: Allocator, a: Rational, b: Rational) Error!Rational {
@ -304,7 +309,7 @@ pub const Rational = struct {
errdefer den.deinit();
try den.mul(&a.den, &b.den);
return initOwned(allocator, num, den);
return finish(allocator, &num, &den);
}
pub fn mul(allocator: Allocator, a: Rational, b: Rational) Error!Rational {
@ -316,7 +321,7 @@ pub const Rational = struct {
errdefer den.deinit();
try den.mul(&a.den, &b.den);
return initOwned(allocator, num, den);
return finish(allocator, &num, &den);
}
pub fn div(allocator: Allocator, a: Rational, b: Rational) Error!Rational {
@ -330,7 +335,7 @@ pub const Rational = struct {
errdefer den.deinit();
try den.mul(&a.den, &b.num);
return initOwned(allocator, num, den);
return finish(allocator, &num, &den);
}
pub fn negate(allocator: Allocator, a: Rational) Error!Rational {
@ -374,9 +379,9 @@ pub const Rational = struct {
if (exponent < 0) {
// Reciprocal: swap, then let normalize fix the sign.
return initOwned(allocator, den, num);
return finish(allocator, &den, &num);
}
return initOwned(allocator, num, den);
return finish(allocator, &num, &den);
}
/// Exact square root, or null when the value is not a perfect square of a
@ -416,7 +421,7 @@ pub const Rational = struct {
return null;
}
return try initOwned(allocator, num_root, den_root);
return try finish(allocator, &num_root, &den_root);
}
/// Largest integer not greater than the value.
@ -431,8 +436,9 @@ pub const Rational = struct {
// exactly floor for a positive denominator (an invariant here).
try q.divFloor(&r, &a.num, &a.den);
const den = try Managed.initSet(allocator, 1);
return initOwned(allocator, q, den);
var den = try Managed.initSet(allocator, 1);
errdefer den.deinit();
return finish(allocator, &q, &den);
}
/// Smallest integer not less than the value.
@ -491,8 +497,9 @@ pub const Rational = struct {
defer factor.deinit();
try acc.mul(&acc, &factor);
}
const den = try Managed.initSet(allocator, 1);
return initOwned(allocator, acc, den);
var den = try Managed.initSet(allocator, 1);
errdefer den.deinit();
return finish(allocator, &acc, &den);
}
// -- Conversion --
@ -1430,3 +1437,152 @@ test "factorial: 20 is exact where f64 is already lossy" {
test "factorial: absurd inputs are rejected" {
try testing.expectError(Error.ExponentTooLarge, Rational.factorial(alloc, 20_001));
}
// -- Allocation-failure safety --
//
// Every `errdefer` in this file exists to release a partially-constructed value
// when a LATER allocation fails. Merely executing those lines proves nothing;
// what matters is that no memory leaks when the failure happens.
//
// `std.testing.FailingAllocator` fails after exactly N allocations, so sweeping
// N across an operation's whole allocation sequence drives every intermediate
// failure point. `testing.allocator` sits underneath and reports a leak at the
// end of the test, which is what actually verifies the errdefer chain.
/// Run `body` repeatedly, failing the 0th allocation, then the 1st, and so on,
/// until the operation completes without needing to fail. Any error other than
/// OutOfMemory is a real failure.
fn oomSweep(comptime body: fn (Allocator) anyerror!void) !void {
var fail_index: usize = 0;
while (fail_index < 512) : (fail_index += 1) {
var failing = std.testing.FailingAllocator.init(alloc, .{ .fail_index = fail_index });
if (body(failing.allocator())) |_| {
// Completed without exhausting the budget: the sweep is done.
return;
} else |err| {
if (err != error.OutOfMemory) return err;
}
}
return error.OomSweepNeverCompleted;
}
fn bodyParseDecimal(a: Allocator) anyerror!void {
var x = try Rational.parseDecimal(a, "-1.25e3");
defer x.deinit();
var y = try Rational.parseDecimal(a, "0.1");
defer y.deinit();
}
fn bodyInitRatio(a: Allocator) anyerror!void {
var x = try Rational.initRatio(a, 6, -8);
defer x.deinit();
var y = try x.clone();
defer y.deinit();
}
fn bodyArithmetic(a: Allocator) anyerror!void {
var x = try Rational.parseDecimal(a, "1.5");
defer x.deinit();
var y = try Rational.parseDecimal(a, "2.25");
defer y.deinit();
var sum = try Rational.add(a, x, y);
defer sum.deinit();
var diff = try Rational.sub(a, x, y);
defer diff.deinit();
var prod = try Rational.mul(a, x, y);
defer prod.deinit();
var quot = try Rational.div(a, x, y);
defer quot.deinit();
var neg = try Rational.negate(a, x);
defer neg.deinit();
var magnitude = try Rational.abs(a, neg);
defer magnitude.deinit();
_ = try Rational.order(a, x, y);
}
fn bodyPowAndSqrt(a: Allocator) anyerror!void {
var base = try Rational.initRatio(a, 9, 16);
defer base.deinit();
var p = try Rational.powInt(a, base, 3);
defer p.deinit();
var q = try Rational.powInt(a, base, -2);
defer q.deinit();
if (try Rational.sqrtExact(a, base)) |root| {
var r = root;
r.deinit();
}
var two = try Rational.initInt(a, 2);
defer two.deinit();
// A non-square: exercises the path that allocates roots, discovers they do
// not square back, and releases them before returning null.
try std.testing.expect((try Rational.sqrtExact(a, two)) == null);
}
fn bodyRounding(a: Allocator) anyerror!void {
var x = try Rational.parseDecimal(a, "-3.7");
defer x.deinit();
var f = try Rational.floor(a, x);
defer f.deinit();
var c = try Rational.ceil(a, x);
defer c.deinit();
var r = try Rational.round(a, x);
defer r.deinit();
var three = try Rational.initInt(a, 3);
defer three.deinit();
var m = try Rational.mod(a, x, three);
defer m.deinit();
}
fn bodyFactorial(a: Allocator) anyerror!void {
var f = try Rational.factorial(a, 12);
defer f.deinit();
}
fn bodyRendering(a: Allocator) anyerror!void {
var x = try Rational.initRatio(a, 1, 7);
defer x.deinit();
const frac = try x.toFractionString(a);
a.free(frac);
const repeating = try x.toDecimalString(a, 12);
a.free(repeating.text);
var terminating = try Rational.parseDecimal(a, "0.125");
defer terminating.deinit();
const exact = try terminating.toDecimalString(a, 12);
a.free(exact.text);
_ = try x.isTerminating(a);
_ = x.toFloat(a);
}
test "OOM safety: parseDecimal leaks nothing at any failure point" {
try oomSweep(bodyParseDecimal);
}
test "OOM safety: initRatio and clone" {
try oomSweep(bodyInitRatio);
}
test "OOM safety: the four arithmetic operations plus negate, abs and order" {
try oomSweep(bodyArithmetic);
}
test "OOM safety: powInt and sqrtExact" {
try oomSweep(bodyPowAndSqrt);
}
test "OOM safety: floor, ceil, round and mod" {
try oomSweep(bodyRounding);
}
test "OOM safety: factorial" {
try oomSweep(bodyFactorial);
}
test "OOM safety: decimal and fraction rendering" {
try oomSweep(bodyRendering);
}