Re: [PATCH 4/4] dir.c: avoid "exceeds maximum object size" error with GCC v12.x

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, May 24 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Technically, the pointer difference `end - start` _could_ be negative,
> and when cast to an (unsigned) `size_t` that would cause problems. In
> this instance, the symptom is:
>
> dir.c: In function 'git_url_basename':
> dir.c:3087:13: error: 'memchr' specified bound [9223372036854775808, 0]
>        exceeds maximum object size 9223372036854775807
>        [-Werror=stringop-overread]
>     CC ewah/bitmap.o
>  3087 |         if (memchr(start, '/', end - start) == NULL
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> While it is a bit far-fetched to think that `end` (which is defined as
> `repo + strlen(repo)`) and `start` (which starts at `repo` and never
> steps beyond the NUL terminator) could result in such a negative
> difference, GCC has no way of knowing that.
>
> See also https://gcc.gnu.org/bugzilla//show_bug.cgi?id=85783.
>
> Let's just add a safety check, primarily for GCC's benefit.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  dir.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/dir.c b/dir.c
> index 5aa6fbad0b7..ea78f606230 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -3076,6 +3076,15 @@ char *git_url_basename(const char *repo, int is_bundle, int is_bare)
>  			end--;
>  	}
>  
> +	/*
> +	 * It should not be possible to overflow `ptrdiff_t` by passing in an
> +	 * insanely long URL, but GCC does not know that and will complain
> +	 * without this check.
> +	 */
> +	if (end - start < 0)
> +		die(_("No directory name could be guessed.\n"

This should start with a lower-case letter, see CodingGuidelines.

> +		      "Please specify a directory on the command line"));
> +
>  	/*
>  	 * Strip trailing port number if we've got only a
>  	 * hostname (that is, there is no dir separator but a