{"thread":{"id":"6119","subject":"Segfault in xdl_merge is back","startedAt":"2006-12-27T04:16:44Z","lastAt":"2007-01-02T21:17:43Z","messageCount":15,"participants":["Shawn Pearce","Linus Torvalds","Johannes Schindelin","Alexandre Julliard","Junio C Hamano","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"30326","messageId":"20061227041644.GA22449@spearce.org","threadId":"6119","inReplyTo":null,"subject":"Segfault in xdl_merge is back","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-27T04:16:44Z","receivedAt":"2006-12-27T04:16:44Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"So I've been able to reproduce the segfault that was earlier reported\nin xdl_merge.  Unfortunately its in the repo that I can't publish.\nI plan to spend some time tomorrow evening to attempt to further\ndebug the problem.  I would certainly appreciate any advice.  :-)\n\nI can say its *not* related to 1510fea7 (or its fix 5caf9232 for\nthat matter).  Tonight I only had a few minutes to look at the issue\nbut reverting 1510fea7/5caf9232 does not fix the segfault on Cygwin,\neven though 5caf9232 appears to have fixed the issue for the original\nreporter on Linux.\n\nIf I recall it correctly we were segfaulting on line 197 of xmerge.c:\n\n    197         t1.ptr = (char *)xe1->xdf2.recs[m->i1]->ptr;\n\naccording to my particular case m->i1 == 70, but it looks like\nxdf2.recs isn't that large as index 70 is not a valid pointer.\n\nI'm suspecting this is actually some sort of memory corruption in\nthe heap (due to a bad malloc/free) as the bug seems to rear its\nhead only based on the data we are allocating/have allocated.\n\nIf you look at what 1510fea7 would do on Linux the 1510fea7 bug\nwould send us into the else case of diff_populate_filespec where we\nmalloc the file data during decompression from the pack.  Yet when\nwe fixed it with 5caf9232 we started to mmap the working tree file,\navoiding the malloc/free.  This behavior, plus the fact that it\nhappens no matter what for a particular merge on Cygwin (but not\nother merges), leads me to suspect heap corruption.\n\nI may try to bisect this on Cygwin, but I may need to go all the way\nback to pre-xdl_merge() to get a working merge-recursive, and I may\njust find the bug pointing at the original merge-recursive code,\nor just find it pointing at a random commit like what happened\nwith 1510fea7.  So bisection may not really help out very much.\n\nHas anyone run merge-recursive through Valgrind lately?  I don't\nhave a setup handy to run it through and see if we have any obvious\nerrors.\n\n-- \nShawn.\n"},{"id":"30328","messageId":"Pine.LNX.4.64.0612262245250.4473@woody.osdl.org","threadId":"6119","inReplyTo":"20061227041644.GA22449@spearce.org","subject":"Re: Segfault in xdl_merge is back","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-12-27T06:49:23Z","receivedAt":"2006-12-27T06:49:23Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 26 Dec 2006, Shawn Pearce wrote:\n> \n> I'm suspecting this is actually some sort of memory corruption in\n> the heap (due to a bad malloc/free) as the bug seems to rear its\n> head only based on the data we are allocating/have allocated.\n\nCan you try to reproduce it under Linux and use \"valgrind\" to run the \nthing that SIGSEGV's? That tends to be a pretty good way to debug bad \nallocations..\n\nSo instead of bisecting it on cygwin, try to use the build that broke on \nLinux too (ie undo the 5caf9232 \"fix\") and when you can reproduce it under \nLinux, compiel withour -O2 and with debug information, and gdb will be a \nlot more useful, but also run it with valgrind..\n\n\t\tLinus\n"},{"id":"30337","messageId":"20061227082440.GC23022@spearce.org","threadId":"6119","inReplyTo":"Pine.LNX.4.64.0612262245250.4473@woody.osdl.org","subject":"Re: Segfault in xdl_merge is back","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-27T08:24:40Z","receivedAt":"2006-12-27T08:24:40Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> wrote:\n> On Tue, 26 Dec 2006, Shawn Pearce wrote:\n> > \n> > I'm suspecting this is actually some sort of memory corruption in\n> > the heap (due to a bad malloc/free) as the bug seems to rear its\n> > head only based on the data we are allocating/have allocated.\n> \n> Can you try to reproduce it under Linux and use \"valgrind\" to run the \n> thing that SIGSEGV's? That tends to be a pretty good way to debug bad \n> allocations..\n> \n> So instead of bisecting it on cygwin, try to use the build that broke on \n> Linux too (ie undo the 5caf9232 \"fix\") and when you can reproduce it under \n> Linux, compiel withour -O2 and with debug information, and gdb will be a \n> lot more useful, but also run it with valgrind..\n\nGood advice.  Unfortunately it may be difficult to get valgrind onto\na Linux system where I can also put that repository which is failing.\n\nJunio suggested that I try running git-merge-file on the three\nblobs that I'm segfaulting on.  That's a pretty quick test.\n\nFailing that I may have to find a way to get valgrind onto a Linux\nsystem - but that could take a month or more.  (The UNIX admins\nare overworked and don't care much about Linux, and I don't have\nmy own Linux system there.)\n\nI'd really like to get this segfault fixed before 1.5.0 ships. Its\na rather nasty bug and I suspect the culprit is already in 1.5.0-rc0.\n\n-- \nShawn.\n"},{"id":"30344","messageId":"Pine.LNX.4.63.0612271214120.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6119","inReplyTo":"20061227041644.GA22449@spearce.org","subject":"Re: Segfault in xdl_merge is back","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-27T11:22:01Z","receivedAt":"2006-12-27T11:22:01Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 26 Dec 2006, Shawn Pearce wrote:\n\n> So I've been able to reproduce the segfault that was earlier reported in \n> xdl_merge.  Unfortunately its in the repo that I can't publish.\n\nSo here I am again, wanting to help, being unable to. *Sigh*. At least \nthis time it is due to legal reasons, not unknown ones.\n\nAs you said in another mail, Junio suggested using git-merge-file on the \nblobs themselves. This is a little tricky, since git-merge-file does not \nread blobs; only files. I'd do this:\n\n\t- in merge-recursive.c just before line 658 I'd add an\n\t  fprintf(stderr, \"xdl_merge: %s %s %s\\n\", sha1_to_hex(a->sha1),\n\t\tsha1_to_hex(o->sha1), sha1_to_hex(b->sha1));\n\t- run the merge until it segfaults\n\t- get the blobs by using the last three SHA1's in the output by\n\t  $ git-show <sha1>  > a   # or \"o\" or \"b\"\n\t- $ git merge-file -p a o b >/dev/null\n\nThis command line ensures that \"a\" is not edited, and you can repeat the \nmerge as often as needed.\n\nIf this still segfaults, I'd like to have the files privately (I will not \nlook at the contents, as they are irrelevant to this particular bug).\n\nCiao,\nDscho\n"},{"id":"30349","messageId":"87fyb11ouy.fsf@wine.dyndns.org","threadId":"6119","inReplyTo":"Pine.LNX.4.63.0612271214120.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: Segfault in xdl_merge is back","fromName":"Alexandre Julliard","fromEmail":"julliard@winehq.org","sentAt":"2006-12-27T12:53:57Z","receivedAt":"2006-12-27T12:53:57Z","isPatch":false,"sender":{"key":"julliard@winehq.org","avatar":null},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> As you said in another mail, Junio suggested using git-merge-file on the \n> blobs themselves. This is a little tricky, since git-merge-file does not \n> read blobs; only files. I'd do this:\n>\n> \t- in merge-recursive.c just before line 658 I'd add an\n> \t  fprintf(stderr, \"xdl_merge: %s %s %s\\n\", sha1_to_hex(a->sha1),\n> \t\tsha1_to_hex(o->sha1), sha1_to_hex(b->sha1));\n> \t- run the merge until it segfaults\n> \t- get the blobs by using the last three SHA1's in the output by\n> \t  $ git-show <sha1>  > a   # or \"o\" or \"b\"\n> \t- $ git merge-file -p a o b >/dev/null\n>\n> This command line ensures that \"a\" is not edited, and you can repeat the \n> merge as often as needed.\n\nI'm seeing the problem here on Linux, I have attached a tarball with\na set of files from Wine that trigger the bug.\n\nThe gdb backtrace looks like this:\n\nCore was generated by `git-merge-file -p a o b'.\nProgram terminated with signal 11, Segmentation fault.\n#0  0x080b60be in xdl_refine_conflicts (xe1=0xbfcaf22c, xe2=0xbfcaf1a4, m=0x8106668, xpp=0xbfcaf348) at xdiff/xmerge.c:197\n197                     t1.ptr = (char *)xe1->xdf2.recs[m->i1]->ptr;\n(gdb) bt\n#0  0x080b60be in xdl_refine_conflicts (xe1=0xbfcaf22c, xe2=0xbfcaf1a4, m=0x8106668, xpp=0xbfcaf348) at xdiff/xmerge.c:197\n#1  0x080b6843 in xdl_do_merge (xe1=0xbfcaf22c, xscr1=0x0, name1=0xbfcafbcf \"a\", xe2=0xbfcaf1a4, xscr2=0x0, name2=0xbfcafbd3 \"b\", level=2, xpp=0xbfcaf348, result=0xbfcaf34c) at xdiff/xmerge.c:351\n#2  0x080b6b8c in xdl_merge (orig=0xbfcaf35c, mf1=0xbfcaf354, name1=0xbfcafbcf \"a\", mf2=0xbfcaf364, name2=0xbfcafbd3 \"b\", xpp=0xbfcaf348, level=2, result=0xbfcaf34c) at xdiff/xmerge.c:408\n#3  0x08067887 in cmd_merge_file (argc=4, argv=0xbfcaf4b8, envp=0x0) at builtin-merge-file.c:43\n#4  0x0804b0e5 in handle_internal_command (argc=5, argv=0xbfcaf4b4, envp=0xbfcaf4cc) at git.c:294\n#5  0x0804b1e7 in main (argc=5, argv=0xbfcaf4b4, envp=0xbfcaf4cc) at git.c:331\n(gdb) p *xe1\n$1 = {xdf1 = {rcha = {head = 0x8105ca8, tail = 0x8107558, isize = 16, nsize = 1056, ancur = 0x8107558, sncur = 0x0, scurr = 0}, nrec = 259, hbits = 9, rhash = 0x81054a0, dstart = 36, dend = 257, \n    recs = 0x8105080, rchg = 0x80ff009 \"\", rindex = 0x8107988, nreff = 18, ha = 0x8107da0}, xdf2 = {rcha = {head = 0x8108370, tail = 0x81085b0, isize = 16, nsize = 176, ancur = 0x81085b0, sncur = 0x0, \n      scurr = 0}, nrec = 41, hbits = 6, rhash = 0x8108268, dstart = 36, dend = 39, recs = 0x81081b8, rchg = 0x80ff119 \"\", rindex = 0x8108670, nreff = 3, ha = 0x8108720}}\n(gdb) p *xe2\n$2 = {xdf1 = {rcha = {head = 0x81060d8, tail = 0x810aad0, isize = 16, nsize = 1056, ancur = 0x810aad0, sncur = 0x0, scurr = 0}, nrec = 259, hbits = 9, rhash = 0x8104878, dstart = 44, dend = 43, \n    recs = 0x8106b18, rchg = 0x8106509 \"\", rindex = 0x810b998, nreff = 0, ha = 0x810bdb0}, xdf2 = {rcha = {head = 0x810ce20, tail = 0x810db40, isize = 16, nsize = 1104, ancur = 0x810db40, sncur = 0x0, \n      scurr = 0}, nrec = 270, hbits = 9, rhash = 0x810c618, dstart = 44, dend = 54, recs = 0x810c1c8, rchg = 0x8106f39 \"\", rindex = 0x810dfa0, nreff = 8, ha = 0x810e3e0}}\n(gdb) p *m\n$3 = {next = 0x0, mode = 0, i1 = 41, i2 = 41, chg1 = 0, chg2 = 229}\n\n\nAnd here's the Valgrind output:\n\n==32758== Memcheck, a memory error detector.\n==32758== Copyright (C) 2002-2006, and GNU GPL'd, by Julian Seward et al.\n==32758== Using LibVEX rev 1658, a library for dynamic binary translation.\n==32758== Copyright (C) 2004-2006, and GNU GPL'd, by OpenWorks LLP.\n==32758== Using valgrind-3.2.1-Debian, a dynamic binary instrumentation framework.\n==32758== Copyright (C) 2000-2006, and GNU GPL'd, by Julian Seward et al.\n==32758== For more details, rerun with: -v\n==32758== \n==32758== Use of uninitialised value of size 4\n==32758==    at 0x80B60BE: xdl_refine_conflicts (xmerge.c:197)\n==32758==    by 0x80B6842: xdl_do_merge (xmerge.c:351)\n==32758==    by 0x80B6B8B: xdl_merge (xmerge.c:408)\n==32758==    by 0x8067886: cmd_merge_file (builtin-merge-file.c:43)\n==32758==    by 0x804B0E4: handle_internal_command (git.c:294)\n==32758==    by 0x804B1E6: main (git.c:331)\n==32758== \n==32758== Invalid read of size 4\n==32758==    at 0x80B60BE: xdl_refine_conflicts (xmerge.c:197)\n==32758==    by 0x80B6842: xdl_do_merge (xmerge.c:351)\n==32758==    by 0x80B6B8B: xdl_merge (xmerge.c:408)\n==32758==    by 0x8067886: cmd_merge_file (builtin-merge-file.c:43)\n==32758==    by 0x804B0E4: handle_internal_command (git.c:294)\n==32758==    by 0x804B1E6: main (git.c:331)\n==32758==  Address 0x4 is not stack'd, malloc'd or (recently) free'd\n==32758== \n==32758== Process terminating with default action of signal 11 (SIGSEGV): dumping core\n==32758==  Access not within mapped region at address 0x4\n==32758==    at 0x80B60BE: xdl_refine_conflicts (xmerge.c:197)\n==32758==    by 0x80B6842: xdl_do_merge (xmerge.c:351)\n==32758==    by 0x80B6B8B: xdl_merge (xmerge.c:408)\n==32758==    by 0x8067886: cmd_merge_file (builtin-merge-file.c:43)\n==32758==    by 0x804B0E4: handle_internal_command (git.c:294)\n==32758==    by 0x804B1E6: main (git.c:331)\n\nHope this helps,\n\n-- \nAlexandre Julliard\njulliard@winehq.org\n\n"},{"id":"30440","messageId":"Pine.LNX.4.63.0612281710350.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6119","inReplyTo":"87fyb11ouy.fsf@wine.dyndns.org","subject":"[PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-28T16:13:33Z","receivedAt":"2006-12-28T16:13:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe function xdl_refine_conflicts() tries to break down huge\nconflicts by doing a diff on the conflicting regions. However,\nthis does not make sense when one side is empty.\n\nWorse, when one side is not only empty, but after EOF, the code\naccessed unmapped memory.\n\nNoticed by Luben Tuikov, Shawn Pearce and Alexandre Julliard, the\nlatter providing a test case.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n---\n\n\tThank you Alexandre! I looked for the bug for quite some time, but \n\twas never close to the real culprit.\n\n t/t6023-merge-file.sh |   22 ++++++++++++++++++++++\n xdiff/xmerge.c        |    4 ++++\n 2 files changed, 26 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex 5d9b6f3..1c21d8c 100644\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -112,5 +112,27 @@ EOF\n test_expect_success \"expected conflict markers, with -L\" \\\n \t\"diff -u test.txt expect.txt\"\n \n+sed \"s/ tu / TU /\" < new1.txt > new5.txt\n+test_expect_failure \"conflict in removed tail\" \\\n+\t\"git-merge-file -p orig.txt new1.txt new5.txt > out\"\n+\n+cat > expect << EOF\n+Dominus regit me,\n+et nihil mihi deerit.\n+In loco pascuae ibi me collocavit,\n+super aquam refectionis educavit me;\n+animam meam convertit,\n+deduxit me super semitas jusitiae,\n+propter nomen suum.\n+<<<<<<< orig.txt\n+=======\n+Nam et si ambulavero in medio umbrae mortis,\n+non timebo mala, quoniam TU mecum es:\n+virga tua et baculus tuus ipsa me consolata sunt.\n+>>>>>>> new5.txt\n+EOF\n+\n+test_expect_success \"expected conflict markers\" \"diff -u expect out\"\n+\n test_done\n \ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 352207e..294450b 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -190,6 +190,10 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,\n \t\tif (m->mode)\n \t\t\tcontinue;\n \n+\t\t/* no sense refining a conflict when one side is empty */\n+\t\tif (m->chg1 == 0 || m->chg2 == 0)\n+\t\t\tcontinue;\n+\n \t\t/*\n \t\t * This probably does not work outside git, since\n \t\t * we have a very simple mmfile structure.\n-- \n1.5.0.rc0.g09372-dirty\n"},{"id":"30444","messageId":"7vejqjvfih.fsf@assigned-by-dhcp.cox.net","threadId":"6119","inReplyTo":"Pine.LNX.4.63.0612281710350.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-28T22:09:58Z","receivedAt":"2006-12-28T22:09:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thank you, everybody.\n"},{"id":"30460","messageId":"20061229041626.GA12072@spearce.org","threadId":"6119","inReplyTo":"Pine.LNX.4.63.0612281710350.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-29T04:16:26Z","receivedAt":"2006-12-29T04:16:26Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> \tThank you Alexandre! I looked for the bug for quite some time, but \n> \twas never close to the real culprit.\n\nThanks for fixing this!\n \n> +<<<<<<< orig.txt\n> +=======\n> +Nam et si ambulavero in medio umbrae mortis,\n> +non timebo mala, quoniam TU mecum es:\n> +virga tua et baculus tuus ipsa me consolata sunt.\n> +>>>>>>> new5.txt\n\nAs a side note I lately have noticed that xdl_merge is producing a\nconflict like the above when one branch added the lower half and\nthe other branch didn't change anything in the area.\n\nI haven't spent any time to try to reproduce it, or to see if RCS'\nmerge utility would automatically merge the file without producing\na conflict.  But right now it does seem like xdl_merge is producing\nconflicts when I didn't think it should be.\n\n-- \nShawn.\n"},{"id":"30506","messageId":"Pine.LNX.4.63.0612301944350.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6119","inReplyTo":"20061229041626.GA12072@spearce.org","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-30T18:53:18Z","receivedAt":"2006-12-30T18:53:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 28 Dec 2006, Shawn Pearce wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> > \tThank you Alexandre! I looked for the bug for quite some time, but \n> > \twas never close to the real culprit.\n> \n> Thanks for fixing this!\n>  \n> > +<<<<<<< orig.txt\n> > +=======\n> > +Nam et si ambulavero in medio umbrae mortis,\n> > +non timebo mala, quoniam TU mecum es:\n> > +virga tua et baculus tuus ipsa me consolata sunt.\n> > +>>>>>>> new5.txt\n> \n> As a side note I lately have noticed that xdl_merge is producing a\n> conflict like the above when one branch added the lower half and\n> the other branch didn't change anything in the area.\n> \n> I haven't spent any time to try to reproduce it, or to see if RCS'\n> merge utility would automatically merge the file without producing\n> a conflict.  But right now it does seem like xdl_merge is producing\n> conflicts when I didn't think it should be.\n\nI thought very long about that problem. It looks like a bug, but it is \nnot. At least in my humble opinion.\n\nIf you touched the same spot in two different versions, say you added a \nfix in one branch, and that fix and a comment in the other one, you might \nbe tempted to automatically resolve that conflict, taking the version with \nthe comment.\n\nBut it helps you catch mismerges: If you add a chunk of identical code in \nthe two branches, but with an increment _before_ it in one branch, and \n_after_ it in the other branch, both should be marked as a conflict. \n\nOf course, you can hit mismerges like the illustrated one _without_ being \nmarked as conflict (e.g. if the chunk of identical code is _not_ added, \nbut only the increments), but we should at least avoid them where \npossible.\n\nCiao,\nDscho\n"},{"id":"30512","messageId":"en6fj1$ji5$1@sea.gmane.org","threadId":"6119","inReplyTo":"Pine.LNX.4.63.0612301944350.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-30T19:47:34Z","receivedAt":"2006-12-30T19:47:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"<opublikowany i wysłany>\n\nJohannes Schindelin wrote:\n\n> On Thu, 28 Dec 2006, Shawn Pearce wrote:\n> \n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n>> >    Thank you Alexandre! I looked for the bug for quite some time, but \n>> >    was never close to the real culprit.\n>> \n>> Thanks for fixing this!\n>>  \n>> > +<<<<<<< orig.txt\n>> > +=======\n>> > +Nam et si ambulavero in medio umbrae mortis,\n>> > +non timebo mala, quoniam TU mecum es:\n>> > +virga tua et baculus tuus ipsa me consolata sunt.\n>> > +>>>>>>> new5.txt\n>> \n>> As a side note I lately have noticed that xdl_merge is producing a\n>> conflict like the above when one branch added the lower half and\n>> the other branch didn't change anything in the area.\n>> \n>> I haven't spent any time to try to reproduce it, or to see if RCS'\n>> merge utility would automatically merge the file without producing\n>> a conflict.  But right now it does seem like xdl_merge is producing\n>> conflicts when I didn't think it should be.\n> \n> I thought very long about that problem. It looks like a bug, but it is \n> not. At least in my humble opinion.\n> \n> If you touched the same spot in two different versions, say you added a \n> fix in one branch, and that fix and a comment in the other one, you might \n> be tempted to automatically resolve that conflict, taking the version with \n> the comment.\n> \n> But it helps you catch mismerges: If you add a chunk of identical code in \n> the two branches, but with an increment _before_ it in one branch, and \n> _after_ it in the other branch, both should be marked as a conflict. \n> \n> Of course, you can hit mismerges like the illustrated one _without_ being \n> marked as conflict (e.g. if the chunk of identical code is _not_ added, \n> but only the increments), but we should at least avoid them where \n> possible.\n\nPerhaps you could make it even more conservating merge conflicts option\n(to tighten merge conflicts even more) to xdl_merge, but not used by default\nbecause as it removes accidental conflicts it increases mismerges (falsely\nnot conflicted).\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"30520","messageId":"Pine.LNX.4.63.0612310208460.25709@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6119","inReplyTo":"en6fj1$ji5$1@sea.gmane.org","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-31T01:09:55Z","receivedAt":"2006-12-31T01:09:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 30 Dec 2006, Jakub Narebski wrote:\n\n> Johannes Schindelin wrote:\n> \n> > Of course, you can hit mismerges like the illustrated one _without_ \n> > being marked as conflict (e.g. if the chunk of identical code is _not_ \n> > added, but only the increments), but we should at least avoid them \n> > where possible.\n> \n> Perhaps you could make it even more conservating merge conflicts option \n> (to tighten merge conflicts even more) to xdl_merge, but not used by \n> default because as it removes accidental conflicts it increases \n> mismerges (falsely not conflicted).\n\nThere is no way to do this sanely. If you want to catch these mismerges, \nyou have to mark _all_ modifications as conflicting.\n\nCiao,\nDscho\n"},{"id":"30668","messageId":"200701021418.48624.jnareb@gmail.com","threadId":"6119","inReplyTo":"Pine.LNX.4.63.0612310208460.25709@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-01-02T13:18:47Z","receivedAt":"2007-01-02T13:18:47Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 31 Dec 2006, Johannes Schindelin wrote:\n\n> On Sat, 30 Dec 2006, Jakub Narebski wrote:\n> \n>> Johannes Schindelin wrote:\n>> \n>>> Of course, you can hit mismerges like the illustrated one _without_ \n>>> being marked as conflict (e.g. if the chunk of identical code is _not_ \n>>> added, but only the increments), but we should at least avoid them \n>>> where possible.\n>> \n>> Perhaps you could make it even more conservating merge conflicts option \n>> (to tighten merge conflicts even more) to xdl_merge, but not used by \n>> default because as it removes accidental conflicts it increases \n>> mismerges (falsely not conflicted).\n> \n> There is no way to do this sanely. If you want to catch these mismerges, \n> you have to mark _all_ modifications as conflicting.\n\nCurrently you have:\n - a level value of 0 means that all overlapping changes are treated\n   as conflicts,\n - a value of 1 means that if the overlapping changes are identical,\n   it is not treated as a conflict.\n - If you set level to 2, overlapping changes will be analyzed, so that\n   almost identical changes will not result in huge conflicts. Rather,\n   only the conflicting lines will be shown inside conflict markers.\n\nI was thinking about:\n - If you set level to 3, if one part after overlapping changes analysis\n   in level 2 has empty conflict region, resolve this conflict as second\n   side. WARNING: this reduces number of merge conflicts, but might give\n   mismerges!\n\nOr something like that.\n-- \nJakub Narebski\nPoland\n"},{"id":"30697","messageId":"Pine.LNX.4.63.0701022144290.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6119","inReplyTo":"200701021418.48624.jnareb@gmail.com","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-01-02T20:58:45Z","receivedAt":"2007-01-02T20:58:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 2 Jan 2007, Jakub Narebski wrote:\n\n> On Sun, 31 Dec 2006, Johannes Schindelin wrote:\n> \n> > On Sat, 30 Dec 2006, Jakub Narebski wrote:\n> > \n> >> Johannes Schindelin wrote:\n> >> \n> >>> Of course, you can hit mismerges like the illustrated one _without_ \n> >>> being marked as conflict (e.g. if the chunk of identical code is _not_ \n> >>> added, but only the increments), but we should at least avoid them \n> >>> where possible.\n> >> \n> >> Perhaps you could make it even more conservating merge conflicts option \n> >> (to tighten merge conflicts even more) to xdl_merge, but not used by \n> >> default because as it removes accidental conflicts it increases \n> >> mismerges (falsely not conflicted).\n> > \n> > There is no way to do this sanely. If you want to catch these mismerges, \n> > you have to mark _all_ modifications as conflicting.\n> \n> Currently you have:\n>  - a level value of 0 means that all overlapping changes are treated\n>    as conflicts,\n>  - a value of 1 means that if the overlapping changes are identical,\n>    it is not treated as a conflict.\n>  - If you set level to 2, overlapping changes will be analyzed, so that\n>    almost identical changes will not result in huge conflicts. Rather,\n>    only the conflicting lines will be shown inside conflict markers.\n> \n> I was thinking about:\n>  - If you set level to 3, if one part after overlapping changes analysis\n>    in level 2 has empty conflict region, resolve this conflict as second\n>    side. WARNING: this reduces number of merge conflicts, but might give\n>    mismerges!\n\nThat is certainly a possibility! But how would you specify it? If you \ndo it as a command line option, you'd have to add it to git-merge, \ngit-pull, git-merge-recursive and git-merge-file. Ugly.\n\nYou could do it as a config variable, but git-merge-file operates without \na git repository. And you'd want it to be overrideable anyway.\n\nEnvironment variable? Too unfriendly to users, because it is not really a \nproper UI.\n\nIt is certainly easy enough to teach the code (but might introduce \nunwanted side effects, when the _deletion_, not the _addition_ was what \nyou wanted):\n\n xdiff/xdiff.h  |    1 +\n xdiff/xmerge.c |   21 ++++++++++++++++++---\n 2 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex fa409d5..bdf12e3 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -52,6 +52,7 @@ extern \"C\" {\n #define XDL_MERGE_MINIMAL 0\n #define XDL_MERGE_EAGER 1\n #define XDL_MERGE_ZEALOUS 2\n+#define XDL_MERGE_OVERZEALOUS 3\n \n typedef struct s_mmfile {\n \tchar *ptr;\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex b83b334..e7b740f 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -180,7 +180,7 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n  * lines. Try hard to show only these few lines as conflicting.\n  */\n static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,\n-\t\txpparam_t const *xpp)\n+\t\tint level, xpparam_t const *xpp)\n {\n \tfor (; m; m = m->next) {\n \t\tmmfile_t t1, t2;\n@@ -225,6 +225,12 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,\n \t\tm->chg1 = xscr->chg1;\n \t\tm->i2 = xscr->i2 + i2;\n \t\tm->chg2 = xscr->chg2;\n+\t\tif (level >= XDL_MERGE_OVERZEALOUS) {\n+\t\t\tif (!m->chg1)\n+\t\t\t\tm->mode = 2;\n+\t\t\telse if (!m->chg2)\n+\t\t\t\tm->mode = 1;\n+\t\t}\n \t\twhile (xscr->next) {\n \t\t\txdmerge_t *m2 = xdl_malloc(sizeof(xdmerge_t));\n \t\t\tif (!m2) {\n@@ -241,6 +247,12 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,\n \t\t\tm->chg1 = xscr->chg1;\n \t\t\tm->i2 = xscr->i2 + i2;\n \t\t\tm->chg2 = xscr->chg2;\n+\t\t\tif (level >= XDL_MERGE_OVERZEALOUS) {\n+\t\t\t\tif (!m->chg1)\n+\t\t\t\t\tm->mode = 2;\n+\t\t\t\telse if (!m->chg2)\n+\t\t\t\t\tm->mode = 1;\n+\t\t\t}\n \t\t}\n \t\txdl_free_env(&xe);\n \t\txdl_free_script(x);\n@@ -252,6 +264,7 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,\n  * level == 0: mark all overlapping changes as conflict\n  * level == 1: mark overlapping changes as conflict only if not identical\n  * level == 2: analyze non-identical changes for minimal conflict set\n+ * level == 3: if one side in the analysis is empty, assume no conflict\n  *\n  * returns < 0 on error, == 0 for no conflicts, else number of conflicts\n  */\n@@ -290,7 +303,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\t\txscr2 = xscr2->next;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (level < 1 || xscr1->i1 != xscr2->i1 ||\n+\t\tif (level < XDL_MERGE_EAGER || xscr1->i1 != xscr2->i1 ||\n \t\t\t\txscr1->chg1 != xscr2->chg1 ||\n \t\t\t\txscr1->chg2 != xscr2->chg2 ||\n \t\t\t\txdl_merge_cmp_lines(xe1, xscr1->i2,\n@@ -355,7 +368,9 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \tif (!changes)\n \t\tchanges = c;\n \t/* refine conflicts */\n-\tif (level > 1 && xdl_refine_conflicts(xe1, xe2, changes, xpp) < 0) {\n+\tif (level >= XDL_MERGE_ZEALOUS &&\n+\t\t\txdl_refine_conflicts(xe1, xe2, changes,\n+\t\t\t\txpp, level) < 0) {\n \t\txdl_cleanup_merge(changes);\n \t\treturn -1;\n \t}\n"},{"id":"30698","messageId":"7vk605t9qz.fsf@assigned-by-dhcp.cox.net","threadId":"6119","inReplyTo":"Pine.LNX.4.63.0701022144290.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-02T21:11:00Z","receivedAt":"2007-01-02T21:11:00Z","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> That is certainly a possibility! But how would you specify it? If you \n> do it as a command line option, you'd have to add it to git-merge, \n> git-pull, git-merge-recursive and git-merge-file. Ugly.\n\nAnother thing to worry about is that this would make things\n\"works most of the time but when it fails it fails silently\"\nwhich would lead to very hard to detect problem in the project\nmanaged by git.  I'd be very hesitant about this for this reason\nalone.\n"},{"id":"30701","messageId":"Pine.LNX.4.63.0701022216420.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6119","inReplyTo":"7vk605t9qz.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-01-02T21:17:43Z","receivedAt":"2007-01-02T21:17:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 2 Jan 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > That is certainly a possibility! But how would you specify it? If you \n> > do it as a command line option, you'd have to add it to git-merge, \n> > git-pull, git-merge-recursive and git-merge-file. Ugly.\n> \n> Another thing to worry about is that this would make things\n> \"works most of the time but when it fails it fails silently\"\n> which would lead to very hard to detect problem in the project\n> managed by git.  I'd be very hesitant about this for this reason\n> alone.\n\nRight. That was what I was alluding to with my comment \"what if you prefer \nthe deletion over the addition?\"\n\nIt _seems_ clever at first sight, but it isn't.\n\nCiao,\nDscho\n"}]}