Re: Bug: Segmentation fault in git diff

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 18, 2021, 20:30 UTC
Message-ID: <xmqq5yw25wdu.fsf@gitster.g>
In-Reply-To: <YR0CkdVNVKa65vbo@danh.dev>

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

Show 18 quoted lines

> This diff could fix the issue, and the test suite still passes:
>
> ---- 8< ----
>
> diff --git a/diff-lib.c b/diff-lib.c
> index f9eadc4fc1..8f303958dd 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -192,7 +192,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>  			 * from the desired stage.
>  			 */
>  			pair = diff_unmerge(&revs->diffopt, ce->name);
> -			if (wt_mode)
> +			if (pair && wt_mode)
>  				pair->two->mode = wt_mode;
>  			if (ce_stage(ce) != diff_unmerged_stage)
>  				continue;
> ---- >8 -----

Looks sensible.

The bug is the result of a bad interaction between cd676a51 (diff --relative: output paths as relative to the current subdirectory, 2008-02-12) that decided to return NULL from diff_unmerge for an irrelevant path outside the --relative area, and 095ce953 (diff-files: show unmerged entries correctly, 2011-04-22) that assumed diff_unmerge() would always yield a usable unmerged pair.

As the NULL pair would never be added to the diff_queue(), when the big loop around this area of code finishes and hands the processing off to diffcore_std(), the mode futzing the original code was attempting to do would have affected absolutely nothing, so I do not think there would be any subtle breakage coming from this fix.

When you submit an "git am"-able patch, please have my Acked-by.

Thanks.