{"thread":{"id":"19223","subject":"Segfault during merge","startedAt":"2009-05-07T08:28:27Z","lastAt":"2009-05-09T16:54:30Z","messageCount":13,"participants":["Dave O","Johannes Schindelin","Jakub Narebski","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"113186","messageId":"alpine.DEB.2.00.0905070102010.30999@narbuckle.genericorp.net","threadId":"19223","inReplyTo":null,"subject":"Segfault during merge","fromName":"Dave O","fromEmail":"cxreg@pobox.com","sentAt":"2009-05-07T08:28:27Z","receivedAt":"2009-05-07T08:28:27Z","isPatch":false,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"Hi, I've encountered a particular merge that causes a segfault, and was\nable to successfully bisect git to commit 36e3b5e.  However, I'm unable\nto come up with a simple repro that doesn't involve the source tree that\nI found it on, which unfortunately I'm not able to share.\n\nI've debugged this a bit, and it seems to happen only when there's\nsufficient file creations and deletions to surpass rename_limit in\ndiffcore_rename, and a rename/delete conflict is encountered.  I don't\nknow enough about the index operations that are performed at that point\nto understand why it crashes git, though.\n\nHere's a backtrace:\n\n#0  0x080c071f in sha_eq (a=0x4 <Address 0x4 out of bounds>, b=0x8cd931c \"\\001:??????\\tAR???`\") at cache.h:597\n#1  0x080c1c2d in merge_trees (o=0xbff274d0, head=0x8cd9338, merge=0x8cd9318, common=0x0, result=0xbff27484)\n     at merge-recursive.c:1155\n#2  0x080c3613 in merge_recursive (o=0xbff274d0, h1=0x8ccd3a0, h2=0x8ccd310, ca=0x8d9abb8, result=0xbff27528)\n     at merge-recursive.c:1285\n#3  0x0807b86e in try_merge_strategy (strategy=<value optimized out>, common=0x8d8f798,\n     head_arg=0x80fe14a \"HEAD\") at builtin-merge.c:565\n#4  0x0807cc23 in cmd_merge (argc=1, argv=0xbff28c08, prefix=0x0) at builtin-merge.c:1110\n#5  0x0804b777 in handle_internal_command (argc=2, argv=0xbff28c08) at git.c:247\n#6  0x0804b962 in main (argc=2, argv=0xbff28c08) at git.c:438\n\ncommon=0x0 in the merge_trees() call is the culprit, and that seems to\nhappen when a previous recursive call incurs \"There are unmerged index\nentries\" in write_tree_from_memory, presumably due to the index issue\nreferenced above.\n\nI was able to stop the segfault by copying the \"make an empty tree\"\nblock into the block following the return from recursion where\nmake_virtual_commit is called, but I suspect this is not the correct\nsolution.\n\nPlease let me know how I can further help debug this issue, or possibly\ncome up with a repro that will help someone else debug it.  Thanks!\n\n     Dave Olszewski\n"},{"id":"113199","messageId":"alpine.DEB.1.00.0905071144370.18521@pacific.mpi-cbg.de","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905070102010.30999@narbuckle.genericorp.net","subject":"Re: Segfault during merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-07T09:48:26Z","receivedAt":"2009-05-07T09:48:26Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 7 May 2009, Dave O wrote:\n\n> Hi, I've encountered a particular merge that causes a segfault, and was\n> able to successfully bisect git to commit 36e3b5e.\n\nAs you already found the commit, you could have Cc:ed me already.  It is \npure luck that I did not miss your bug report.\n\n> However, I'm unable to come up with a simple repro that doesn't involve \n> the source tree that I found it on, which unfortunately I'm not able to \n> share.\n\nSigh.\n\n> I've debugged this a bit, and it seems to happen only when there's\n> sufficient file creations and deletions to surpass rename_limit in\n> diffcore_rename, and a rename/delete conflict is encountered.\n\nMaybe you can set merge.renameLimit really small, and then reproduce?\n\nCiao,\nDscho\n"},{"id":"113201","messageId":"m3y6t9md2u.fsf@localhost.localdomain","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905070102010.30999@narbuckle.genericorp.net","subject":"Re: Segfault during merge","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-07T10:17:54Z","receivedAt":"2009-05-07T10:17:54Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dave O <cxreg@pobox.com> writes:\n\n> Hi, I've encountered a particular merge that causes a segfault, and was\n> able to successfully bisect git to commit 36e3b5e.  However, I'm unable\n> to come up with a simple repro that doesn't involve the source tree that\n> I found it on, which unfortunately I'm not able to share.\n\n[...]\n> Please let me know how I can further help debug this issue, or possibly\n> come up with a repro that will help someone else debug it.  Thanks!\n\nCouldn't you use repository anonymizing tool (which replaces all\ncontent, but preserves shape of history) which was published as\nexample some time ago on git mailing list?  Unfortunately I don't\nhave it bookmarked, so you would have to search for it yourself.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"113302","messageId":"alpine.DEB.2.00.0905072131470.30999@narbuckle.genericorp.net","threadId":"19223","inReplyTo":"alpine.DEB.1.00.0905071144370.18521@pacific.mpi-cbg.de","subject":"Re: Segfault during merge","fromName":"Dave O","fromEmail":"cxreg@pobox.com","sentAt":"2009-05-08T04:37:18Z","receivedAt":"2009-05-08T04:37:18Z","isPatch":false,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Thu, 7 May 2009, Johannes Schindelin wrote:\n\n>> Hi, I've encountered a particular merge that causes a segfault, and was\n>> able to successfully bisect git to commit 36e3b5e.\n>\n> As you already found the commit, you could have Cc:ed me already.  It is\n> pure luck that I did not miss your bug report.\n\nSorry, didn't think to do so.\n\nAfter some thought, I was able to come up with a simple script that\nreproduces this crash.  I'm not sure what the policy on this list is for\nattachments, so I've put it at this URL:\n\nhttp://www.genericorp.net/~count/git-merge-crash\n\nIt requires bash and the perl /usr/bin/rename command for simplicity's\nsake.  At the top of the script is a diagram of what it does.\n\nHopefully this helps you identify what's happening here.  Let me know if\nI can help further.\n\n     Dave\n"},{"id":"113368","messageId":"alpine.DEB.1.00.0905082229520.4601@intel-tinevez-2-302","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905072131470.30999@narbuckle.genericorp.net","subject":"[PATCH] Fix segfault in merge-recursive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-08T20:30:32Z","receivedAt":"2009-05-08T20:30:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen there is no \"common\" tree (for whatever reason), we must not\nthrow a segmentation fault.\n\nNoticed by Dave O.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tSorry, did not have much time, so I only fixed the segfault.  \n\tCould you verify that the result is correct?\n\n merge-recursive.c          |   23 +++++--\n t/t3031-merge-criscross.sh |  135 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 151 insertions(+), 7 deletions(-)\n create mode 100755 t/t3031-merge-criscross.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a3721ef..920ccc1 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -176,17 +176,26 @@ static int git_merge_trees(int index_only,\n \t\topts.index_only = 1;\n \telse\n \t\topts.update = 1;\n-\topts.merge = 1;\n-\topts.head_idx = 2;\n \topts.fn = threeway_merge;\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n \n-\tinit_tree_desc_from_tree(t+0, common);\n-\tinit_tree_desc_from_tree(t+1, head);\n-\tinit_tree_desc_from_tree(t+2, merge);\n+\tif (common) {\n+\t\topts.merge = 1;\n+\t\topts.head_idx = 2;\n+\t\tinit_tree_desc_from_tree(t+0, common);\n+\t\tinit_tree_desc_from_tree(t+1, head);\n+\t\tinit_tree_desc_from_tree(t+2, merge);\n+\t\trc = unpack_trees(3, t, &opts);\n+\t}\n+\telse {\n+\t\topts.merge = 0;\n+\t\topts.head_idx = 1;\n+\t\tinit_tree_desc_from_tree(t+0, head);\n+\t\tinit_tree_desc_from_tree(t+1, merge);\n+\t\trc = unpack_trees(2, t, &opts);\n+\t}\n \n-\trc = unpack_trees(3, t, &opts);\n \tcache_tree_free(&active_cache_tree);\n \treturn rc;\n }\n@@ -1152,7 +1161,7 @@ int merge_trees(struct merge_options *o,\n \t\tcommon = shift_tree_object(head, common);\n \t}\n \n-\tif (sha_eq(common->object.sha1, merge->object.sha1)) {\n+\tif (common && sha_eq(common->object.sha1, merge->object.sha1)) {\n \t\toutput(o, 0, \"Already uptodate!\");\n \t\t*result = head;\n \t\treturn 1;\ndiff --git a/t/t3031-merge-criscross.sh b/t/t3031-merge-criscross.sh\nnew file mode 100755\nindex 0000000..525afea\n--- /dev/null\n+++ b/t/t3031-merge-criscross.sh\n@@ -0,0 +1,135 @@\n+#!/bin/sh\n+\n+test_description='merge-recursive backend test'\n+\n+. ./test-lib.sh\n+\n+#         A      <- create some files\n+#        / \\\n+#       B   C    <- cause rename/delete conflicts between B and C\n+#      /     \\\n+# ->  1       1  <- merge-bases for F and G: B1, C1\n+#     |\\     /|\n+#     2 D   E 2\n+#     | |   | |\n+#     | 1   1 |  <- overload rename_limit in E1\n+#     |  \\ /  |\n+#     |   X   |\n+#     |  / \\  |\n+#     | /   \\ |\n+#     |/     \\|\n+#     F       G  <- merge E into B, D into C\n+#      \\     /\n+#       \\   /\n+#        \\ /\n+#         H      <- recursive merge crashes\n+#\n+\n+# initialize\n+test_expect_success 'setup' '\n+\tmkdir data &&\n+\n+\ttest_debug create a bunch of files &&\n+\tn=1 &&\n+\twhile test $n -le 1000\n+\tdo\n+\t\techo $n > data/$n &&\n+\t\tn=$(($n+1)) ||\n+\t\tbreak\n+\tdone &&\n+\n+\ttest_debug check them in &&\n+\tgit add data &&\n+\tgit commit -m A &&\n+\tgit branch A &&\n+\n+\ttest_debug remove some files in one branch &&\n+\tgit checkout -b B A &&\n+\tgit rm data/99* &&\n+\tgit add data &&\n+\tgit commit -m B &&\n+\n+\ttest_debug few more commits on B &&\n+\techo testB > data/testB &&\n+\tgit add data &&\n+\tgit commit -m B1 &&\n+\n+\ttest_debug with a branch off of it &&\n+\tgit branch D &&\n+\n+\techo testB2 > data/testB2 &&\n+\tgit add data &&\n+\tgit commit -m B2 &&\n+\n+\ttest_debug put some commits on D &&\n+\tgit checkout D &&\n+\techo testD > data/testD &&\n+\tgit add data &&\n+\tgit commit -m D &&\n+\n+\techo testD1 > data/testD1 &&\n+\tgit add data &&\n+\tgit commit -m D1 &&\n+\n+\ttest_debug back up to the top, create another branch and cause a rename  &&\n+\ttest_debug conflict with the files we deleted earlier &&\n+\tgit checkout -b C A &&\n+\tgit rm --cached data/99* &&\n+\trename \"s!/9!/moved-9!\" data/99* &&\n+\tgit add data &&\n+\tgit commit -m C &&\n+\n+\ttest_debug few more commits on C &&\n+\techo testC > data/testC &&\n+\tgit add data &&\n+\tgit commit -m C1 &&\n+\n+\ttest_debug with a branch off of it &&\n+\tgit branch E &&\n+\n+\techo testC2 > data/testC2 &&\n+\tgit add data &&\n+\tgit commit -m C2 &&\n+\n+\ttest_debug put a commits on E &&\n+\tgit checkout E &&\n+\techo testE > data/testE &&\n+\tgit add data &&\n+\tgit commit -m E &&\n+\n+\ttest_debug and now, overload add/delete &&\n+\tgit rm data/[123456]* &&\n+\tn=10000 &&\n+\twhile test $n -le 11000\n+\tdo\n+\t\techo $n > data/$n &&\n+\t\tn=$(($n+1)) ||\n+\t\tbreak\n+\tdone &&\n+\tgit add data &&\n+\tgit commit -m E1 &&\n+\n+\n+\ttest_debug now, merge E into B &&\n+\tgit checkout B &&\n+\ttest_must_fail git merge E &&\n+\ttest_debug force-resolve? &&\n+\tgit add data &&\n+\tgit commit -m F &&\n+\tgit branch F &&\n+\n+\n+\ttest_debug and merge D into C &&\n+\tgit checkout C &&\n+\ttest_must_fail git merge D &&\n+\ttest_debug force-resolve? &&\n+\tgit add data &&\n+\tgit commit -m G &&\n+\tgit branch G\n+'\n+\n+test_expect_failure 'now, force a recursive merge between F and G' '\n+\tgit merge F\n+'\n+\n+test_done\n-- \n1.6.2.1.493.g67cf3\n"},{"id":"113372","messageId":"alpine.DEB.2.00.0905081436070.30999@narbuckle.genericorp.net","threadId":"19223","inReplyTo":"alpine.DEB.1.00.0905082229520.4601@intel-tinevez-2-302","subject":"Re: [PATCH] Fix segfault in merge-recursive","fromName":"Dave O","fromEmail":"cxreg@pobox.com","sentAt":"2009-05-08T21:41:21Z","receivedAt":"2009-05-08T21:41:21Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Fri, 8 May 2009, Johannes Schindelin wrote:\n\n>\n> When there is no \"common\" tree (for whatever reason), we must not\n> throw a segmentation fault.\n>\n> Noticed by Dave O.\n\nWhile this patch does prevent a segfault, it totally fails to recognize\nany conflicts in the merge.  Reverting 36e3b5e produces an ordinary\nmerge conflict with some rename/delete conflicts, and others including\ncontent related conflicts.  I'm not sure I wouldn't rather have the\nsegfault than the grossly incorrect automerge.\n\nI'll continue debugging the triggering condition to see if I can\nunderstand why the index is left dirty, leading to this NULL tree.\n\nThanks!\n\n     Dave\n"},{"id":"113375","messageId":"alpine.DEB.1.00.0905090012410.4601@intel-tinevez-2-302","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905081436070.30999@narbuckle.genericorp.net","subject":"Re: [PATCH] Fix segfault in merge-recursive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-08T22:14:15Z","receivedAt":"2009-05-08T22:14:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 8 May 2009, Dave O wrote:\n\n> On Fri, 8 May 2009, Johannes Schindelin wrote:\n> \n> > When there is no \"common\" tree (for whatever reason), we must not \n> > throw a segmentation fault.\n> >\n> > Noticed by Dave O.\n> \n> While this patch does prevent a segfault, it totally fails to recognize \n> any conflicts in the merge.  Reverting 36e3b5e produces an ordinary \n> merge conflict with some rename/delete conflicts, and others including \n> content related conflicts.  I'm not sure I wouldn't rather have the \n> segfault than the grossly incorrect automerge.\n> \n> I'll continue debugging the triggering condition to see if I can \n> understand why the index is left dirty, leading to this NULL tree.\n\nOne thing I realized while trying to quickly fix the issue for you was \nthat the recognized merge base was NULL.  I.e. merge-recursive did _not_ \nfind a merge base.\n\n>From your description, it seemed that it should have found a merge base, \nbut due to too many renames, maybe it did not.\n\nProbably that is the issue.\n\n(Sorry, too tired to do anything about it.)\n\nCiao,\nDscho\n"},{"id":"113380","messageId":"7vocu3p3pr.fsf@alter.siamese.dyndns.org","threadId":"19223","inReplyTo":"alpine.DEB.1.00.0905082229520.4601@intel-tinevez-2-302","subject":"Re: [PATCH] Fix segfault in merge-recursive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-08T23:36:16Z","receivedAt":"2009-05-08T23:36:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> When there is no \"common\" tree (for whatever reason), we must not\n> throw a segmentation fault.\n\nYou described why the old code was wrong (i.e. \"init_tree_desc_from_tree\nis called with common == NULL\"), but there is no mention why the new code\nis correct.  For the purpose of satisfying the above statement, you could\nhave just exit(0) as well ;-)\n\n> +\telse {\n> +\t\topts.merge = 0;\n> +\t\topts.head_idx = 1;\n> +\t\tinit_tree_desc_from_tree(t+0, head);\n> +\t\tinit_tree_desc_from_tree(t+1, merge);\n> +\t\trc = unpack_trees(2, t, &opts);\n> +\t}\n\nThis looks more like a half of branch-switch from HEAD to MERGE, not a\nmerge between HEAD and MERGE as two equal histories.  Shouldn't it be\ndoing a three-way tree merge using an empty tree object as the common\nancestor instead, just like merge_recursive.c::merge_recursive() itself\ndoes?\n"},{"id":"113381","messageId":"alpine.DEB.2.00.0905081624230.30999@narbuckle.genericorp.net","threadId":"19223","inReplyTo":"alpine.DEB.1.00.0905090012410.4601@intel-tinevez-2-302","subject":"Re: [PATCH] Fix segfault in merge-recursive","fromName":"Dave O","fromEmail":"cxreg@pobox.com","sentAt":"2009-05-08T23:54:44Z","receivedAt":"2009-05-08T23:54:44Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Sat, 9 May 2009, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Fri, 8 May 2009, Dave O wrote:\n>\n>> On Fri, 8 May 2009, Johannes Schindelin wrote:\n>>\n>>> When there is no \"common\" tree (for whatever reason), we must not\n>>> throw a segmentation fault.\n>>>\n>>> Noticed by Dave O.\n>>\n>> While this patch does prevent a segfault, it totally fails to recognize\n>> any conflicts in the merge.  Reverting 36e3b5e produces an ordinary\n>> merge conflict with some rename/delete conflicts, and others including\n>> content related conflicts.  I'm not sure I wouldn't rather have the\n>> segfault than the grossly incorrect automerge.\n>>\n>> I'll continue debugging the triggering condition to see if I can\n>> understand why the index is left dirty, leading to this NULL tree.\n>\n> One thing I realized while trying to quickly fix the issue for you was\n> that the recognized merge base was NULL.  I.e. merge-recursive did _not_\n> find a merge base.\n>\n> but due to too many renames, maybe it did not.\n>\n> Probably that is the issue.\n\nThat's not what I witnessed, although it's possible I missed something:\n\n./git-merge-crash: line 127: 29751 Segmentation fault      git merge F\ncount@bokonon:~/git-crash/crash-test$ git merge-base -a F G\n8ffd08037781ab7811f9e7983b87a29ea9ea21d9\n79ac36c0bd8525e087fdb278bac9cabfa655ba47\ncount@bokonon:~/git-crash/crash-test$ git merge-base -a 8ffd080 79ac36c\n03ca38c681cd9f832fe68d30ea2d8dfa54cbaf75\n\nWhat I did find, is that the tree is coming back NULL due to the early\nreturn in write_tree_from_memory(), which in turn is due to\nunmerged_cache() returning true.  If verbosity is up high enough, that\nfunction will indicate that the paths of the unmerged entries are\nexactly the ones affected in the commit I referenced earlier:\n\n   There are unmerged index entries:\n   3 data/moved-99\n   3 data/moved-990\n   [...]\n\nInterestingly, I was able to remove quite a bit of the script and still\ninduce the crash, including the part where it causes too many renames.\nIt appears that all that's needed is a delete/rename conflict in a\nrecursive call.\n\nThe new version is here:\nhttp://genericorp.net/~count/git-merge-crash-shorter\n\nOnce again, I don't really know what the implications of the index\noperations that are happening here are, but the update_stages() call\nin a recursive merge must be doing surprising.\n\n     Dave\n"},{"id":"113390","messageId":"alpine.DEB.2.00.0905082224450.30999@narbuckle.genericorp.net","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905081624230.30999@narbuckle.genericorp.net","subject":"[PATCH] Don't update index while recursing (was Re: Segfault during merge)","fromName":"Dave O","fromEmail":"cxreg@pobox.com","sentAt":"2009-05-09T05:30:56Z","receivedAt":"2009-05-09T05:30:56Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Fri, 8 May 2009, Dave O wrote:\n\n> Once again, I don't really know what the implications of the index\n> operations that are happening here are, but the update_stages() call\n> in a recursive merge must be doing surprising.\n\nAfter writing this, I took another look around merge-recursive.c, and\nrealized that all the calls to update_stages() except this one were\ncareful only to do it when o->call_depth was 0.  This simple patch seems\nto fully rectify the problem.\n\n---\n  merge-recursive.c |   11 ++++++-----\n  1 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a3721ef..f5df9b9 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -933,11 +933,12 @@ static int process_renames(struct merge_options *o,\n  \t\t\t\t       ren1_src, ren1_dst, branch1,\n  \t\t\t\t       branch2);\n  \t\t\t\tupdate_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);\n-\t\t\t\tupdate_stages(ren1_dst, NULL,\n-\t\t\t\t\t\tbranch1 == o->branch1 ?\n-\t\t\t\t\t\tren1->pair->two : NULL,\n-\t\t\t\t\t\tbranch1 == o->branch1 ?\n-\t\t\t\t\t\tNULL : ren1->pair->two, 1);\n+\t\t\t\tif (!o->call_depth)\n+\t\t\t\t\tupdate_stages(ren1_dst, NULL,\n+\t\t\t\t\t\t\tbranch1 == o->branch1 ?\n+\t\t\t\t\t\t\tren1->pair->two : NULL,\n+\t\t\t\t\t\t\tbranch1 == o->branch1 ?\n+\t\t\t\t\t\t\tNULL : ren1->pair->two, 1);\n  \t\t\t} else if (!sha_eq(dst_other.sha1, null_sha1)) {\n  \t\t\t\tconst char *new_path;\n  \t\t\t\tclean_merge = 0;\n-- \n1.6.3.dirty\n"},{"id":"113392","messageId":"alpine.DEB.1.00.0905090947120.27348@pacific.mpi-cbg.de","threadId":"19223","inReplyTo":"7vocu3p3pr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix segfault in merge-recursive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-09T07:48:34Z","receivedAt":"2009-05-09T07:48:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 8 May 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > When there is no \"common\" tree (for whatever reason), we must not\n> > throw a segmentation fault.\n> \n> You described why the old code was wrong (i.e. \"init_tree_desc_from_tree \n> is called with common == NULL\"), but there is no mention why the new \n> code is correct.  For the purpose of satisfying the above statement, you \n> could have just exit(0) as well ;-)\n\nYes, I should have marked this patch as \"please test if the result is what \nyou think it should be\", because I was running out of time and therefore \ncould not properly think things through.\n\n> > +\telse {\n> > +\t\topts.merge = 0;\n> > +\t\topts.head_idx = 1;\n> > +\t\tinit_tree_desc_from_tree(t+0, head);\n> > +\t\tinit_tree_desc_from_tree(t+1, merge);\n> > +\t\trc = unpack_trees(2, t, &opts);\n> > +\t}\n> \n> This looks more like a half of branch-switch from HEAD to MERGE, not a\n> merge between HEAD and MERGE as two equal histories.  Shouldn't it be\n> doing a three-way tree merge using an empty tree object as the common\n> ancestor instead, just like merge_recursive.c::merge_recursive() itself\n> does?\n\nBut of course!\n\nThanks,\nDscho\n"},{"id":"113393","messageId":"alpine.DEB.1.00.0905090954520.27348@pacific.mpi-cbg.de","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905082224450.30999@narbuckle.genericorp.net","subject":"Re: [PATCH] Don't update index while recursing (was Re: Segfault during merge)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-09T07:57:57Z","receivedAt":"2009-05-09T07:57:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 8 May 2009, Dave O wrote:\n\n> On Fri, 8 May 2009, Dave O wrote:\n> \n> > Once again, I don't really know what the implications of the index \n> > operations that are happening here are, but the update_stages() call \n> > in a recursive merge must be doing surprising.\n> \n> After writing this, I took another look around merge-recursive.c, and \n> realized that all the calls to update_stages() except this one were \n> careful only to do it when o->call_depth was 0.  This simple patch seems \n> to fully rectify the problem.\n\nACK.\n\nCould you provide a commit message saying that call_depth > 0 requires \ntrees to be constructed from the files with conflicts and that the stages \nthusly must not be updated?\n\nOh, and you may want to adjust the test I made from your script (you said \nyou made it shorter, but you made the original version shorter, which does \nnot run in the test suite unmodified).\n\nAnd then a Signed-off-by, and you're good to go!\n\nSorry for my lousy attempt to help...\n\nCiao,\nDscho\n"},{"id":"113416","messageId":"7vzldm6wu1.fsf@alter.siamese.dyndns.org","threadId":"19223","inReplyTo":"alpine.DEB.2.00.0905081624230.30999@narbuckle.genericorp.net","subject":"Re: [PATCH] Fix segfault in merge-recursive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-09T16:54:30Z","receivedAt":"2009-05-09T16:54:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave O <cxreg@pobox.com> writes:\n\n> Once again, I don't really know what the implications of the index\n> operations that are happening here are, but the update_stages() call\n> in a recursive merge must be doing surprising.\n\nWhen you are trying to come up with the final result (i.e. depth=0), you\nwant to record how the conflict arose by registering the state of the\ncommon ancestor, your branch and the other branch in the index, hence you\nwant to do update_stages().\n\nWhen you are merging with positive depth, that is because of a criss-cross\nmerge situation.  In such a case, you would need to record the tentative\nresult, with conflict markers and all as if the merge went cleanly, even\nif there are conflicts, in order to write it out as a tree object later to\nbe used as a common ancestor tree.  update_file() calls update_file_flags()\nwith update_cache=1 to signal that the result needs to be written to the\nindex at stage #0 (i.e. merged), and the code should not clobber the index\nfurther by calling update_stages().\n\nYour patch looks correct.  Thanks.\n"}]}