* [BUG] replay: segmentation fault when mistyping target to --onto
@ 2025-12-11 16:34 Kristoffer Haugsbakk
2025-12-11 17:56 ` [PATCH] replay: move onto NULL check before first use René Scharfe
0 siblings, 1 reply; 4+ messages in thread
From: Kristoffer Haugsbakk @ 2025-12-11 16:34 UTC (permalink / raw)
To: git; +Cc: Siddharth Asthana
$ ./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
```
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] replay: move onto NULL check before first use
2025-12-11 16:34 [BUG] replay: segmentation fault when mistyping target to --onto Kristoffer Haugsbakk
@ 2025-12-11 17:56 ` René Scharfe
2025-12-15 10:10 ` Phillip Wood
0 siblings, 1 reply; 4+ messages in thread
From: René Scharfe @ 2025-12-11 17:56 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git; +Cc: Siddharth Asthana
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] replay: move onto NULL check before first use
2025-12-11 17:56 ` [PATCH] replay: move onto NULL check before first use René Scharfe
@ 2025-12-15 10:10 ` Phillip Wood
2025-12-15 12:04 ` Kristoffer Haugsbakk
0 siblings, 1 reply; 4+ messages in thread
From: Phillip Wood @ 2025-12-15 10:10 UTC (permalink / raw)
To: René Scharfe, Kristoffer Haugsbakk, git; +Cc: Siddharth Asthana
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 <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;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] replay: move onto NULL check before first use
2025-12-15 10:10 ` Phillip Wood
@ 2025-12-15 12:04 ` Kristoffer Haugsbakk
0 siblings, 0 replies; 4+ messages in thread
From: Kristoffer Haugsbakk @ 2025-12-15 12:04 UTC (permalink / raw)
To: Phillip Wood, René Scharfe, git; +Cc: Siddharth Asthana
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.
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-12-15 12:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-11 16:34 [BUG] replay: segmentation fault when mistyping target to --onto Kristoffer Haugsbakk
2025-12-11 17:56 ` [PATCH] replay: move onto NULL check before first use René Scharfe
2025-12-15 10:10 ` Phillip Wood
2025-12-15 12:04 ` Kristoffer Haugsbakk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox