[PATCH 0/2] grep, diff: do not silently truncate sizes of 4GiB or more

[PATCH 0/2] grep, diff: do not silently truncate sizes of 4GiB or more

[Elia Pinto]

This series continues the search for truncated sizes on platforms where
"long" is 32 bits but size_t is 64 bits (64-bit Windows). This time it
covers grep and diff, where a file of 4GiB or more produced wrong
results without any error.

- 1/2 grep: on the working tree and --no-index, it silently missed
  matches beyond the truncated size. Now it uses size_t and works,
  for blobs too.
- 2/2 diff: on the working tree, textconv, --no-index and combine-diff,
  it produced a diff of a prefix, and a patch that truncates the file
  when applied (as git add -p does). Now it dies with a clear error,
  like the blob side already did.

Of the remaining -Wshorten-64-to-32 warnings in the diff and grep code,
these are the only reachable ones. The others are in xdiff internals
(inputs limited to MAX_XDIFF_SIZE, 1023MiB), in buffer_is_binary()
(which looks at 8000 bytes), or in places where the size can't exceed
4GiB. Widening diff_filespec.size to size_t, so that diff actually works
above 4GiB, is a much bigger change, left for later.

No tests, as they would need 4GiB of input on such a platform. Both
effects were verified by simulating the truncation at 16 bits, and the
commit messages give the details.

It doesn't depend on the other series in flight and merges cleanly with
them. Based on master 50fe5cc63f.

Elia Pinto (2):
  grep: use size_t for the size of buffers to search
  diff: die instead of truncating sizes of 4GiB or more

 combine-diff.c  | 11 +++++++----
 diff-no-index.c |  2 +-
 diff.c          |  8 ++++----
 grep.c          | 16 ++++++++--------
 grep.h          |  4 ++--
 5 files changed, 22 insertions(+), 19 deletions(-)


base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
2.53.0

[PATCH 1/2] grep: use size_t for the size of buffers to search

[Elia Pinto]

struct grep_source tracks the size of the search buffer in an "unsigned
long", while grep_source_load_file() reads worktree files into memory
using size_t. On platforms where "long" is 32 bits but size_t is 64 bits
(such as 64-bit Windows), processing a file of 4GiB or more truncates
the size, causing "git grep" to search only the first "size mod 2^32"
bytes and silently miss matches beyond that threshold. With gs->size
truncated to 16 bits, "git grep" and "git grep --no-index" find nothing
in a 100KiB file whose last line matches, while grepping the same file
as a blob finds it.

Use size_t for struct grep_source.size and across the internal matching
engine functions in grep.c and grep.h. Additionally, remove the
cast_size_t_to_ulong() call in grep_source_load_oid(), allowing grepping
blobs of 4GiB or more to succeed instead of aborting.

Triggering this requires processing 4GiB or more of input on a platform
where "long" is 32 bits but size_t is 64 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 grep.c | 16 ++++++++--------
 grep.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/grep.c b/grep.c
index 1d75d31421..a8c01b4737 100644
--- a/grep.c
+++ b/grep.c
@@ -864,9 +864,9 @@ void free_grep_patterns(struct grep_opt *opt)
 		free_pattern_expr(opt->pattern_expression);
 }
 
