Segfault in git merge-tree (1.8.2.2)

Segfault in git merge-tree (1.8.2.2)

[Andreas Jacobsen]

Hi,

I'm getting a segfault in git merge-tree using v1.8.2.2 on MacOS
10.8.3. I can't share the repo, but I can build patches and check if
they fix the problem :)

Here's a bt:

(gdb) run
Starting program:
/usr/local/Cellar/git/1.8.2.2/libexec/git-core/git-merge-tree
027058e6ac8d03e029c4e1455bf90f63cd20e65b FETCH_HEAD master
Reading symbols for shared libraries +++++.............................. done

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x000000010003e532 in threeway_callback ()
(gdb) bt
#0  0x000000010003e532 in threeway_callback ()
#1  0x00000001000da03b in traverse_trees ()
#2  0x000000010003e46b in threeway_callback ()
#3  0x00000001000da03b in traverse_trees ()
#4  0x000000010003e46b in threeway_callback ()
#5  0x00000001000da03b in traverse_trees ()
#6  0x000000010003e46b in threeway_callback ()
#7  0x00000001000da03b in traverse_trees ()
#8  0x000000010003e46b in threeway_callback ()
#9  0x00000001000da03b in traverse_trees ()
#10 0x000000010003e46b in threeway_callback ()
#11 0x00000001000da03b in traverse_trees ()
#12 0x000000010003e46b in threeway_callback ()
#13 0x00000001000da03b in traverse_trees ()
#14 0x000000010003e46b in threeway_callback ()
#15 0x00000001000da03b in traverse_trees ()
#16 0x000000010003e46b in threeway_callback ()
#17 0x00000001000da03b in traverse_trees ()
#18 0x000000010003e46b in threeway_callback ()
#19 0x00000001000da03b in traverse_trees ()
#20 0x000000010003e46b in threeway_callback ()
#21 0x00000001000da03b in traverse_trees ()
#22 0x000000010003e46b in threeway_callback ()
#23 0x00000001000da03b in traverse_trees ()
#24 0x000000010003df02 in cmd_merge_tree ()
#25 0x0000000100000e99 in handle_internal_command ()
#26 0x0000000100000c59 in main ()

-Andreas

Re: Segfault in git merge-tree (1.8.2.2)

[John Keeping]

On Mon, May 06, 2013 at 03:02:10PM +0200, Andreas Jacobsen wrote:
> I'm getting a segfault in git merge-tree using v1.8.2.2 on MacOS
> 10.8.3. I can't share the repo, but I can build patches and check if
> they fix the problem :)

Can you rebuild with debugging information and try the backtrace again?

Something like:

    make CFLAGS='-O0 -g'

Then use the "git" in bin-wrappers/.

Re: Segfault in git merge-tree (1.8.2.2)

[Andreas Jacobsen]

Sure, here you go, this time built from the HEAD I found on github
(7d3ccdffb5d28970dd7a4d177cfcca690ccd0c22) with:

NO_GETTEXT=1 make prefix=/usr/local/Cellar/git/HEAD CC=cc CFLAGS='-O0
-g' install (this is homebrew's setup, but I built it manually rather
than using the recipe.)

And the gdb output:

(gdb) set args merge-tree 027058e6ac8d03e029c4e1455bf90f63cd20e65b
FETCH_HEAD master
(gdb) run
Starting program: /usr/local/bin/git merge-tree
027058e6ac8d03e029c4e1455bf90f63cd20e65b FETCH_HEAD master
Reading symbols for shared libraries +++++.............................. done

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x000000010006f1a3 in add_merge_entry (entry=0x100432ba0) at
builtin/merge-tree.c:24
24 *merge_result_end = entry;
(gdb) bt
#0  0x000000010006f1a3 in add_merge_entry (entry=0x100432ba0) at
builtin/merge-tree.c:24
#1  0x000000010006ee24 in unresolved (info=0x7fff5fbfd508,
n=0x1004367c0) at builtin/merge-tree.c:270
#2  0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=0,
entry=0x1004367c0, info=0x7fff5fbfd508) at builtin/merge-tree.c:330
#3  0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfd5d0,
info=0x7fff5fbfd508) at tree-walk.c:407
#4  0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfd5d0,
base=0x100436780
"bibliotek/httpservice/src/main/java/no/ok/it/lib/http/service",
df_conflict=1) at builtin/merge-tree.c:341
#5  0x000000010006f082 in unresolved_directory (info=0x7fff5fbfd7d8,
n=0x100436660, df_conflict=1) at builtin/merge-tree.c:221
#6  0x000000010006ed2c in unresolved (info=0x7fff5fbfd7d8,
n=0x100436660) at builtin/merge-tree.c:258
#7  0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100436660, info=0x7fff5fbfd7d8) at builtin/merge-tree.c:330
#8  0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfd8a0,
info=0x7fff5fbfd7d8) at tree-walk.c:407
#9  0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfd8a0,
base=0x100435e70
"bibliotek/httpservice/src/main/java/no/ok/it/lib/http",
df_conflict=1) at builtin/merge-tree.c:341
#10 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfdaa8,
n=0x100435d50, df_conflict=1) at builtin/merge-tree.c:221
#11 0x000000010006ed2c in unresolved (info=0x7fff5fbfdaa8,
n=0x100435d50) at builtin/merge-tree.c:258
#12 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100435d50, info=0x7fff5fbfdaa8) at builtin/merge-tree.c:330
#13 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfdb70,
info=0x7fff5fbfdaa8) at tree-walk.c:407
#14 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfdb70,
base=0x100435d10 "bibliotek/httpservice/src/main/java/no/ok/it/lib",
df_conflict=1) at builtin/merge-tree.c:341
#15 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfdd78,
n=0x100436450, df_conflict=1) at builtin/merge-tree.c:221
#16 0x000000010006ed2c in unresolved (info=0x7fff5fbfdd78,
n=0x100436450) at builtin/merge-tree.c:258
#17 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100436450, info=0x7fff5fbfdd78) at builtin/merge-tree.c:330
#18 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfde40,
info=0x7fff5fbfdd78) at tree-walk.c:407
#19 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfde40,
base=0x100436400 "bibliotek/httpservice/src/main/java/no/ok/it",
df_conflict=1) at builtin/merge-tree.c:341
#20 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfe048,
n=0x100433c80, df_conflict=1) at builtin/merge-tree.c:221
#21 0x000000010006ed2c in unresolved (info=0x7fff5fbfe048,
n=0x100433c80) at builtin/merge-tree.c:258
#22 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100433c80, info=0x7fff5fbfe048) at builtin/merge-tree.c:330
#23 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfe110,
info=0x7fff5fbfe048) at tree-walk.c:407
#24 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfe110,
base=0x100433c50 "bibliotek/httpservice/src/main/java/no/ok",
df_conflict=1) at builtin/merge-tree.c:341
#25 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfe318,
n=0x10042d2a0, df_conflict=1) at builtin/merge-tree.c:221
#26 0x000000010006ed2c in unresolved (info=0x7fff5fbfe318,
n=0x10042d2a0) at builtin/merge-tree.c:258
#27 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x10042d2a0, info=0x7fff5fbfe318) at builtin/merge-tree.c:330
#28 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfe3e0,
info=0x7fff5fbfe318) at tree-walk.c:407
#29 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfe3e0,
base=0x10042d250 "bibliotek/httpservice/src/main/java/no",
df_conflict=1) at builtin/merge-tree.c:341
#30 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfe5e8,
n=0x100435750, df_conflict=1) at builtin/merge-tree.c:221
#31 0x000000010006ed2c in unresolved (info=0x7fff5fbfe5e8,
n=0x100435750) at builtin/merge-tree.c:258
#32 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100435750, info=0x7fff5fbfe5e8) at builtin/merge-tree.c:330
#33 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfe6b0,
info=0x7fff5fbfe5e8) at tree-walk.c:407
#34 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfe6b0,
base=0x100431d70 "bibliotek/httpservice/src/main/java", df_conflict=1)
at builtin/merge-tree.c:341
#35 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfe8b8,
n=0x100431460, df_conflict=1) at builtin/merge-tree.c:221
#36 0x000000010006ed2c in unresolved (info=0x7fff5fbfe8b8,
n=0x100431460) at builtin/merge-tree.c:258
#37 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100431460, info=0x7fff5fbfe8b8) at builtin/merge-tree.c:330
#38 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfe980,
info=0x7fff5fbfe8b8) at tree-walk.c:407
#39 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfe980,
base=0x100431420 "bibliotek/httpservice/src/main", df_conflict=1) at
builtin/merge-tree.c:341
#40 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfeb88,
n=0x100433560, df_conflict=1) at builtin/merge-tree.c:221
#41 0x000000010006ed2c in unresolved (info=0x7fff5fbfeb88,
n=0x100433560) at builtin/merge-tree.c:258
#42 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100433560, info=0x7fff5fbfeb88) at builtin/merge-tree.c:330
#43 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfec50,
info=0x7fff5fbfeb88) at tree-walk.c:407
#44 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfec50,
base=0x1004325d0 "bibliotek/httpservice/src", df_conflict=1) at
builtin/merge-tree.c:341
#45 0x000000010006f082 in unresolved_directory (info=0x7fff5fbfee58,
n=0x100434ff0, df_conflict=1) at builtin/merge-tree.c:221
#46 0x000000010006ed2c in unresolved (info=0x7fff5fbfee58,
n=0x100434ff0) at builtin/merge-tree.c:258
#47 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100434ff0, info=0x7fff5fbfee58) at builtin/merge-tree.c:330
#48 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbfef20,
info=0x7fff5fbfee58) at tree-walk.c:407
#49 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbfef20,
base=0x100431770 "bibliotek/httpservice", df_conflict=1) at
builtin/merge-tree.c:341
#50 0x000000010006f082 in unresolved_directory (info=0x7fff5fbff128,
n=0x100433000, df_conflict=1) at builtin/merge-tree.c:221
#51 0x000000010006ed2c in unresolved (info=0x7fff5fbff128,
n=0x100433000) at builtin/merge-tree.c:258
#52 0x000000010006eaa2 in threeway_callback (n=3, mask=4, dirmask=4,
entry=0x100433000, info=0x7fff5fbff128) at builtin/merge-tree.c:330
#53 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbff1f0,
info=0x7fff5fbff128) at tree-walk.c:407
#54 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbff1f0,
base=0x10042fae0 "bibliotek", df_conflict=0) at
builtin/merge-tree.c:341
#55 0x000000010006f082 in unresolved_directory (info=0x7fff5fbff3f8,
n=0x100430530, df_conflict=0) at builtin/merge-tree.c:221
#56 0x000000010006ed2c in unresolved (info=0x7fff5fbff3f8,
n=0x100430530) at builtin/merge-tree.c:258
#57 0x000000010006eaa2 in threeway_callback (n=3, mask=7, dirmask=7,
entry=0x100430530, info=0x7fff5fbff3f8) at builtin/merge-tree.c:330
#58 0x000000010018eef2 in traverse_trees (n=3, t=0x7fff5fbff4b0,
info=0x7fff5fbff3f8) at tree-walk.c:407
#59 0x000000010006e8e8 in merge_trees_recursive (t=0x7fff5fbff4b0,
base=0x1001b548a "", df_conflict=0) at builtin/merge-tree.c:341
#60 0x000000010006e302 in merge_trees (t=0x7fff5fbff4b0,
base=0x1001b548a "") at builtin/merge-tree.c:346
#61 0x000000010006e1db in cmd_merge_tree (argc=4, argv=0x7fff5fbff720,
prefix=0x0) at builtin/merge-tree.c:373
#62 0x0000000100002940 in run_builtin (p=0x100210c40, argc=4,
argv=0x7fff5fbff720) at git.c:284
#63 0x0000000100001837 in handle_internal_command (argc=4,
argv=0x7fff5fbff720) at git.c:446
#64 0x0000000100002169 in run_argv (argcp=0x7fff5fbff6e8,
argv=0x7fff5fbff6e0) at git.c:492
#65 0x0000000100001675 in main (argc=4, argv=0x7fff5fbff720) at git.c:567
Current language:  auto; currently minimal

