Re: Git 2.19 Segmentation fault 11 on macOS

From: Thomas Gummerer <t.gummerer@gmail.com>
Date: Sep 11, 2018, 17:29 UTC
Message-ID: <20180911172903.GC4865@hank.intra.tgummerer.com>
In-Reply-To: <20180911163419.GB4865@hank.intra.tgummerer.com>

On 09/11, Thomas Gummerer wrote:

Show 47 quoted lines

> I think you're on the right track here.  I can not test this on Mac
> OS, but on Linux, the following fails when running the test under
> valgrind:
> 
>     diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
>     index 2237c7f4af..a8b0ef8c1d 100755
>     --- a/t/t3206-range-diff.sh
>     +++ b/t/t3206-range-diff.sh
>     @@ -142,4 +142,9 @@ test_expect_success 'changed message' '
>             test_cmp expected actual
>      '
>      
>     +test_expect_success 'amend and check' '
>     +       git commit --amend -m "new message" &&
>     +       git range-diff master HEAD@{1} HEAD
>     +'
>     +
>      test_done
> 
> valgrind gives me the following:
> 
> ==18232== Invalid read of size 4
> ==18232==    at 0x34D7B5: compute_assignment (linear-assignment.c:54)
> ==18232==    by 0x2A4253: get_correspondences (range-diff.c:245)
> ==18232==    by 0x2A4BFB: show_range_diff (range-diff.c:427)
> ==18232==    by 0x19D453: cmd_range_diff (range-diff.c:108)
> ==18232==    by 0x122698: run_builtin (git.c:418)
> ==18232==    by 0x1229D8: handle_builtin (git.c:637)
> ==18232==    by 0x122BCC: run_argv (git.c:689)
> ==18232==    by 0x122D90: cmd_main (git.c:766)
> ==18232==    by 0x1D55A3: main (common-main.c:45)
> ==18232==  Address 0x4f4d844 is 0 bytes after a block of size 4 alloc'd
> ==18232==    at 0x483777F: malloc (vg_replace_malloc.c:299)
> ==18232==    by 0x3381B0: do_xmalloc (wrapper.c:60)
> ==18232==    by 0x338283: xmalloc (wrapper.c:87)
> ==18232==    by 0x2A3F8C: get_correspondences (range-diff.c:207)
> ==18232==    by 0x2A4BFB: show_range_diff (range-diff.c:427)
> ==18232==    by 0x19D453: cmd_range_diff (range-diff.c:108)
> ==18232==    by 0x122698: run_builtin (git.c:418)
> ==18232==    by 0x1229D8: handle_builtin (git.c:637)
> ==18232==    by 0x122BCC: run_argv (git.c:689)
> ==18232==    by 0x122D90: cmd_main (git.c:766)
> ==18232==    by 0x1D55A3: main (common-main.c:45)
> ==18232== 
> 
> I'm looking into why that fails.  Also adding Dscho to Cc here as the
> author of this code.

The diff below seems to fix it. Not submitting this as a proper patch, as I don't quite understand what the original code tried to do here. However this does pass all tests we currently have and fixes the out of bounds memory read that's caught by valgrind (and that I imagine could cause the segfault on Mac OS).

This matches how the initial minimum for the reduction transfer is calculated in [1].

I'll try to convince myself of the right solution, but should someone more familiar with the linear-assignment algorithm have an idea, feel free to take this over :)

[1]: https://github.com/src-d/lapjv/blob/master/lap.h#L276

--- >8 ---

diff --git a/linear-assignment.c b/linear-assignment.c
index 9b3e56e283..ab0aa5fd41 100644
--- a/linear-assignment.c
+++ b/linear-assignment.c
@@ -51,7 +51,7 @@ void compute_assignment(int column_count, int row_count, int *cost,
 		else if (j1 < -1)
 			row2column[i] = -2 - j1;
 		else {
-			int min = COST(!j1, i) - v[!j1];
+			int min = INT_MAX;
 			for (j = 1; j < column_count; j++)
 				if (j != j1 && min > COST(j, i) - v[j])
 					min = COST(j, i) - v[j];
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 2237c7f4af..a8b0ef8c1d 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -142,4 +142,9 @@ test_expect_success 'changed message' '
 	test_cmp expected actual
 '
 
+test_expect_success 'amend and check' '
+	git commit --amend -m "new message" &&
+	git range-diff master HEAD@{1} HEAD
+'
+
 test_done

--- >8 ---