fix segfault on audit

This commit is contained in:
Emil Lerch 2026-05-02 11:07:17 -07:00
parent fef126471f
commit 67351bc936
Signed by: lobo
GPG key ID: A7B62D657EF764F8

View file

@ -226,14 +226,20 @@ pub fn listCommitsTouching(
var argv: std.ArrayList([]const u8) = .empty;
defer argv.deinit(allocator);
// Track the allocated `--since=...` string so we can free it regardless
// of which index it ends up at in `argv`. (Don't rely on positional
// arithmetic it's brittle and freeing a string literal like "--"
// would segfault on the debug allocator's memset-to-undefined.)
var since_owned: ?[]u8 = null;
defer if (since_owned) |s| allocator.free(s);
try argv.appendSlice(allocator, &.{
"git", "-C", root,
"log", "--format=%H %ct",
});
if (since_iso) |s| {
const since_arg = try std.fmt.allocPrint(allocator, "--since={s}", .{s});
// The string lives inside `argv` until we invoke the child; we
// free it after the child completes. Track it for cleanup.
since_owned = since_arg;
try argv.append(allocator, since_arg);
}
try argv.appendSlice(allocator, &.{ "--", rel_path });
@ -242,18 +248,9 @@ pub fn listCommitsTouching(
.allocator = allocator,
.argv = argv.items,
.max_output_bytes = 16 * 1024 * 1024,
}) catch {
if (since_iso != null) {
// argv.items[6] is the "--since=..." we allocated above.
allocator.free(argv.items[6]);
}
return error.GitUnavailable;
};
}) catch return error.GitUnavailable;
defer allocator.free(result.stdout);
defer allocator.free(result.stderr);
if (since_iso != null) {
allocator.free(argv.items[6]);
}
switch (result.term) {
.Exited => |code| if (code != 0) return error.GitLogFailed,
@ -469,6 +466,21 @@ test "listCommitsTouching returns at least one commit for build.zig" {
try std.testing.expect(commits[0].timestamp > 1_577_836_800);
}
test "listCommitsTouching with non-null since_iso does not segfault" {
// Regression: previously freed argv.items[6] which was the "--"
// string literal when since_iso was non-null, segfaulting on the
// debug allocator's memset-to-undefined.
const allocator = std.testing.allocator;
const info = findRepo(allocator, "build.zig") catch return;
defer allocator.free(info.root);
defer allocator.free(info.rel_path);
// The test is primarily about not segfaulting; we don't assert on
// commits.len since git's --since parsing may decline values that
// are too far back (e.g. "100 years ago" can hit pre-epoch dates).
const commits = listCommitsTouching(allocator, info.root, info.rel_path, "30 years ago") catch return;
defer freeCommitTouches(allocator, commits);
}
test "commitAtOrBeforeDate returns a SHA for a past date" {
const allocator = std.testing.allocator;
const info = findRepo(allocator, "build.zig") catch return;
@ -498,6 +510,34 @@ test "commitAtOrBeforeDate returns null for date before repo existed" {
try std.testing.expect(sha_opt == null);
}
test "commitAtOrBeforeDate: --until=DATE covers end of day, not current time-of-day" {
// Regression test for a subtle git UX trap. `git log --until=DATE`
// with a bare YYYY-MM-DD applies the *current wall-clock
// time-of-day* to DATE, rather than treating DATE as end-of-day.
// So at 10:40am, `--until=2026-04-25` would exclude a commit
// timestamped 2026-04-25 11:13am even though it's clearly within
// 2026-04-25. We work around this by pinning the cutoff to
// 23:59:59 in `commitAtOrBeforeDate`.
//
// This test pins the fix by querying a date far in the past and
// confirming we get a commit (i.e. the helper doesn't over-
// exclude). A more targeted test would need controllable
// committer timestamps, which requires spinning up a tmp repo.
// The behavior is validated end-to-end by compare + contributions
// agreeing on `--since 1W` totals (see src/commands/contributions.zig
// tests).
const allocator = std.testing.allocator;
const info = findRepo(allocator, "build.zig") catch return;
defer allocator.free(info.root);
defer allocator.free(info.rel_path);
// Future-dated cutoff should always return the tip of history
// regardless of current wall-clock time.
const sha_opt = commitAtOrBeforeDate(allocator, info.root, info.rel_path, "2099-01-01") catch return;
try std.testing.expect(sha_opt != null);
if (sha_opt) |s| allocator.free(s);
}
test "resolveCommitRange: legacy clean → HEAD~1..HEAD" {
var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator);
defer arena_state.deinit();