Skip to content

Commit 98bfb51

Browse files
authored
diff: avoid segfault with freed entries (#835)
This is also going upstream as part of gitgitgadget#2027. However, I found out about this segfault due to a reproducible error happening in the 1JS monorepo's PR build pipelines. After trying to uncover a repro by adding tracing to my own fork and using that version within the pipeline, I was finally able to get a reproducer in a 1JS Codespace. Debugging with a locally-build version of Git helped me find the situation to fix this. 1JS has a workaround for this issue by running `git status` before the necessary build step that runs the `git diff` command. That refresh of the index prevents the freed diff queue items, preventing the issue. This reduces the urgency somewhat, but I also don't know where else this could be impacting users. * [X] This is an early version of work already under review upstream. I'm recommending this version on top of the `vfs-2.52.0` branch so we can get this fixed more quickly for an upcoming release of microsoft/git.
2 parents 26ef682 + 98e06c8 commit 98bfb51

2 files changed

Lines changed: 40 additions & 0 deletions

File tree

diff.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7053,6 +7053,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
70537053
if (!diffopt->flags.no_index)
70547054
diffopt->skip_stat_unmatch++;
70557055
diff_free_filepair(p);
7056+
q->queue[i] = NULL;
70567057
}
70577058
}
70587059
free(q->queue);
@@ -7096,6 +7097,10 @@ void diff_queued_diff_prefetch(void *repository)
70967097

70977098
for (i = 0; i < q->nr; i++) {
70987099
struct diff_filepair *p = q->queue[i];
7100+
7101+
if (!p)
7102+
continue;
7103+
70997104
diff_add_if_missing(repo, &to_fetch, p->one);
71007105
diff_add_if_missing(repo, &to_fetch, p->two);
71017106
}

t/t4067-diff-partial-clone.sh

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,41 @@ test_expect_success 'diff with rename detection batches blobs' '
132132
test_line_count = 1 done_lines
133133
'
134134

135+
test_expect_success 'diff succeeds even if entries are removed from queue' '
136+
test_when_finished "rm -rf server client trace" &&
137+
138+
test_create_repo server &&
139+
for l in a c e g i p
140+
do
141+
echo $l >server/$l &&
142+
git -C server add $l || return 1
143+
done &&
144+
git -C server commit -m x &&
145+
146+
for l in a e i
147+
do
148+
git -C server rm $l || return 1
149+
done &&
150+
151+
for l in b d f i
152+
do
153+
echo $l$l >server/$l &&
154+
git -C server add $l || return 1
155+
done &&
156+
git -C server commit -a -m x &&
157+
158+
test_config -C server uploadpack.allowfilter 1 &&
159+
test_config -C server uploadpack.allowanysha1inwant 1 &&
160+
git clone --filter=blob:limit=0 "file://$(pwd)/server" client &&
161+
162+
for file in $(ls client)
163+
do
164+
cat client/$file >$file &&
165+
mv $file client/$file || return 1
166+
done &&
167+
git -C client diff --name-only --relative HEAD^
168+
'
169+
135170
test_expect_success 'diff does not fetch anything if inexact rename detection is not needed' '
136171
test_when_finished "rm -rf server client trace" &&
137172

0 commit comments

Comments
 (0)