On 6 May 2013 15:39, John Keeping <john@keeping.me.uk> wrote:
> On Mon, May 06, 2013 at 03:02:10PM +0200, Andreas Jacobsen wrote:
>> I'm getting a segfault in git merge-tree using v1.8.2.2 on MacOS
>> 10.8.3. I can't share the repo, but I can build patches and check if
>> they fix the problem :)
>
> Can you rebuild with debugging information and try the backtrace again?
>
> Something like:
>
>     make CFLAGS='-O0 -g'
>
> Then use the "git" in bin-wrappers/.

Re: Segfault in git merge-tree (1.8.2.2)

[John Keeping]

On Mon, May 06, 2013 at 04:13:28PM +0200, Andreas Jacobsen wrote:
> Sure, here you go, this time built from the HEAD I found on github
> (7d3ccdffb5d28970dd7a4d177cfcca690ccd0c22) with:
> 
> NO_GETTEXT=1 make prefix=/usr/local/Cellar/git/HEAD CC=cc CFLAGS='-O0
> -g' install (this is homebrew's setup, but I built it manually rather
> than using the recipe.)
> 
> And the gdb output:
> 
> (gdb) set args merge-tree 027058e6ac8d03e029c4e1455bf90f63cd20e65b
> FETCH_HEAD master
> (gdb) run
> Starting program: /usr/local/bin/git merge-tree
> 027058e6ac8d03e029c4e1455bf90f63cd20e65b FETCH_HEAD master
> Reading symbols for shared libraries +++++.............................. done
> 
> Program received signal EXC_BAD_ACCESS, Could not access memory.
> Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
> 0x000000010006f1a3 in add_merge_entry (entry=0x100432ba0) at
> builtin/merge-tree.c:24
> 24 *merge_result_end = entry;

