{"thread":{"id":"64612","subject":"[BUG] replay: segmentation fault when mistyping target to --onto","startedAt":"2025-12-11T16:34:38Z","lastAt":"2025-12-15T12:05:17Z","messageCount":4,"participants":["Kristoffer Haugsbakk","René Scharfe","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"532051","messageId":"3d83161b-ec34-404a-bb0e-bf4da7ac1db5@app.fastmail.com","threadId":"64612","inReplyTo":null,"subject":"[BUG] replay: segmentation fault when mistyping target to --onto","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-12-11T16:34:17Z","receivedAt":"2025-12-11T16:34:38Z","isPatch":false,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"    $ ./bin-wrappers/git replay --onto=\"$doesntexist\" \"$commit\"'^!'\n    Segmentation fault (core dumped)\n\nI did a bisect starting on current `seen` at a0bdfe7b (Merge branch\n'bc/sha1-256-interop-02' into seen, 2025-12-11). That found the “first\nbad commit” 15cd4ef1 (replay: make atomic ref updates the default\nbehavior, 2025-11-06) (which is on `master`). I started with v2.52.0 as\nthe first known “good”, which I manually checked.\n\nSame segmentation fault on `next` at 674ac2bd (Merge branch\n'kh/doc-send-email-paragraph-fix' into next, 2025-12-10).\n\nThe following is basically the bisect script except I changed it to make\nsense outside my own repo.\n\n```\n#!/bin/sh\n\nmake || exit 125\n\n# Mistyped `seen` for example\ndoesntexist=boh1eixe\n# Current commit for topic kh/doc-pre-commit-fix\ncommit=8cbbdc92f77a20014d9c425c8b9e4af46e492204\n\n./bin-wrappers/git replay --onto=\"$doesntexist\" \"$commit\"'^!'\n\nif test $? = 139\nthen\n    exit 1\nelse\n    # Presumably regular failure:\n    #     fatal: Replaying down to root commit is not supported yet!\n    exit 0\nfi\n```\n"},{"id":"532052","messageId":"9db2b913-b5d6-4617-b079-b4612eaa2b97@web.de","threadId":"64612","inReplyTo":"3d83161b-ec34-404a-bb0e-bf4da7ac1db5@app.fastmail.com","subject":"[PATCH] replay: move onto NULL check before first use","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2025-12-11T17:56:54Z","receivedAt":"2025-12-11T17:57:02Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"cmd_replay() aborts if the pointer \"onto\" is NULL after argument\nparsing, e.g. when specifying a non-existing commit with --onto.\n15cd4ef1f4 (replay: make atomic ref updates the default behavior,\n2025-11-06) added code that dereferences this pointer before the check.\nSwitch their places to avoid a segmentation fault.\n\nReported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/replay.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 507b909df7..64ad2f0f04 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -454,6 +454,9 @@ int cmd_replay(int argc,\n \tdetermine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name,\n \t\t\t      &onto, &update_refs);\n \n+\tif (!onto) /* FIXME: Should handle replaying down to root commit */\n+\t\tdie(\"Replaying down to root commit is not supported yet!\");\n+\n \t/* Build reflog message */\n \tif (advance_name_opt)\n \t\tstrbuf_addf(&reflog_msg, \"replay --advance %s\", advance_name_opt);\n@@ -472,9 +475,6 @@ int cmd_replay(int argc,\n \t\t}\n \t}\n \n-\tif (!onto) /* FIXME: Should handle replaying down to root commit */\n-\t\tdie(\"Replaying down to root commit is not supported yet!\");\n-\n \tif (prepare_revision_walk(&revs) < 0) {\n \t\tret = error(_(\"error preparing revisions\"));\n \t\tgoto cleanup;\n-- \n2.52.0\n"},{"id":"532180","messageId":"a017e50f-7c8f-461f-8627-2fd1445d29f6@gmail.com","threadId":"64612","inReplyTo":"9db2b913-b5d6-4617-b079-b4612eaa2b97@web.de","subject":"Re: [PATCH] replay: move onto NULL check before first use","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-12-15T10:10:27Z","receivedAt":"2025-12-15T10:10:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 11/12/2025 17:56, René Scharfe wrote:\n> cmd_replay() aborts if the pointer \"onto\" is NULL after argument\n> parsing, e.g. when specifying a non-existing commit with --onto.\n> 15cd4ef1f4 (replay: make atomic ref updates the default behavior,\n> 2025-11-06) added code that dereferences this pointer before the check.\n> Switch their places to avoid a segmentation fault.\n\nThis fixes the regression nicely. There is a preexisting bug that we \ntreat an invalid --onto argument the same as a missing argument but that \ncan be fixed separately.\n\nThanks\n\nPhillip\n\n> Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>\n> Signed-off-by: René Scharfe <l.s.r@web.de>\n> ---\n>   builtin/replay.c | 6 +++---\n>   1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin/replay.c b/builtin/replay.c\n> index 507b909df7..64ad2f0f04 100644\n> --- a/builtin/replay.c\n> +++ b/builtin/replay.c\n> @@ -454,6 +454,9 @@ int cmd_replay(int argc,\n>   \tdetermine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name,\n>   \t\t\t      &onto, &update_refs);\n>   \n> +\tif (!onto) /* FIXME: Should handle replaying down to root commit */\n> +\t\tdie(\"Replaying down to root commit is not supported yet!\");\n> +\n>   \t/* Build reflog message */\n>   \tif (advance_name_opt)\n>   \t\tstrbuf_addf(&reflog_msg, \"replay --advance %s\", advance_name_opt);\n> @@ -472,9 +475,6 @@ int cmd_replay(int argc,\n>   \t\t}\n>   \t}\n>   \n> -\tif (!onto) /* FIXME: Should handle replaying down to root commit */\n> -\t\tdie(\"Replaying down to root commit is not supported yet!\");\n> -\n>   \tif (prepare_revision_walk(&revs) < 0) {\n>   \t\tret = error(_(\"error preparing revisions\"));\n>   \t\tgoto cleanup;\n\n"},{"id":"532185","messageId":"a395825a-a9e9-4cde-bf2d-f9b72de9212d@app.fastmail.com","threadId":"64612","inReplyTo":"a017e50f-7c8f-461f-8627-2fd1445d29f6@gmail.com","subject":"Re: [PATCH] replay: move onto NULL check before first use","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-12-15T12:04:54Z","receivedAt":"2025-12-15T12:05:17Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Dec 15, 2025, at 11:10, Phillip Wood wrote:\n> On 11/12/2025 17:56, René Scharfe wrote:\n>> cmd_replay() aborts if the pointer \"onto\" is NULL after argument\n>> parsing, e.g. when specifying a non-existing commit with --onto.\n>> 15cd4ef1f4 (replay: make atomic ref updates the default behavior,\n>> 2025-11-06) added code that dereferences this pointer before the check.\n>> Switch their places to avoid a segmentation fault.\n>\n> This fixes the regression nicely. There is a preexisting bug that we\n> treat an invalid --onto argument the same as a missing argument but that\n> can be fixed separately.\n\nI have a commit cooking (locally) which makes the command die when it\ncannot find commit-ish for `--onto` or `--advance` (whitespace mangled\ndiff):\n\n```\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 507b909df7d..72d62aa34a6 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -39,7 +39,7 @@ static struct commit *peel_committish(struct repository *repo, const char *name)\n \tstruct object_id oid;\n\n \tif (repo_get_oid(repo, name, &oid))\n-\t\treturn NULL;\n+\t\tdie(_(\"'%s' is not a valid commit-ish\"), name);\n \tobj = parse_object(repo, &oid);\n \treturn (struct commit *)repo_peel_to_type(repo, name, 0, obj,\n \t\t\t\t\t\t  OBJ_COMMIT);\n```\n\nInstead of dieing like this:\n\n    Replaying down to root commit is not supported yet!\n\nI hope that doesn’t cause any leak issues when I test it later.\n"}]}