[PATCH 0/3] use size_t for a few more object and buffer sizes

[PATCH 0/3] use size_t for a few more object and buffer sizes

[Elia Pinto]

On 64-bit Windows, "long" is still 32 bits wide. Whenever a size_t
travels through an "unsigned long", anything above 4 GiB is silently
lost. Johannes's js/objects-larger-than-4gb-on-windows work fixed the
read side of the object database, and Peff's jk/xdiff-size_t did the
same for the diff and merge machinery. This series picks up three
places that were left narrow.

 - odb: the write API still takes the object length as unsigned long.
   A large file that is converted before being stored (for example
   with core.autocrlf, which Git for Windows enables by default) can
   reach odb_write_object() with a size_t length above 4 GiB. The
   blob is then written truncated, with no error, and the index
   happily records it.

 - diff: diff_words_fill() grows its buffer with a signed "long"
   allocation size. Its output can be twice its input, and on LLP64
   the growth computation overflows past about 700MiB, which is well
   within what xdiff accepts. In practice Git dies on a huge
   allocation request.

 - convert: the helpers that decide whether a file is binary, has
   CRLF line endings or "$Id$" keywords take an unsigned long length.
   For files over 4GiB they only look at a prefix. In the same area,
   read_blob_data_from_index() narrows its result with
   cast_size_t_to_ulong(), so "git add" with core.autocrlf dies when
   the staged version is that large.

There are no new tests. Each case needs files of several GiB and a
32-bit "long", and the commit messages explain how to reproduce them.

How these were found
--------------------

I wanted a list of all the places where a 64-bit size is narrowed on
Windows, rather than waiting for the next bug report. Linux compilers
cannot see the problem, because there "long" and "size_t" have the
same width. So I analysed the tree as a Windows target instead.

The first step is a compilation database from a real MinGW
configuration. Forcing --target onto a database generated on Linux
does not work well: Linux-only defines leak in, regex.h is missing
from MinGW, and the precompiled header is rejected for the wrong
target. A Meson cross build avoids all of that:

  $ cat mingw64.cross
  [binaries]
  c       = 'x86_64-w64-mingw32-gcc'
  ar      = 'x86_64-w64-mingw32-ar'
  strip   = 'x86_64-w64-mingw32-strip'
  windres = 'x86_64-w64-mingw32-windres'

  [host_machine]
  system     = 'windows'
  cpu_family = 'x86_64'
  cpu        = 'x86_64'
  endian     = 'little'

  $ meson setup build-win --cross-file mingw64.cross \
	-Db_pch=false -Dsane_tool_path=/usr/bin \
	-Dcurl=disabled -Dexpat=disabled -Dpcre2=disabled \
	-Dgettext=disabled -Diconv=disabled -Drust=disabled
  $ meson compile -C build-win

Two of these options are not obvious. Without -Dsane_tool_path,
setup stops because meson.build adds "C:/Program Files/Git/bin" to
the program search path, which is not absolute on a Linux host. And
-Db_pch=false is needed because clang cannot read the precompiled
header that GCC produces. The build only has to get far enough to
generate the headers; link errors do not matter here.

The second step runs clang-tidy over that database with
-Wshorten-64-to-32, for the x86_64-w64-mingw32 target, skipping the
bundled zlib and OpenSSL. One detail cost me a while: with
"-checks=-*", clang-tidy drops compiler warnings unless
clang-diagnostic-shorten-64-to-32 is enabled explicitly. I wrapped
all this in a small script, which also refuses to report "no
findings" when some translation units failed to parse:

  https://github.com/devzero2000/toyscript/blob/main/check_llp64_portability.sh

It is not meant for inclusion. I mention it so that the numbers below
can be reproduced, and in case someone finds it useful.

What the scan shows
-------------------

All 569 translation units were analysed, none failed. There are 1432
64-to-32 truncation warnings, but most of them are not specific to
Windows: about 1200 narrow a size_t to an int, as they do on Linux.
Only 237 have "long" or "unsigned long" as their destination.

Most of those 237 sit on API boundaries that are known and that
commit c6a4629e32 names explicitly: diff, blame, textconv and
fast-import. Others are in tree walking, object parsing and the pack
tools, where an oversized object makes Git die rather than misbehave.
I would like to convert them in later small series, one area at a
time, in the same style as jk/xdiff-size_t.

Johannes, the first patch continues your write-side work. I hope it
does not step on a follow-up you already have in progress.

Elia Pinto (3):
  odb: use size_t for the length of objects to write
  diff: use size_t for the allocation size of word-diff buffers
  convert: use size_t for buffer sizes

 attr.c                |  2 +-
 convert.c             | 14 +++++++-------
 diff.c                |  4 ++--
 odb.c                 |  2 +-
 odb.h                 |  4 ++--
 odb/source-inmemory.c |  2 +-
 odb/source-loose.c    |  2 +-
 odb/source.h          |  2 +-
 read-cache-ll.h       |  2 +-
 read-cache.c          |  4 ++--
 10 files changed, 19 insertions(+), 19 deletions(-)


base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926
--
2.53.0

[PATCH 1/3] odb: use size_t for the length of objects to write

[Elia Pinto]

c6a4629e32 (odb: use size_t for object_info.sizep and the size APIs,
2026-06-15) widened the read side of the object database API, and the
write_object() callback of odb_source already takes a size_t. But
odb_write_object_ext(), odb_write_object() and odb_source_write_object()
in front of it, as well as write_loose_object() behind it, still take
an "unsigned long", which is only 32 bits wide on Windows.

Most callers hand them a size_t from a strbuf or from odb_read_object(),
which silently gets truncated. The most visible case is index_mem() and
index_stream_convert_blob(): when "git add" or "git hash-object -w"
runs a file over 4GiB through a conversion (core.autocrlf, which is on
by default in Git for Windows, an eol or ident attribute, or a clean
filter), we write a blob made of the first "size mod 2^32" bytes. The
index records it with the stat data of the real file, so "git status"
stays clean and nobody notices until the file is checked out again.
"git hash-object" without "-w" uses hash_object_file(), which takes a
size_t, so the two disagree on the object ID of the same file.

Use size_t throughout, including for the size kept by the in-memory
source.

The existing "hash filtered files over 4GB correctly" test in t1007
only hashes. Running the same with "git hash-object -w" on Windows
shows the problem, but needs another pass over a 5GiB file, so no new
test is added here.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 odb.c                 | 2 +-
 odb.h                 | 4 ++--
 odb/source-inmemory.c | 2 +-
 odb/source-loose.c    | 2 +-
 odb/source.h          | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index 98c5c862f5..6e9daf11e4 100644
--- a/odb.c
+++ b/odb.c
@@ -667,7 +667,7 @@ void odb_assert_oid_type(struct object_database *odb,
 }
 
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid_in,
diff --git a/odb.h b/odb.h
index 0ccc47ee73..25d02fae75 100644
--- a/odb.h
+++ b/odb.h
@@ -633,14 +633,14 @@ enum odb_write_object_flags {
  * Returns 0 on success, a negative error code otherwise.
  */
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid,
 			 enum odb_write_object_flags flags);
 
 static inline int odb_write_object(struct object_database *odb,
-				   const void *buf, unsigned long len,
+				   const void *buf, size_t len,
 				   enum object_type type,
 				   struct object_id *oid)
 {
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index 868e723c81..ab7173cd2d 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -10,7 +10,7 @@
 struct inmemory_object {
 	enum object_type type;
 	const void *buf;
-	unsigned long size;
+	size_t size;
 };
 
 static const struct inmemory_object *find_cached_object(struct odb_source_inmemory *source,
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 0664634938..fe1a298364 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -770,7 +770,7 @@ static int end_loose_object_common(struct odb_source_loose *loose,
 
 static int write_loose_object(struct odb_source_loose *loose,
 			      const struct object_id *oid, char *hdr,
-			      int hdrlen, const void *buf, unsigned long len,
+			      int hdrlen, const void *buf, size_t len,
 			      const time_t *mtime, unsigned flags)
 {
 	int fd, ret;
diff --git a/odb/source.h b/odb/source.h
index 1867347dfc..8e6ed96399 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -469,7 +469,7 @@ static inline int odb_source_freshen_object(struct odb_source *source,
  * object ID and the compatibility object ID, if non-NULL.
  */
 static inline int odb_source_write_object(struct odb_source *source,
-					  const void *buf, unsigned long len,
+					  const void *buf, size_t len,
 					  enum object_type type,
 					  const struct object_id *oid,
 					  const struct object_id *compat_oid,
-- 
2.53.0

Re: [PATCH 1/3] odb: use size_t for the length of objects to write

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 12:16:37PM +0200, Elia Pinto wrote:
> c6a4629e32 (odb: use size_t for object_info.sizep and the size APIs,
> 2026-06-15) widened the read side of the object database API, and the
> write_object() callback of odb_source already takes a size_t. But
> odb_write_object_ext(), odb_write_object() and odb_source_write_object()
> in front of it, as well as write_loose_object() behind it, still take
> an "unsigned long", which is only 32 bits wide on Windows.

Sometimes I wish that C would give us an easier way to detect such
mismatches. It of course does have optional warnings, but enabling those
now would probably lead to thousands of warnings.

> Most callers hand them a size_t from a strbuf or from odb_read_object(),
> which silently gets truncated. The most visible case is index_mem() and
> index_stream_convert_blob(): when "git add" or "git hash-object -w"
> runs a file over 4GiB through a conversion (core.autocrlf, which is on
> by default in Git for Windows, an eol or ident attribute, or a clean
> filter), we write a blob made of the first "size mod 2^32" bytes. The
> index records it with the stat data of the real file, so "git status"
> stays clean and nobody notices until the file is checked out again.
> "git hash-object" without "-w" uses hash_object_file(), which takes a
> size_t, so the two disagree on the object ID of the same file.

Awesome.

> Use size_t throughout, including for the size kept by the in-memory
> source.
> 
> The existing "hash filtered files over 4GB correctly" test in t1007
> only hashes. Running the same with "git hash-object -w" on Windows
> shows the problem, but needs another pass over a 5GiB file, so no new
> test is added here.

But wouldn't it make sense to add a test? It's a rather serious bug, and
the existing tests are marked with EXPENSIVE already anyway. So I don't
think this is a good enough argument to not add another EXPENSIVE test.

> diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
> index 868e723c81..ab7173cd2d 100644
> --- a/odb/source-inmemory.c
> +++ b/odb/source-inmemory.c
> @@ -10,7 +10,7 @@
>  struct inmemory_object {
>  	enum object_type type;
>  	const void *buf;
> -	unsigned long size;
> +	size_t size;
>  };

I really hope nobody writes 4GB blobs into memory :) But despite that,
this fix is correct anyway.

Patrick

[PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers

[Elia Pinto]

Since 3c7a31204f (xdiff: use size_t for buffer sizes, 2026-09-30) the
size of an mmfile_t is a size_t, but the word-diff code still grows
these buffers with ALLOC_GROW() using an "unsigned long" (in struct
diff_words_buffer) or a "long" (in diff_words_fill()) for the allocated
size.

On platforms where "long" is 32 bits wide this is not just cosmetic.
diff_words_fill() writes every word followed by a newline, so its
output can be up to twice as large as its input, which is limited by
MAX_XDIFF_SIZE to just under 1GiB. alloc_nr() computes (alloc + 16) * 3
in the type of "alloc", which overflows a signed 32-bit "long" as soon
as the buffer grows past about 700MiB. The result is undefined behavior,
in practice a negative size that REALLOC_ARRAY() turns into a huge
allocation request and a die().

The buffer in diff_words_append() is bounded by MAX_XDIFF_SIZE, so
there it is only a matter of consistency, but if that limit ever went
away, ALLOC_GROW() would truncate the requested size, and the memcpy()
right after it would overflow the buffer.

Use size_t for both.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index b4ac17f8ef..9483931c05 100644
--- a/diff.c
+++ b/diff.c
@@ -1959,7 +1959,7 @@ static void emit_rewrite_diff(const char *name_a,
 
 struct diff_words_buffer {
 	mmfile_t text;
-	unsigned long alloc;
+	size_t alloc;
 	struct diff_words_orig {
 		const char *begin, *end;
 	} *orig;
@@ -2192,7 +2192,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
 		regex_t *word_regex)
 {
 	int i, j;
-	long alloc = 0;
+	size_t alloc = 0;
 
 	out->size = 0;
 	out->ptr = NULL;
-- 
2.53.0

Re: [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 12:16:38PM +0200, Elia Pinto wrote:
> diff --git a/diff.c b/diff.c
> index b4ac17f8ef..9483931c05 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1959,7 +1959,7 @@ static void emit_rewrite_diff(const char *name_a,
>  
>  struct diff_words_buffer {
>  	mmfile_t text;
> -	unsigned long alloc;
> +	size_t alloc;
>  	struct diff_words_orig {
>  		const char *begin, *end;
>  	} *orig;
> @@ -2192,7 +2192,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
>  		regex_t *word_regex)
>  {
>  	int i, j;
> -	long alloc = 0;
> +	size_t alloc = 0;

This is especially offending. I double checked that we really don't make
use of a potential negative value, but all this is ever used for is
`ALLOC_GROW()`.

Patrick

[PATCH 3/3] convert: use size_t for buffer sizes

[Elia Pinto]

The conversion code receives its buffers with a size_t length, but the
helpers that scan them, gather_stats(), gather_convert_stats(),
gather_convert_stats_ascii() and count_ident(), take an "unsigned long".
On platforms where that is 32 bits wide, like Windows, they only look
at the first "size mod 2^32" bytes of a file over 4GiB. The decision
whether a file is binary, or contains CRLF line endings or "$Id$"
keywords, is then based on that prefix, and we may convert the line
endings of a file that is binary further down, or not expand keywords
that appear after the first 4GiB.

read_blob_data_from_index(), which feeds has_crlf_in_index(),
get_cached_convert_stats_ascii() and the reading of .gitattributes from
the index, gets the size of the blob as a size_t, but hands it out
through cast_size_t_to_ulong(), so on such platforms "git add" of a file
whose staged version is over 4GiB dies when core.autocrlf is in effect.

Use size_t for all of them.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 attr.c          |  2 +-
 convert.c       | 14 +++++++-------
 read-cache-ll.h |  2 +-
 read-cache.c    |  4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/attr.c b/attr.c
index 0e63f1b6de..1acb4ef430 100644
--- a/attr.c
+++ b/attr.c
@@ -793,7 +793,7 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,
 {
 	struct attr_stack *stack = NULL;
 	char *buf;
-	unsigned long size;
+	size_t size;
 	int sparse_dir_pos = -1;
 
 	if (!istate)
diff --git a/convert.c b/convert.c
index 036506842c..ca5594cab0 100644
--- a/convert.c
+++ b/convert.c
@@ -42,9 +42,9 @@ struct text_stat {
 	unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
+static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
 {
-	unsigned long i;
+	size_t i;
 
 	memset(stats, 0, sizeof(*stats));
 
@@ -102,7 +102,7 @@ static int convert_is_binary(const struct text_stat *stats)
 	return 0;
 }
 
-static unsigned int gather_convert_stats(const char *data, unsigned long size)
+static unsigned int gather_convert_stats(const char *data, size_t size)
 {
 	struct text_stat stats;
 	int ret = 0;
@@ -119,7 +119,7 @@ static unsigned int gather_convert_stats(const char *data, unsigned long size)
 	return ret;
 }
 
-static const char *gather_convert_stats_ascii(const char *data, unsigned long size)
+static const char *gather_convert_stats_ascii(const char *data, size_t size)
 {
 	unsigned int convert_stats = gather_convert_stats(data, size);
 
@@ -141,7 +141,7 @@ const char *get_cached_convert_stats_ascii(struct index_state *istate,
 					   const char *path)
 {
 	const char *ret;
-	unsigned long sz;
+	size_t sz;
 	void *data = read_blob_data_from_index(istate, path, &sz);
 	ret = gather_convert_stats_ascii(data, sz);
 	free(data);
@@ -223,7 +223,7 @@ static void check_global_conv_flags_eol(const char *path,
 
 static int has_crlf_in_index(struct index_state *istate, const char *path)
 {
-	unsigned long sz;
+	size_t sz;
 	void *data;
 	const char *crp;
 	int has_crlf = 0;
@@ -1077,7 +1077,7 @@ static int read_convert_config(const char *var, const char *value,
 	return 0;
 }
 
-static int count_ident(const char *cp, unsigned long size)
+static int count_ident(const char *cp, size_t size)
 {
 	/*
 	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 066dd8bc3b..d279059b5f 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -416,7 +416,7 @@ int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
 void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 int index_name_is_other(struct index_state *, const char *, int);
-void *read_blob_data_from_index(struct index_state *, const char *, unsigned long *);
+void *read_blob_data_from_index(struct index_state *, const char *, size_t *);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index c4cf08a3a3..635dad1ba3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3472,7 +3472,7 @@ int index_name_is_other(struct index_state *istate, const char *name,
 }
 
 void *read_blob_data_from_index(struct index_state *istate,
-				const char *path, unsigned long *size)
+				const char *path, size_t *size)
 {
 	int pos, len;
 	size_t sz;
@@ -3503,7 +3503,7 @@ void *read_blob_data_from_index(struct index_state *istate,
 		return NULL;
 	}
 	if (size)
-		*size = cast_size_t_to_ulong(sz);
+		*size = sz;
 	return data;
 }
 
-- 
2.53.0

Re: [PATCH 3/3] convert: use size_t for buffer sizes

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 12:16:39PM +0200, Elia Pinto wrote:
> diff --git a/convert.c b/convert.c
> index 036506842c..ca5594cab0 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -42,9 +42,9 @@ struct text_stat {
>  	unsigned printable, nonprintable;
>  };
>  
> -static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
> +static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
>  {
> -	unsigned long i;
> +	size_t i;
>  
>  	memset(stats, 0, sizeof(*stats));
>  

One thing to note here is that `struct text_stat` still uses `unsigned`
counters. Is this something we should fix, as well?

> @@ -1077,7 +1077,7 @@ static int read_convert_config(const char *var, const char *value,
>  	return 0;
>  }
>  
> -static int count_ident(const char *cp, unsigned long size)
> +static int count_ident(const char *cp, size_t size)
>  {
>  	/*
>  	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"

This function is counting `$Id$` markers, so it's rather unlikely that
it ever overflows. Theoretically it's possible though.

Patrick