Re: [PATCH/RFC 0/3] pedantic errors in next

["René Scharfe" <l.s.r@web.de>]

Am 10.08.21 um 08:16 schrieb Carlo Marcelo Arenas Belón:
> Thanks,
>
> in the discussion above René[1] proposed a fix for UBsan issues that were
> reported and that it is still missing.
>
> my version of it didn't require the extra 4 bytes or showed issues with
> notes so is probably incomplete and should be replaced from the original
> if possible, but follows below:

With your three patches plus the one below t3301-notes.sh and several more
fail on an Apple M1.  Adding an unused int member to struct leaf_node fixes
that.  I didn't dig deeper into the notes code to understand the actual
issue, though.

>
> Carlo
>
> [1] https://lore.kernel.org/git/bab9f889-ee2e-d3c3-0319-e297b59261a0@web.de/
>
> +CC René for advise
> --- >8 ---
> Date: Sun, 8 Aug 2021 20:45:56 -0700
> Subject: [PATCH] build: fixes for SANITIZE=undefined (WIP)
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> mostly from instructions/code provided by René in :
>
>   https://lore.kernel.org/git/20210807224957.GA5068@dcvr/
>
> tested with Xcode in macOS 11.5.1 (x86_64)
> ---
>  hash.h        | 2 +-
>  object-file.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hash.h b/hash.h
> index 27a180248f..3127ba1ef8 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -115,7 +115,7 @@ static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *s
>
>  struct object_id {
>  	unsigned char hash[GIT_MAX_RAWSZ];
> -	int algo;
> +	uint8_t algo;
>  };
>
>  /* A suitably aligned type for stack allocations of hash contexts. */
> diff --git a/object-file.c b/object-file.c
> index 374f3c26bf..2fa282a9b4 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2406,7 +2406,7 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,
>  	struct strbuf buf = STRBUF_INIT;
>  	size_t word_bits = bitsizeof(odb->loose_objects_subdir_seen[0]);
>  	size_t word_index = subdir_nr / word_bits;
> -	size_t mask = 1 << (subdir_nr % word_bits);
> +	size_t mask = 1U << (subdir_nr % word_bits);
>  	uint32_t *bitmap;
>
>  	if (subdir_nr < 0 ||
>

The first hunk is about alignment (and missing the notes fix, as mentioned).
The second hunk is about shifting a signed 32-bit value 31 places to the
left, which is undefined (because technically there are only 31 value bits).
Those are different issues and they should be addressed by separate patches,
I think.  That's why I submitted a patch for the the second one in
http://public-inbox.org/git/bab9f889-ee2e-d3c3-0319-e297b59261a0@web.de/.

René