diffcore-break: avoid segfault with freed entries

After we have freed the file pair, we should set the queue reference to null.
When computing a diff in a partial clone, there is a chance that we
could trigger a prefetch of missing objects when there are freed entries in
the global diff queue due to break-rewrites detection. The segfault only occurs
if an entry has been freed by break-rewrites and there is an entry
to be prefetched.

There is a new test in t4067 that trigger the segmentation fault that results
in this case. The test explicitly fetch the necessary blobs to trigger the
break rewrites, some blobs are left to be prefetched.

The fix is to set the queue pointer to NULL after it is freed, the prefetch
will skip NULL entries.

Signed-off-by: Han Young <hanyang.tony@bytedance.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diffcore-break.c b/diffcore-break.c
index c4c2173f3096bcd4f5935ec16ba9c18b9732f9e6..9b11fe2fa0e622afaa9ad7ab4af81e1a68d21482 100644 (file)
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -222,6 +222,7 @@ void diffcore_break(struct repository *r, int break_score)
                                free(p); /* not diff_free_filepair(), we are
                                          * reusing one and two here.
                                          */
+                               q->queue[i] = NULL;
                                continue;
                        }
                }

diff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh
index 72f25de44950aeabec6fe921e4c9542b678a1679..30813109ac044e145007b8b00c83355a12101435 100755 (executable)
--- a/t/t4067-diff-partial-clone.sh
+++ b/t/t4067-diff-partial-clone.sh
@@ -132,6 +132,37 @@ test_expect_success 'diff with rename detection batches blobs' '
        test_line_count = 1 done_lines
 '
 
+test_expect_success 'diff succeeds even if prefetch triggered by break-rewrites' '
+       test_when_finished "rm -rf server client trace" &&
+
+       test_create_repo server &&
+       echo xyz >server/foo &&
+       mkdir server/bar &&
+       test_seq -f "line %d" 1 100 >server/bar/baz &&
+       git -C server add -A &&
+       git -C server commit -m x &&
+
+       echo xyzz >server/foo &&
+       test_seq -f "line %d" 90 190 >server/bar/baz &&
+       git -C server add -A &&
+       git -C server commit -m x &&
+
+       test_config -C server uploadpack.allowfilter 1 &&
+       test_config -C server uploadpack.allowanysha1inwant 1 &&
+       git clone --filter=blob:limit=0 "file://$(pwd)/server" client &&
+
+       # Fetch bar/baz without fetching foo.
+       # Foo will be lazily fetched during break rewrites detection.
+       git -C client checkout HEAD~1 bar &&
+
+       # Ensure baz in the working tree is different from baz in HEAD~1.
+       # We need baz to trigger break-rewrites detection.
+       git -C client reset --hard HEAD &&
+
+       # break-rewrites detction in reset.
+       git -C client reset HEAD~1
+'
+
 test_expect_success 'diff succeeds even if entries are removed from queue' '
        test_when_finished "rm -rf server client trace" &&