# Segfault with merge-tree on multiple Git versions 15 messages from 2013-03-27 to 2013-03-27. Participants: Charlie Smurthwaite, thomas, John Keeping, Thomas Rast, Junio C Hamano, Jed Brown, Jeff King. Thread: https://gitlist.dev/t/33302 ## Charlie Smurthwaite, 2013-03-27 15:29 Subject: Segfault with merge-tree on multiple Git versions Message-ID: <51531059.8000407@atechmedia.com> URL: https://gitlist.dev/e/51531059.8000407%40atechmedia.com ``` I am experiencing a segmentation fault in various versions of Git using different repositories. Specifically, I have reproduced it using a public repo and the latest stable Git version. Other repos trigger the error on different versions. Full info can be found below. Thanks, Charlie Test repository: https://github.com/atech/mail Test Command git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f 8d6bdf012941d876b2279994e02f1bb0d5c26e7d d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 Environment: Linux codebase-staging 2.6.32-41-server #91-Ubuntu SMP Wed Jun 13 11:58:56 UTC 2012 x86_64 GNU/Linux Git: git version 1.8.2 Output: charlie@codebase-staging:~/mail$ git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f 8d6bdf012941d876b2279994e02f1bb0d5c26e7d d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 Segmentation fault Charlie Smurthwaite aTech Media tel. 01202 901 222 (ext. 603) email. charlie@atechmedia.com web. atechmedia.com aTech Media Limited is a registered company in England and Wales. Registration Number 5523199. Registered Office: Unit 9 Winchester Place, North Street, Poole, Dorset, BH15 1NX. VAT Registration Number: GB 868 861 560. This e-mail is confidential and for the intended recipient only. If you are not the intended recipient, be advised that you have received this e-mail in error and that any use, dissemination, forwarding, printing, or copying of this e-mail is prohibited. If you have received this e-mail in error, please notify the sender. ``` ## thomas, 2013-03-27 15:53 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <87boa47saw.fsf@linux-k42r.v.cablecom.net> URL: https://gitlist.dev/e/87boa47saw.fsf%40linux-k42r.v.cablecom.net In-Reply-To: <51531059.8000407@atechmedia.com> ``` Charlie Smurthwaite writes: > I am experiencing a segmentation fault in various versions of Git using > different repositories. Specifically, I have reproduced it using a > public repo and the latest stable Git version. Other repos trigger the > error on different versions. > > Full info can be found below. Thanks, > > Charlie > > > Test repository: > https://github.com/atech/mail > > Test Command > git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f > 8d6bdf012941d876b2279994e02f1bb0d5c26e7d > d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 I happened to walk past on IRC and found I could easily reproduce it, so I bisected: 35ffe7583108ab236dcf81226690388491d9962f is the first bad commit commit 35ffe7583108ab236dcf81226690388491d9962f Author: Junio C Hamano Date: Thu Dec 13 15:51:29 2012 -0800 merge-tree: fix d/f conflicts The previous commit documented two known breakages revolving around a case where one side flips a tree into a blob (or vice versa), where the original code simply gets confused and feeds a mixture of trees and blobs into either the recursive merge-tree (and recursing into the blob will fail) or three-way merge (and merging tree contents together with blobs will fail). Fix it by feeding trees (and only trees) into the recursive merge-tree machinery and blobs (and only blobs) into the three-way content level merge machinery separately; when this happens, the entire merge has to be marked as conflicting at the structure level. Signed-off-by: Junio C Hamano It seems to be a vanilla null dereference: Program received signal SIGSEGV, Segmentation fault. 0x0000000000453bf9 in add_merge_entry (entry=0x0) at builtin/merge-tree.c:24 24 *merge_result_end = entry; (gdb) bt #0 0x0000000000453bf9 in add_merge_entry (entry=0x0) at builtin/merge-tree.c:24 #1 0x00000000004545f4 in unresolved (info=0x7fffffffce90, n=0x7ff7f0) at builtin/merge-tree.c:265 #2 0x0000000000454741 in threeway_callback (n=3, mask=7, dirmask=7, entry=0x7ff7f0, info=0x7fffffffce90) at builtin/merge-tree.c:330 #3 0x00000000005233f3 in traverse_trees (n=3, t=0x7fffffffcf10, info=0x7fffffffce90) at tree-walk.c:407 #4 0x0000000000454792 in merge_trees_recursive (t=0x7fffffffcf10, base=0x800530 "lib/mail", df_conflict=1) at builtin/merge-tree.c:341 #5 0x0000000000454382 in unresolved_directory (info=0x7fffffffd120, n=0x800420, df_conflict=1) at builtin/merge-tree.c:216 #6 0x0000000000454507 in unresolved (info=0x7fffffffd120, n=0x800420) at builtin/merge-tree.c:253 #7 0x0000000000454741 in threeway_callback (n=3, mask=7, dirmask=7, entry=0x800420, info=0x7fffffffd120) at builtin/merge-tree.c:330 #8 0x00000000005233f3 in traverse_trees (n=3, t=0x7fffffffd1a0, info=0x7fffffffd120) at tree-walk.c:407 #9 0x0000000000454792 in merge_trees_recursive (t=0x7fffffffd1a0, base=0x7fd170 "lib", df_conflict=1) at builtin/merge-tree.c:341 #10 0x0000000000454382 in unresolved_directory (info=0x7fffffffd3b0, n=0x8069f0, df_conflict=1) at builtin/merge-tree.c:216 #11 0x0000000000454507 in unresolved (info=0x7fffffffd3b0, n=0x8069f0) at builtin/merge-tree.c:253 #12 0x0000000000454741 in threeway_callback (n=3, mask=7, dirmask=7, entry=0x8069f0, info=0x7fffffffd3b0) at builtin/merge-tree.c:330 #13 0x00000000005233f3 in traverse_trees (n=3, t=0x7fffffffd450, info=0x7fffffffd3b0) at tree-walk.c:407 #14 0x0000000000454792 in merge_trees_recursive (t=0x7fffffffd450, base=0x5510fc "", df_conflict=0) at builtin/merge-tree.c:341 #15 0x00000000004547bc in merge_trees (t=0x7fffffffd450, base=0x5510fc "") at builtin/merge-tree.c:346 #16 0x00000000004548ef in cmd_merge_tree (argc=4, argv=0x7fffffffd728, prefix=0x0) at builtin/merge-tree.c:373 #17 0x00000000004056ec in run_builtin (p=0x7a1c88 , argc=4, argv=0x7fffffffd728) at git.c:273 #18 0x000000000040587f in handle_internal_command (argc=4, argv=0x7fffffffd728) at git.c:434 #19 0x0000000000405a4b in main (argc=4, argv=0x7fffffffd728) at git.c:523 Unfortunately I'm not familiar with the merge code, but if you can't reproduce at your end let me know. -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## John Keeping, 2013-03-27 15:58 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <20130327155850.GN2286@serenity.lan> URL: https://gitlist.dev/e/20130327155850.GN2286%40serenity.lan In-Reply-To: <87boa47saw.fsf@linux-k42r.v.cablecom.net> ``` On Wed, Mar 27, 2013 at 04:53:27PM +0100, thomas wrote: > Charlie Smurthwaite writes: > > > I am experiencing a segmentation fault in various versions of Git using > > different repositories. Specifically, I have reproduced it using a > > public repo and the latest stable Git version. Other repos trigger the > > error on different versions. > > > > Full info can be found below. Thanks, > > > > Charlie > > > > > > Test repository: > > https://github.com/atech/mail > > > > Test Command > > git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f > > 8d6bdf012941d876b2279994e02f1bb0d5c26e7d > > d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 > > I happened to walk past on IRC and found I could easily reproduce it, so > I bisected: > > 35ffe7583108ab236dcf81226690388491d9962f is the first bad commit > commit 35ffe7583108ab236dcf81226690388491d9962f > Author: Junio C Hamano > Date: Thu Dec 13 15:51:29 2012 -0800 > > merge-tree: fix d/f conflicts > > The previous commit documented two known breakages revolving around > a case where one side flips a tree into a blob (or vice versa), > where the original code simply gets confused and feeds a mixture of > trees and blobs into either the recursive merge-tree (and recursing > into the blob will fail) or three-way merge (and merging tree contents > together with blobs will fail). > > Fix it by feeding trees (and only trees) into the recursive > merge-tree machinery and blobs (and only blobs) into the three-way > content level merge machinery separately; when this happens, the > entire merge has to be marked as conflicting at the structure level. > > Signed-off-by: Junio C Hamano Looks like a simple typo in merge-tree.c::unresolved: -- >8 -- merge-tree: fix typo in merge-tree.c::unresolved When calculating whether there is a d/f conflict, the calculation of whether both sides are directories generates an incorrect references mask because it does not use the loop index to set the correct bit. Fix this typo. Signed-off-by: John Keeping diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index e0d0b7d..bc912e3 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -245,7 +245,7 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3]) unsigned dirmask = 0, mask = 0; for (i = 0; i < 3; i++) { - mask |= (1 << 1); + mask |= (1 << i); if (n[i].mode && S_ISDIR(n[i].mode)) dirmask |= (1 << i); } ``` ## Thomas Rast, 2013-03-27 16:05 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <874nfw7rrm.fsf@linux-k42r.v.cablecom.net> URL: https://gitlist.dev/e/874nfw7rrm.fsf%40linux-k42r.v.cablecom.net In-Reply-To: <20130327155850.GN2286@serenity.lan> ``` John Keeping writes: > merge-tree: fix typo in merge-tree.c::unresolved > > When calculating whether there is a d/f conflict, the calculation of > whether both sides are directories generates an incorrect references > mask because it does not use the loop index to set the correct bit. > Fix this typo. > > Signed-off-by: John Keeping > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > index e0d0b7d..bc912e3 100644 > --- a/builtin/merge-tree.c > +++ b/builtin/merge-tree.c > @@ -245,7 +245,7 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3]) > unsigned dirmask = 0, mask = 0; > > for (i = 0; i < 3; i++) { > - mask |= (1 << 1); > + mask |= (1 << i); > if (n[i].mode && S_ISDIR(n[i].mode)) > dirmask |= (1 << i); > } Indeed, that fixes it. -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## Junio C Hamano, 2013-03-27 16:33 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <7v1ub0ol91.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1ub0ol91.fsf%40alter.siamese.dyndns.org In-Reply-To: <20130327155850.GN2286@serenity.lan> ``` John Keeping writes: > Looks like a simple typo in merge-tree.c::unresolved: Thanks. > > -- >8 -- > merge-tree: fix typo in merge-tree.c::unresolved > > When calculating whether there is a d/f conflict, the calculation of > whether both sides are directories generates an incorrect references > mask because it does not use the loop index to set the correct bit. > Fix this typo. > > Signed-off-by: John Keeping > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > index e0d0b7d..bc912e3 100644 > --- a/builtin/merge-tree.c > +++ b/builtin/merge-tree.c > @@ -245,7 +245,7 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3]) > unsigned dirmask = 0, mask = 0; > > for (i = 0; i < 3; i++) { > - mask |= (1 << 1); > + mask |= (1 << i); > if (n[i].mode && S_ISDIR(n[i].mode)) > dirmask |= (1 << i); > } ``` ## Junio C Hamano, 2013-03-27 17:06 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <7vsj3gn55b.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsj3gn55b.fsf%40alter.siamese.dyndns.org In-Reply-To: <51531059.8000407@atechmedia.com> ``` Charlie Smurthwaite writes: > I am experiencing a segmentation fault in various versions of Git using > different repositories. > ... > Test Command > git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f > 8d6bdf012941d876b2279994e02f1bb0d5c26e7d > d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 Thanks for a report (and thanks to John and Thomas for finding the typo). Nobody I know uses merge-tree; the last real change we did was back from July 2010, and the only reason I was looking at it recently was because I was planning to write a new merge strategy using it. Mind if I ask what you are using it for? ``` ## Charlie Smurthwaite, 2013-03-27 17:17 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <5153298C.8020107@atechmedia.com> URL: https://gitlist.dev/e/5153298C.8020107%40atechmedia.com In-Reply-To: <7vsj3gn55b.fsf@alter.siamese.dyndns.org> ``` On 27/03/13 17:06, Junio C Hamano wrote: > Charlie Smurthwaite writes: > >> I am experiencing a segmentation fault in various versions of Git using >> different repositories. >> ... >> Test Command >> git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f >> 8d6bdf012941d876b2279994e02f1bb0d5c26e7d >> d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 > Thanks for a report (and thanks to John and Thomas for finding the > typo). > > Nobody I know uses merge-tree; the last real change we did was back > from July 2010, and the only reason I was looking at it recently was > because I was planning to write a new merge strategy using it. > > Mind if I ask what you are using it for? Thank you everybody for investigating this and creating a patch. Can I assume that this fix will reach somebody who can apply it to master? With regard our use, we run an SCM hosting service http://codebasehq.com and are in the process of deploying a merge-request feature. We use git-merge-tree to determine whether a Git merge can be completed automatically (without manual conflict resolution), and if so offer the user a button to execute an actual merge. If there is a better way to do this, I'd be happy to consider it. Charlie ``` ## Charlie Smurthwaite, 2013-03-27 17:52 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <515331F2.3060703@atechmedia.com> URL: https://gitlist.dev/e/515331F2.3060703%40atechmedia.com In-Reply-To: <7vsj3gn55b.fsf@alter.siamese.dyndns.org> ``` On 27/03/13 17:06, Junio C Hamano wrote: > Charlie Smurthwaite writes: > >> I am experiencing a segmentation fault in various versions of Git using >> different repositories. >> ... >> Test Command >> git merge-tree 26bb22a052fef9f74063afd4fc6fc11fe200b19f >> 8d6bdf012941d876b2279994e02f1bb0d5c26e7d >> d5ef97ac407d945f231cd7c8fb1cfe48b3a12083 > Thanks for a report (and thanks to John and Thomas for finding the > typo). > > Nobody I know uses merge-tree; the last real change we did was back > from July 2010, and the only reason I was looking at it recently was > because I was planning to write a new merge strategy using it. > > Mind if I ask what you are using it for? I am also using this to obtain a diff that would be applied if a merge were to be run. Is there a better way to obtain this information that is more commonly used? ``` ## Jed Brown, 2013-03-27 18:06 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <874nfw4t0r.fsf@59A2.org> URL: https://gitlist.dev/e/874nfw4t0r.fsf%4059A2.org In-Reply-To: <515331F2.3060703@atechmedia.com> ``` Charlie Smurthwaite writes: > I am also using this to obtain a diff that would be applied if a merge > were to be run. Is there a better way to obtain this information that is > more commonly used? You can do an actual merge using detached HEAD: $ git checkout --detach upstream-branch $ git merge topic-branch This has the benefit that if there are conflicts, you can resolve them here and commit the result so that rerere can auto-resolve them later. Are you looking for something that can be run in a bare repo? ``` ## Charlie Smurthwaite, 2013-03-27 18:46 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <51533E8D.5050206@atechmedia.com> URL: https://gitlist.dev/e/51533E8D.5050206%40atechmedia.com In-Reply-To: <874nfw4t0r.fsf@59A2.org> ``` On 27/03/13 18:06, Jed Brown wrote: > Charlie Smurthwaite writes: > >> I am also using this to obtain a diff that would be applied if a merge >> were to be run. Is there a better way to obtain this information that is >> more commonly used? > You can do an actual merge using detached HEAD: > > $ git checkout --detach upstream-branch > $ git merge topic-branch > > This has the benefit that if there are conflicts, you can resolve them > here and commit the result so that rerere can auto-resolve them later. > > Are you looking for something that can be run in a bare repo? Yes, I would need to be able to do this on a bare repo for my use case. Thanks! ``` ## Jed Brown, 2013-03-27 19:16 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <87zjxo3b7b.fsf@59A2.org> URL: https://gitlist.dev/e/87zjxo3b7b.fsf%4059A2.org In-Reply-To: <51533E8D.5050206@atechmedia.com> ``` Charlie Smurthwaite writes: > Yes, I would need to be able to do this on a bare repo for my use case. And if it's on the server, you don't want this to be observable, so you don't want HEAD to move around. I don't know a better way than: $ git clone --shared -b upstream-branch bare-repo.git /tmp/merge-repo $ cd /tmp/merge-repo $ git pull URL incoming-branch Cloning with --shared just writes a path into .git/objects/info/alternatives and it doesn't need to be on the same file system (unlike --local). Since 'git merge-tree' just works with trees, it has less information than 'git merge'. ``` ## John Keeping, 2013-03-27 19:45 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <20130327194521.GQ2286@serenity.lan> URL: https://gitlist.dev/e/20130327194521.GQ2286%40serenity.lan In-Reply-To: <87zjxo3b7b.fsf@59A2.org> ``` On Wed, Mar 27, 2013 at 02:16:24PM -0500, Jed Brown wrote: > Charlie Smurthwaite writes: > > > Yes, I would need to be able to do this on a bare repo for my use case. > > And if it's on the server, you don't want this to be observable, so > you don't want HEAD to move around. I don't know a better way than: > > $ git clone --shared -b upstream-branch bare-repo.git /tmp/merge-repo > $ cd /tmp/merge-repo > $ git pull URL incoming-branch > > Cloning with --shared just writes a path into .git/objects/info/alternatives > and it doesn't need to be on the same file system (unlike --local). > > Since 'git merge-tree' just works with trees, it has less information > than 'git merge'. You could use a temporary index and do something like: rm -f TMP_INDEX GIT_INDEX_FILE=TMP_INDEX export GIT_INDEX_FILE git read-tree -m $base $ours $theirs && git merge-index git-merge-one-file -a then inspect that with "git diff-index --cached $ours". Note that this will fail if there are conflicts and I don't know what git-merge-tree will do in that case. ``` ## Jeff King, 2013-03-27 20:01 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <20130327200113.GC26380@sigill.intra.peff.net> URL: https://gitlist.dev/e/20130327200113.GC26380%40sigill.intra.peff.net In-Reply-To: <20130327194521.GQ2286@serenity.lan> ``` On Wed, Mar 27, 2013 at 07:45:21PM +0000, John Keeping wrote: > On Wed, Mar 27, 2013 at 02:16:24PM -0500, Jed Brown wrote: > > Charlie Smurthwaite writes: > > > > > Yes, I would need to be able to do this on a bare repo for my use case. > > > > And if it's on the server, you don't want this to be observable, so > > you don't want HEAD to move around. I don't know a better way than: > > > > $ git clone --shared -b upstream-branch bare-repo.git /tmp/merge-repo > > $ cd /tmp/merge-repo > > $ git pull URL incoming-branch > > > > Cloning with --shared just writes a path into .git/objects/info/alternatives > > and it doesn't need to be on the same file system (unlike --local). > > > > Since 'git merge-tree' just works with trees, it has less information > > than 'git merge'. > > You could use a temporary index and do something like: > > rm -f TMP_INDEX > GIT_INDEX_FILE=TMP_INDEX > export GIT_INDEX_FILE > git read-tree -m $base $ours $theirs && > git merge-index git-merge-one-file -a > > then inspect that with "git diff-index --cached $ours". That is precisely how we do it at GitHub. You probably want to add in "--aggressive" to your read-tree to cover a few more simple cases. If there are conflicts, we just bail and say "this can't be merged", and expect the user to do it themselves using git. -Peff ``` ## Junio C Hamano, 2013-03-27 20:04 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <7vtxnwlidq.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtxnwlidq.fsf%40alter.siamese.dyndns.org In-Reply-To: <20130327194521.GQ2286@serenity.lan> ``` John Keeping writes: > You could use a temporary index and do something like: > > rm -f TMP_INDEX > GIT_INDEX_FILE=TMP_INDEX > export GIT_INDEX_FILE > git read-tree -m $base $ours $theirs && > git merge-index git-merge-one-file -a > > then inspect that with "git diff-index --cached $ours". Good. > Note that this will fail if there are conflicts and I don't know what > git-merge-tree will do in that case. I _think_ Charlies's use case is to detect trivial merges to tell the requestee that a merge request can be done on site, so failing is fine when there are conflicts. merge-tree should report conflicts as well. ``` ## Charlie Smurthwaite, 2013-03-27 21:10 Subject: Re: Segfault with merge-tree on multiple Git versions Message-ID: <51536063.50405@atechmedia.com> URL: https://gitlist.dev/e/51536063.50405%40atechmedia.com In-Reply-To: <20130327200113.GC26380@sigill.intra.peff.net> ``` On 27/03/13 20:01, Jeff King wrote: > On Wed, Mar 27, 2013 at 07:45:21PM +0000, John Keeping wrote: > >> On Wed, Mar 27, 2013 at 02:16:24PM -0500, Jed Brown wrote: >>> Charlie Smurthwaite writes: >>> >>>> Yes, I would need to be able to do this on a bare repo for my use case. >>> And if it's on the server, you don't want this to be observable, so >>> you don't want HEAD to move around. I don't know a better way than: >>> >>> $ git clone --shared -b upstream-branch bare-repo.git /tmp/merge-repo >>> $ cd /tmp/merge-repo >>> $ git pull URL incoming-branch >>> >>> Cloning with --shared just writes a path into .git/objects/info/alternatives >>> and it doesn't need to be on the same file system (unlike --local). >>> >>> Since 'git merge-tree' just works with trees, it has less information >>> than 'git merge'. >> You could use a temporary index and do something like: >> >> rm -f TMP_INDEX >> GIT_INDEX_FILE=TMP_INDEX >> export GIT_INDEX_FILE >> git read-tree -m $base $ours $theirs && >> git merge-index git-merge-one-file -a >> >> then inspect that with "git diff-index --cached $ours". > That is precisely how we do it at GitHub. You probably want to add in > "--aggressive" to your read-tree to cover a few more simple cases. If > there are conflicts, we just bail and say "this can't be merged", and > expect the user to do it themselves using git. > > -Peff This may be ideal. I will compare it with merge-tree to see which will suit best. Thank you everyone for your help here. Charlie ```