# [BUG] replay: segmentation fault when mistyping target to --onto 4 messages from 2025-12-11 to 2025-12-15. Participants: Kristoffer Haugsbakk, René Scharfe, Phillip Wood. Thread: https://gitlist.dev/t/64612 ## Kristoffer Haugsbakk, 2025-12-11 16:34 Subject: [BUG] replay: segmentation fault when mistyping target to --onto Message-ID: <3d83161b-ec34-404a-bb0e-bf4da7ac1db5@app.fastmail.com> URL: https://gitlist.dev/e/3d83161b-ec34-404a-bb0e-bf4da7ac1db5%40app.fastmail.com ``` $ ./bin-wrappers/git replay --onto="$doesntexist" "$commit"'^!' Segmentation fault (core dumped) I did a bisect starting on current `seen` at a0bdfe7b (Merge branch 'bc/sha1-256-interop-02' into seen, 2025-12-11). That found the “first bad commit” 15cd4ef1 (replay: make atomic ref updates the default behavior, 2025-11-06) (which is on `master`). I started with v2.52.0 as the first known “good”, which I manually checked. Same segmentation fault on `next` at 674ac2bd (Merge branch 'kh/doc-send-email-paragraph-fix' into next, 2025-12-10). The following is basically the bisect script except I changed it to make sense outside my own repo. ˋˋˋ #!/bin/sh make || exit 125 # Mistyped `seen` for example doesntexist=boh1eixe # Current commit for topic kh/doc-pre-commit-fix commit=8cbbdc92f77a20014d9c425c8b9e4af46e492204 ./bin-wrappers/git replay --onto="$doesntexist" "$commit"'^!' if test $? = 139 then exit 1 else # Presumably regular failure: # fatal: Replaying down to root commit is not supported yet! exit 0 fi ˋˋˋ ``` ## René Scharfe, 2025-12-11 17:56 Subject: [PATCH] replay: move onto NULL check before first use Message-ID: <9db2b913-b5d6-4617-b079-b4612eaa2b97@web.de> URL: https://gitlist.dev/e/9db2b913-b5d6-4617-b079-b4612eaa2b97%40web.de In-Reply-To: <3d83161b-ec34-404a-bb0e-bf4da7ac1db5@app.fastmail.com> ``` cmd_replay() aborts if the pointer "onto" is NULL after argument parsing, e.g. when specifying a non-existing commit with --onto. 15cd4ef1f4 (replay: make atomic ref updates the default behavior, 2025-11-06) added code that dereferences this pointer before the check. Switch their places to avoid a segmentation fault. Reported-by: Kristoffer Haugsbakk Signed-off-by: René Scharfe --- builtin/replay.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/replay.c b/builtin/replay.c index 507b909df7..64ad2f0f04 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -454,6 +454,9 @@ int cmd_replay(int argc, determine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name, &onto, &update_refs); + if (!onto) /* FIXME: Should handle replaying down to root commit */ + die("Replaying down to root commit is not supported yet!"); + /* Build reflog message */ if (advance_name_opt) strbuf_addf(&reflog_msg, "replay --advance %s", advance_name_opt); @@ -472,9 +475,6 @@ int cmd_replay(int argc, } } - if (!onto) /* FIXME: Should handle replaying down to root commit */ - die("Replaying down to root commit is not supported yet!"); - if (prepare_revision_walk(&revs) < 0) { ret = error(_("error preparing revisions")); goto cleanup; -- 2.52.0 ``` ## Phillip Wood, 2025-12-15 10:10 Subject: Re: [PATCH] replay: move onto NULL check before first use Message-ID: URL: https://gitlist.dev/e/a017e50f-7c8f-461f-8627-2fd1445d29f6%40gmail.com In-Reply-To: <9db2b913-b5d6-4617-b079-b4612eaa2b97@web.de> ``` On 11/12/2025 17:56, René Scharfe wrote: > cmd_replay() aborts if the pointer "onto" is NULL after argument > parsing, e.g. when specifying a non-existing commit with --onto. > 15cd4ef1f4 (replay: make atomic ref updates the default behavior, > 2025-11-06) added code that dereferences this pointer before the check. > Switch their places to avoid a segmentation fault. This fixes the regression nicely. There is a preexisting bug that we treat an invalid --onto argument the same as a missing argument but that can be fixed separately. Thanks Phillip > Reported-by: Kristoffer Haugsbakk > Signed-off-by: René Scharfe > --- > builtin/replay.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/builtin/replay.c b/builtin/replay.c > index 507b909df7..64ad2f0f04 100644 > --- a/builtin/replay.c > +++ b/builtin/replay.c > @@ -454,6 +454,9 @@ int cmd_replay(int argc, > determine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name, > &onto, &update_refs); > > + if (!onto) /* FIXME: Should handle replaying down to root commit */ > + die("Replaying down to root commit is not supported yet!"); > + > /* Build reflog message */ > if (advance_name_opt) > strbuf_addf(&reflog_msg, "replay --advance %s", advance_name_opt); > @@ -472,9 +475,6 @@ int cmd_replay(int argc, > } > } > > - if (!onto) /* FIXME: Should handle replaying down to root commit */ > - die("Replaying down to root commit is not supported yet!"); > - > if (prepare_revision_walk(&revs) < 0) { > ret = error(_("error preparing revisions")); > goto cleanup; ``` ## Kristoffer Haugsbakk, 2025-12-15 12:04 Subject: Re: [PATCH] replay: move onto NULL check before first use Message-ID: URL: https://gitlist.dev/e/a395825a-a9e9-4cde-bf2d-f9b72de9212d%40app.fastmail.com In-Reply-To: ``` On Mon, Dec 15, 2025, at 11:10, Phillip Wood wrote: > On 11/12/2025 17:56, René Scharfe wrote: >> cmd_replay() aborts if the pointer "onto" is NULL after argument >> parsing, e.g. when specifying a non-existing commit with --onto. >> 15cd4ef1f4 (replay: make atomic ref updates the default behavior, >> 2025-11-06) added code that dereferences this pointer before the check. >> Switch their places to avoid a segmentation fault. > > This fixes the regression nicely. There is a preexisting bug that we > treat an invalid --onto argument the same as a missing argument but that > can be fixed separately. I have a commit cooking (locally) which makes the command die when it cannot find commit-ish for `--onto` or `--advance` (whitespace mangled diff): ˋˋˋ diff --git a/builtin/replay.c b/builtin/replay.c index 507b909df7d..72d62aa34a6 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -39,7 +39,7 @@ static struct commit *peel_committish(struct repository *repo, const char *name) struct object_id oid; if (repo_get_oid(repo, name, &oid)) - return NULL; + die(_("'%s' is not a valid commit-ish"), name); obj = parse_object(repo, &oid); return (struct commit *)repo_peel_to_type(repo, name, 0, obj, OBJ_COMMIT); ˋˋˋ Instead of dieing like this: Replaying down to root commit is not supported yet! I hope that doesn’t cause any leak issues when I test it later. ```