threads / bug / 64612

[BUG] replay: segmentation fault when mistyping target to --onto

Subject: [BUG] replay: segmentation fault when mistyping target to --onto

## tl;dr

4 messages between Dec 11, 2025 and Dec 15, 2025.

replies: 3people: 3as markdown or json

Kristoffer Haugsbakk· Dec 11, 2025, 16:34 UTC · lore
    $ ./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· Dec 11, 2025, 17:56 UTC · re: Kristoffer Haugsbakk · lore

[PATCH] replay: move onto NULL check before first use

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 <kristofferhaugsbakk@fastmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 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· Dec 15, 2025, 10:10 UTC · re: René Scharfe · lore

Re: [PATCH] replay: move onto NULL check before first use

On 11/12/2025 17:56, René Scharfe wrote:
Show 5 quoted lines
> 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
Show 30 quoted lines
> Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>   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· Dec 15, 2025, 12:04 UTC · re: Phillip Wood · lore

Re: [PATCH] replay: move onto NULL check before first use

On Mon, Dec 15, 2025, at 11:10, Phillip Wood wrote:
Show 10 quoted lines
> 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.

← back to recent threads