git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] xdiff: clamp function context indices in post-image

From
Jeff King <peff@peff.net>
Date
Jul 23, 2019, 19:27 UTC
Message-ID
<20190723192704.GA4065@sigill.intra.peff.net>

After finding a function line for --function-context in the pre-image, xdl_emit_diff() calculates the equivalent line in the post-image. It assumes that the lines between changes are the same on both sides. If the option --ignore-blank-lines was also given then this is not necessarily true.

Clamp the calculation results for start and end of the function context to prevent out-of-bounds array accesses.

Note that this _just_ fixes the case where our mismatch sends us off the beginning of the file. There are likely other cases where our assumption causes us to go to the wrong line within the file. Nobody has developed a test case yet, and the ultimate fix is likely more complicated than this patch. But this at least prevents a segfault in the meantime.

Credit for finding the bug goes to "Liu Wei of Tencent Security Xuanwu Lab".

Reported-by: 刘炜 <lw17qhdz@gmail.com>
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/t4015-diff-whitespace.sh | 22 ++++++++++++++++++++++
 xdiff/xemit.c              |  4 ++--
 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index ab4670d236..6b087df3dc 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -2008,4 +2008,26 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '
 	test_cmp expected actual
 '
 
+# Note that the "6" in the expected hunk header below is funny, since we only
+# show 5 lines (the missing one was blank and thus ignored). This is how
+# --ignore-blank-lines behaves even without --function-context, and this test
+# is just checking the interaction of the two features. Don't take it as an
+# endorsement of that output.
+test_expect_success 'combine --ignore-blank-lines with --function-context' '
+	test_write_lines 1 "" 2 3 4 5 >a &&
+	test_write_lines 1    2 3 4   >b &&
+	test_must_fail git diff --no-index \
+		--ignore-blank-lines --function-context a b >actual.raw &&
+	sed -n "/@@/,\$p" <actual.raw >actual &&
+	cat <<-\EOF >expect &&
+	@@ -1,6 +1,4 @@
+	 1
+	 2
+	 3
+	 4
+	-5
+	EOF
+	test_cmp expect actual
+'
+
 test_done
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 7778dc2b19..30713ae9a9 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -210,7 +210,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 			if (fs1 < 0)
 				fs1 = 0;
 			if (fs1 < s1) {
-				s2 -= s1 - fs1;
+				s2 = XDL_MAX(s2 - (s1 - fs1), 0);
 				s1 = fs1;
 			}
 		}
@@ -232,7 +232,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 			if (fe1 < 0)
 				fe1 = xe->xdf1.nrec;
 			if (fe1 > e1) {
-				e2 += fe1 - e1;
+				e2 = XDL_MIN(e2 + (fe1 - e1), xe->xdf2.nrec);
 				e1 = fe1;
 			}
 
-- 
2.22.0.993.gcc1030c86b
Message 1 of 1 in “xdiff: clamp function context indices in post-image”
  1. xdiff: clamp function context indices in post-imageJeff King, Jul 23, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.