Thanks.  I have an idea of what's going on, but the set up is in an
earlier pass and it only fails the next time it gets into
add_merge_entry.

Can you try adding the following patch on top?  Hopefully the added
debug is in the right caller, otherwise the new assert at the top will
point us to the right one.

-- >8 --
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ec49917..8eebab7 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -23,6 +23,7 @@ static void add_merge_entry(struct merge_list *entry)
 {
 	*merge_result_end = entry;
 	merge_result_end = &entry->next;
+	assert(merge_result_end);
 }
 
 static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict);
@@ -267,6 +268,12 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3])
 	if (n[0].mode && !S_ISDIR(n[0].mode))
 		entry = link_entry(1, info, n + 0, entry);
 
+	if (!entry) {
+		fprintf(stderr, "n[0].mode = %d\nn[1].mode = %d\nn[2].mode = %d\n",
+			n[0].mode, n[1].mode, n[2].mode);
+		assert(FALSE);
+	}
+
 	add_merge_entry(entry);
 }
 

Re: Segfault in git merge-tree (1.8.2.2)

[John Keeping]

On Mon, May 06, 2013 at 03:29:23PM +0100, John Keeping wrote:
> On Mon, May 06, 2013 at 04:13:28PM +0200, Andreas Jacobsen wrote:
> > Sure, here you go, this time built from the HEAD I found on github
> > (7d3ccdffb5d28970dd7a4d177cfcca690ccd0c22) with:
> > 
> > NO_GETTEXT=1 make prefix=/usr/local/Cellar/git/HEAD CC=cc CFLAGS='-O0
> > -g' install (this is homebrew's setup, but I built it manually rather
> > than using the recipe.)
> > 
> > And the gdb output:
> > 
> > (gdb) set args merge-tree 027058e6ac8d03e029c4e1455bf90f63cd20e65b
> > FETCH_HEAD master
> > (gdb) run
> > Starting program: /usr/local/bin/git merge-tree
> > 027058e6ac8d03e029c4e1455bf90f63cd20e65b FETCH_HEAD master
> > Reading symbols for shared libraries +++++.............................. done
> > 
> > Program received signal EXC_BAD_ACCESS, Could not access memory.
> > Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
> > 0x000000010006f1a3 in add_merge_entry (entry=0x100432ba0) at
> > builtin/merge-tree.c:24
> > 24 *merge_result_end = entry;
> 
> Thanks.  I have an idea of what's going on, but the set up is in an
> earlier pass and it only fails the next time it gets into
> add_merge_entry.
> 
> Can you try adding the following patch on top?  Hopefully the added
> debug is in the right caller, otherwise the new assert at the top will
> point us to the right one.

