{"thread":{"id":"25258","subject":"rebase -i segmentation fault and another problem","startedAt":"2010-09-27T14:23:27Z","lastAt":"2010-09-28T10:03:40Z","messageCount":4,"participants":["Zbyszek Szmek","Brandon Casey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"151824","messageId":"20100927142327.GW6756@in.waw.pl","threadId":"25258","inReplyTo":null,"subject":"rebase -i segmentation fault and another problem","fromName":"Zbyszek Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2010-09-27T14:23:27Z","receivedAt":"2010-09-27T14:23:27Z","isPatch":false,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"Hi,\nI get a core dump when trying to remove a commit from the tip\nof a branch which was merged with a branch with a different\nroot. Sorry if this is unclear - picture might help:\n\n *-*-m-r\n /\n z-*\n\n(m is merge point, and r is the commit to be removed,\n z is initial commit on the other branch).\n\nA simple 'git reset --hard HEAD^' works, I know. But:\n\n$ git rebase --interactive HEAD~2\nSegmentation fault (core dumped)\n\nCould not apply 532e7e0... z\n\nThe core files comes from 'git cherry-pick ...',\nand indeed it's enough to run this command directly:\n\n$ git-cherry-pick --ff 532e7e0\n[2] 9582 segmentation fault (core dumped) git-cherry-pick --ff 532e7e0\n\ngdb:\nProgram terminated with signal 11, Segmentation fault.\n#0 do_pick_commit (argc=3, argv=) at builtin/revert.c:445\n445 \t\t if (allow_ff && !hashcmp(parent->object.sha1, head))\n\n(gdb) bt\n#0 do_pick_commit (argc=3, argv=) at builtin/revert.c:445\n#1 revert_or_cherry_pick (argc=3, argv=) at builtin/revert.c:576\n#2 0x000000000040499a in run_builtin (argc=3, argv=0x7fff86b20ba8) at git.c:275\n#3 handle_internal_command (argc=3, argv=0x7fff86b20ba8) at git.c:431\n#4 0x0000000000404f9f in main (argc=3, argv=0x7fff86b20ba8) at git.c:516\n\nRepo which shows this behaviour can be created with:\ngit init repo1; cd repo1\ntouch a; git add a; git commit -m a\ncd ..\ngit init repo2; cd repo2\ntouch b; git add b; git commit -m b\ncd ../repo1\ngit pull ../repo2\ntouch c; git add c; git commit -m c\ngit rebase --interactive HEAD~2\n (just save without doing anything -> segmentation fault)\n\n\n$ git --version\ngit version 1.7.3 (compiled from git/master) \n\n--------------------------------------------------------\n\nAnother small problem with git rebase --interactive: in the same repo,\nrunning git rebase HEAD~1 and removing the only commit on the list\ndoesn't do anything (repo is unchanged). The expected behaviour\nwould be to remove the tip of branch...\n\nBest,\nZbyszek\n"},{"id":"151833","messageId":"E2A2pg3JJJ3HO95lSjieK3cGmuaKW6JyYGAV6A_XDmFJCJGyAYa00A@cipher.nrlssc.navy.mil","threadId":"25258","inReplyTo":"20100927142327.GW6756@in.waw.pl","subject":"Re: rebase -i segmentation fault and another problem","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-09-27T16:01:10Z","receivedAt":"2010-09-27T16:01:10Z","isPatch":false,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 09/27/2010 09:23 AM, Zbyszek Szmek wrote:\n> Hi,\n> I get a core dump when trying to remove a commit from the tip\n> of a branch which was merged with a branch with a different\n> root. Sorry if this is unclear - picture might help:\n> \n> *-*-m-r\n> /\n> z-*\n> \n> (m is merge point, and r is the commit to be removed,\n> z is initial commit on the other branch).\n> \n> A simple 'git reset --hard HEAD^' works, I know. But:\n> \n> $ git rebase --interactive HEAD~2\n> Segmentation fault (core dumped)\n> \n> Could not apply 532e7e0... z\n> \n> The core files comes from 'git cherry-pick ...',\n> and indeed it's enough to run this command directly:\n> \n> $ git-cherry-pick --ff 532e7e0\n> [2] 9582 segmentation fault (core dumped) git-cherry-pick --ff 532e7e0\n> \n> gdb:\n> Program terminated with signal 11, Segmentation fault.\n> #0 do_pick_commit (argc=3, argv=) at builtin/revert.c:445\n> 445 \t\t if (allow_ff && !hashcmp(parent->object.sha1, head))\n ^^^^^^\nLooks like gdb points us to exactly where the problem is.\n\"parent\" should be checked for NULL as it is in the 'if' statement\nfollowing this one at line 445.\n\nSince your 'b' commit is a root commit, it has no parents. In the cascading\n'if' statement preceding line 445, parent was set to NULL for this case.\nThen here at line 445, we try to dereference parent to get object.sha1. fail.\n\nThe fix seems to be this (copy/paste):\n\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 4b47ace..57b51e4 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -442,7 +442,7 @@ static int do_pick_commit(void)\n else\n parent = commit->parents->item;\n \n- if (allow_ff && !hashcmp(parent->object.sha1, head))\n+ if (allow_ff && parent && !hashcmp(parent->object.sha1, head))\n return fast_forward_to(commit->object.sha1, head);\n \n if (parent && parse_commit(parent) < 0)\n\n\n\n> (gdb) bt\n> #0 do_pick_commit (argc=3, argv=) at builtin/revert.c:445\n> #1 revert_or_cherry_pick (argc=3, argv=) at builtin/revert.c:576\n> #2 0x000000000040499a in run_builtin (argc=3, argv=0x7fff86b20ba8) at git.c:275\n> #3 handle_internal_command (argc=3, argv=0x7fff86b20ba8) at git.c:431\n> #4 0x0000000000404f9f in main (argc=3, argv=0x7fff86b20ba8) at git.c:516\n> \n> Repo which shows this behaviour can be created with:\n> git init repo1; cd repo1\n> touch a; git add a; git commit -m a\n> cd ..\n> git init repo2; cd repo2\n> touch b; git add b; git commit -m b\n> cd ../repo1\n> git pull ../repo2\n> touch c; git add c; git commit -m c\n> git rebase --interactive HEAD~2\n> (just save without doing anything -> segmentation fault)\n> \n> \n> $ git --version\n> git version 1.7.3 (compiled from git/master) \n\n\nAnd a stacktrace... You included all of the information needed to\ninvestigate and reproduce this issue. Great bug report! Thanks.\n\n> --------------------------------------------------------\n> \n> Another small problem with git rebase --interactive: in the same repo,\n> running git rebase HEAD~1 and removing the only commit on the list\n> doesn't do anything (repo is unchanged). The expected behaviour\n> would be to remove the tip of branch...\n\nThis is actually documented behavior.\n\nRead the last comment in the buffer that rebase --interactive gives\nyou (after the list of commits). It says:\n\n # If you remove a line here THAT COMMIT WILL BE LOST.\n # However, if you remove everything, the rebase will be aborted.\n\n-Brandon\n"},{"id":"151846","messageId":"op5xt5mU5aQhqmO9a_49Hhsxhtg2iW_beFHkzibXMWI534-yRKsOVCvL_25O3xS3R6hTEbD06Kc@cipher.nrlssc.navy.mil","threadId":"25258","inReplyTo":"E2A2pg3JJJ3HO95lSjieK3cGmuaKW6JyYGAV6A_XDmFJCJGyAYa00A@cipher.nrlssc.navy.mil","subject":"[PATCH] builtin/revert.c: don't dereference a NULL pointer","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-27T17:29:45Z","receivedAt":"2010-09-27T17:29:45Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\ncherry-pick will segfault when transplanting a root commit if the --ff\noption is used. This happens because the \"parent\" pointer is set to NULL\nwhen the commit being cherry-picked has no parents. Later, when \"parent\"\nis dereferenced, the cherry-pick segfaults.\n\nFix this by checking whether \"parent\" is NULL before dereferencing it and\nadd a test for this case of cherry-picking a root commit with --ff.\n\nReported-by: Zbyszek Szmek \nSigned-off-by: Brandon Casey \n---\n builtin/revert.c | 2 +-\n t/t3506-cherry-pick-ff.sh | 10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 4b47ace..57b51e4 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -442,7 +442,7 @@ static int do_pick_commit(void)\n \telse\n \t\tparent = commit->parents->item;\n \n-\tif (allow_ff && !hashcmp(parent->object.sha1, head))\n+\tif (allow_ff && parent && !hashcmp(parent->object.sha1, head))\n \t\treturn fast_forward_to(commit->object.sha1, head);\n \n \tif (parent && parse_commit(parent) < 0)\ndiff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\nindex e17ae71..51ca391 100755\n--- a/t/t3506-cherry-pick-ff.sh\n+++ b/t/t3506-cherry-pick-ff.sh\n@@ -95,4 +95,14 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent with --f\n \ttest_must_fail git cherry-pick --ff -m 3 C\n '\n \n+test_expect_success 'cherry pick a root commit with --ff' '\n+\tgit reset --hard first -- &&\n+\tgit rm file1 &&\n+\techo first >file2 &&\n+\tgit add file2 &&\n+\tgit commit --amend -m \"file2\" &&\n+\tgit cherry-pick --ff first &&\n+\ttest \"$(git rev-parse --verify HEAD)\" = \"1df192cd8bc58a2b275d842cede4d221ad9000d1\"\n+'\n+\n test_done\n-- \n1.7.3\n"},{"id":"151891","messageId":"20100928100340.GC6756@in.waw.pl","threadId":"25258","inReplyTo":"op5xt5mU5aQhqmO9a_49Hhsxhtg2iW_beFHkzibXMWI534-yRKsOVCvL_25O3xS3R6hTEbD06Kc@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] builtin/revert.c: don't dereference a NULL pointer","fromName":"Zbyszek Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2010-09-28T10:03:40Z","receivedAt":"2010-09-28T10:03:40Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"Works as advertised. Tests also pass.\n\nThanks,\nZbyszek\n\nOn Mon, Sep 27, 2010 at 12:29:45PM -0500, Brandon Casey wrote:\n> From: Brandon Casey \n> \n> cherry-pick will segfault when transplanting a root commit if the --ff\n> option is used. This happens because the \"parent\" pointer is set to NULL\n> when the commit being cherry-picked has no parents. Later, when \"parent\"\n> is dereferenced, the cherry-pick segfaults.\n> \n> Fix this by checking whether \"parent\" is NULL before dereferencing it and\n> add a test for this case of cherry-picking a root commit with --ff.\n> \n> Reported-by: Zbyszek Szmek \n> Signed-off-by: Brandon Casey \n> ---\n> builtin/revert.c | 2 +-\n> t/t3506-cherry-pick-ff.sh | 10 ++++++++++\n> 2 files changed, 11 insertions(+), 1 deletions(-)\n> \n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 4b47ace..57b51e4 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -442,7 +442,7 @@ static int do_pick_commit(void)\n> \telse\n> \t\tparent = commit->parents->item;\n> \n> -\tif (allow_ff && !hashcmp(parent->object.sha1, head))\n> +\tif (allow_ff && parent && !hashcmp(parent->object.sha1, head))\n> \t\treturn fast_forward_to(commit->object.sha1, head);\n> \n> \tif (parent && parse_commit(parent) < 0)\n> diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\n> index e17ae71..51ca391 100755\n> --- a/t/t3506-cherry-pick-ff.sh\n> +++ b/t/t3506-cherry-pick-ff.sh\n> @@ -95,4 +95,14 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent with --f\n> \ttest_must_fail git cherry-pick --ff -m 3 C\n> '\n> \n> +test_expect_success 'cherry pick a root commit with --ff' '\n> +\tgit reset --hard first -- &&\n> +\tgit rm file1 &&\n> +\techo first >file2 &&\n> +\tgit add file2 &&\n> +\tgit commit --amend -m \"file2\" &&\n> +\tgit cherry-pick --ff first &&\n> +\ttest \"$(git rev-parse --verify HEAD)\" = \"1df192cd8bc58a2b275d842cede4d221ad9000d1\"\n> +'\n> +\n> test_done\n> -- \n> 1.7.3\n"}]}