-static const char *end_of_line(const char *cp, unsigned long *left)
+static const char *end_of_line(const char *cp, size_t *left)
 {
-	unsigned long l = *left;
+	size_t l = *left;
 	while (l && *cp != '\n') {
 		l--;
 		cp++;
@@ -1454,7 +1454,7 @@ static int should_lookahead(struct grep_opt *opt)
 }
 
 static int look_ahead(struct grep_opt *opt,
-		      unsigned long *left_p,
+		      size_t *left_p,
 		      unsigned *lno_p,
 		      const char **bol_p)
 {
@@ -1567,7 +1567,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 {
 	const char *bol;
 	const char *peek_bol = NULL;
-	unsigned long left;
+	size_t left;
 	unsigned lno = 1;
 	unsigned last_hit = 0;
 	int binary_match_only = 0;
@@ -1735,7 +1735,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 			goto next_line;
 		}
 		if (show_function && (!peek_bol || peek_bol < bol)) {
-			unsigned long peek_left = left;
+			size_t peek_left = left;
 			const char *peek_eol = eol;
 
 			/*
@@ -1854,7 +1854,7 @@ int grep_source(struct grep_opt *opt, struct grep_source *gs)
 
 static void grep_source_init_buf(struct grep_source *gs,
 				 const char *buf,
-				 unsigned long size)
+				 size_t size)
 {
 	gs->type = GREP_SOURCE_BUF;
 	gs->name = NULL;
@@ -1865,7 +1865,7 @@ static void grep_source_init_buf(struct grep_source *gs,
 	gs->identifier = NULL;
 }
 
-int grep_buffer(struct grep_opt *opt, const char *buf, unsigned long size)
+int grep_buffer(struct grep_opt *opt, const char *buf, size_t size)
 {
 	struct grep_source gs;
 	int r;
@@ -1935,7 +1935,7 @@ static int grep_source_load_oid(struct grep_source *gs)
 
 	gs->buf = odb_read_object(gs->repo->objects, gs->identifier,
 				  &type, &size_st);
-	gs->size = cast_size_t_to_ulong(size_st);
+	gs->size = size_st;
 	if (!gs->buf)
 		return error(_("'%s': unable to read %s"),
 			     gs->name,
diff --git a/grep.h b/grep.h
index 13e26a9318..0bd705dfc0 100644
--- a/grep.h
+++ b/grep.h
@@ -212,7 +212,7 @@ void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *orig
 void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);
 void compile_grep_patterns(struct grep_opt *opt);
 void free_grep_patterns(struct grep_opt *opt);
-int grep_buffer(struct grep_opt *opt, const char *buf, unsigned long size);
+int grep_buffer(struct grep_opt *opt, const char *buf, size_t size);
 
 /* The field parameter is only used to filter header patterns
  * (where appropriate). If filtering isn't desirable
@@ -235,7 +235,7 @@ struct grep_source {
 	struct repository *repo; /* if GREP_SOURCE_OID */
 
 	const char *buf;
-	unsigned long size;
+	size_t size;
 
 	char *path; /* for attribute lookups */
 	struct userdiff_driver *driver;
-- 
2.53.0

[PATCH 2/2] diff: die instead of truncating sizes of 4GiB or more

[Elia Pinto]

On platforms where "long" is 32 bits but size_t is 64 bits, such as
64-bit Windows, worktree files, textconv outputs, convert_to_git()
results, and temporary diff files of 4GiB or larger have their sizes
silently truncated when they are stored in an "unsigned long", such as
the size field of struct diff_filespec.

This causes "git diff" to produce a silent incorrect diff based on a
prefix, and commands like "git add -p" to apply a patch that truncates
the file. With the worktree size truncated to 16 bits, "git diff --stat"
reports 1 insertion and 5042 deletions for a one-line change in a
7901-line file, and applying the resulting patch leaves 2859 lines.

Use cast_size_t_to_ulong() in these code paths, matching the blob
side, so that Git dies with a clear error instead of producing wrong
results or corrupted patches. Widening diff_filespec.size is a much
larger change left for a separate patch.

Triggering this requires processing 4GiB or more of input on a platform
where "long" is 32 bits but size_t is 64 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 combine-diff.c  | 11 +++++++----
 diff-no-index.c |  2 +-
 diff.c          |  8 ++++----
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 717d537688..edf0427a80 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -322,7 +322,8 @@ static char *grab_blob(struct repository *r,
 	} else if (textconv) {
 		struct diff_filespec *df = alloc_filespec(path);
 		fill_filespec(df, oid, 1, mode);
-		*size = fill_textconv(r, textconv, df, &blob);
+		*size = cast_size_t_to_ulong(fill_textconv(r, textconv,
+							   df, &blob));
 		free_filespec(df);
 	} else {
 		size_t size_st = 0;
@@ -1069,7 +1070,9 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 		} else if (textconv) {
 			struct diff_filespec *df = alloc_filespec(elem->path);
 			fill_filespec(df, null_oid(the_hash_algo), 0, st.st_mode);
-			result_size = fill_textconv(opt->repo, textconv, df, &result);
+			result_size = cast_size_t_to_ulong(
+				fill_textconv(opt->repo, textconv,
+					      df, &result));
 			free_filespec(df);
 		} else if (0 <= (fd = open(elem->path, O_RDONLY))) {
 			size_t len = xsize_t(st.st_size);
@@ -1086,7 +1089,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			if (!is_file)
 				elem->mode = canon_mode(S_IFLNK);
 
-			result_size = len;
+			result_size = cast_size_t_to_ulong(len);
 			result = xmallocz(len);
 
 			done = read_in_full(fd, result, len);
@@ -1103,7 +1106,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 						   elem->path, result, len, &buf, global_conv_flags_eol)) {
 					free(result);
 					result = strbuf_detach(&buf, &len);
-					result_size = len;
+					result_size = cast_size_t_to_ulong(len);
 				}
 			}
 		}
diff --git a/diff-no-index.c b/diff-no-index.c
index a9151cc27a..dee80eb4ed 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -116,7 +116,7 @@ static void populate_common(struct diff_filespec *s, struct strbuf *buf)
 
 	s->should_munmap = 0;
 	s->data = strbuf_detach(buf, &size);
-	s->size = size;
+	s->size = cast_size_t_to_ulong(size);
 	s->should_free = 1;
 	s->is_stdin = 1;
 }
diff --git a/diff.c b/diff.c
index b4ac17f8ef..9b1c69b378 100644
--- a/diff.c
+++ b/diff.c
@@ -4516,7 +4516,7 @@ int diff_populate_filespec(struct repository *r,
 			s->size = 0;
 			return err;
 		}
-		s->size = xsize_t(st.st_size);
+		s->size = cast_size_t_to_ulong(xsize_t(st.st_size));
 		if (!s->size)
 			goto empty;
 		if (S_ISLNK(st.st_mode)) {
@@ -4569,7 +4569,7 @@ int diff_populate_filespec(struct repository *r,
 			munmap(s->data, s->size);
 			s->should_munmap = 0;
 			s->data = strbuf_detach(&buf, &size);
-			s->size = size;
+			s->size = cast_size_t_to_ulong(size);
 			s->should_free = 1;
 		}
 	}
@@ -4662,7 +4662,7 @@ static void prep_temp_blob(struct index_state *istate,
 	if (convert_to_working_tree(istate, path,
 			(const char *)blob, (size_t)size, &buf, &meta)) {
 		blob = buf.buf;
-		size = buf.len;
+		size = cast_size_t_to_ulong(buf.len);
 	}
 	if (write_in_full(temp->tempfile->fd, blob, size) < 0 ||
 	    close_tempfile_gently(temp->tempfile))
@@ -7838,7 +7838,7 @@ int textconv_object(struct repository *r,
 		return 0;
 	}
 
-	*buf_size = fill_textconv(r, textconv, df, buf);
+	*buf_size = cast_size_t_to_ulong(fill_textconv(r, textconv, df, buf));
 	free_filespec(df);
 	return 1;
 }
-- 
2.53.0