Actually, don't worry about it.  I've tracked down the problem and have
a failing test case for t4300.  Patch to follow shortly.

[PATCH] merge-tree: handle directory/empty conflict correctly

[John Keeping]

git-merge-tree causes a null pointer dereference when a directory
entry exists in only one or two of the three trees being compared with
no corresponding entry in the other tree(s).

When this happens, we want to handle the entry as a directory and not
attempt to mark it as a file merge.  Do this by setting the entries bit
in the directory mask when the entry is missing or when it is a
directory, only performing the file comparison when we know that a file
entry exists.

Reported-by: Andreas Jacobsen <andreas@andreasjacobsen.com>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
Andreas, can you try this patch and see if it fixes your test case?

 builtin/merge-tree.c  |  6 +++++-
 t/t4300-merge-tree.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ec49917..61cbde4 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -251,7 +251,11 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3])
 
 	for (i = 0; i < 3; i++) {
 		mask |= (1 << i);
-		if (n[i].mode && S_ISDIR(n[i].mode))
+		/*
+		 * Treat missing entries as directories so that we return
+		 * after unresolved_directory has handled this.
+		 */
+		if (!n[i].mode || S_ISDIR(n[i].mode))
 			dirmask |= (1 << i);
 	}
 
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 2defb42..9015e47 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -259,6 +259,57 @@ EXPECTED
 	test_cmp expected actual
 '
 
+test_expect_success 'tree add A, B (same)' '
+	cat >expect <<-\EOF &&
+	EOF
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "file" add-tree-A &&
+	git merge-tree initial add-tree-A add-tree-A >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'tree add A, B (different)' '
+	cat >expect <<-\EOF &&
+	added in both
+	  our    100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/file
+	  their  100644 ba629238ca89489f2b350e196ca445e09d8bb834 sub/file
+	@@ -1 +1,5 @@
+	+<<<<<<< .our
+	 AAA
+	+=======
+	+BBB
+	+>>>>>>> .their
+	EOF
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "AAA" add-tree-a-b-A &&
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "BBB" add-tree-a-b-B &&
+	git merge-tree initial add-tree-a-b-A add-tree-a-b-B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'tree unchanged A, removed B' '
+	cat >expect <<-\EOF &&
+	removed in remote
+	  base   100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/file
+	  our    100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/file
+	@@ -1 +0,0 @@
+	-AAA
+	EOF
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "AAA" tree-remove-b-initial &&
+	git rm sub/file &&
+	test_tick &&
+	git commit -m "remove sub/file" &&
+	git tag tree-remove-b-B &&
+	git merge-tree tree-remove-b-initial tree-remove-b-initial tree-remove-b-B >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'turn file to tree' '
 	git reset --hard initial &&
 	rm initial-file &&
-- 
1.8.3.rc0.149.g98a72f2.dirty

Re: [PATCH] merge-tree: handle directory/empty conflict correctly

[Andreas Jacobsen]

On 6 May 2013 17:20, John Keeping <john@keeping.me.uk> wrote:
> Andreas, can you try this patch and see if it fixes your test case?

Worked perfectly. Thank you, John :)

Re: [PATCH] merge-tree: handle directory/empty conflict correctly

[Junio C Hamano]

Andreas Jacobsen <andreas@andreasjacobsen.com> writes:

> On 6 May 2013 17:20, John Keeping <john@keeping.me.uk> wrote:
>> Andreas, can you try this patch and see if it fixes your test case?
>
> Worked perfectly. Thank you, John :)

Thanks, both.