From: "Eric W. Biederman" <ebiederm@gmail•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org,
"brian m. carlson" <sandals@crustytoothpaste•net>,
"Eric W. Biederman" <ebiederm@xmission•com>
Subject: Re: [PATCH 21/30] repository: Implement extensions.compatObjectFormat
Date: Sun, 01 Oct 2023 19:48:56 -0500 [thread overview]
Message-ID: <87r0mdetmv.fsf@gmail.froward.int.ebiederm.org> (raw)
In-Reply-To: <xmqqedigizms.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 29 Sep 2023 11:48:59 -0700")
Junio C Hamano <gitster@pobox•com> writes:
> "Eric W. Biederman" <ebiederm@gmail•com> writes:
>
>> Did you have any manual merge conflicts you had to resolve?
>> If so it is possible to see the merge result you had?
>
> The only merge-fix I had to apply to make everything compile was
> this:
>
> diff --git a/bloom.c b/bloom.c
> index ff131893cd..59eb0a0481 100644
> --- a/bloom.c
> +++ b/bloom.c
> @@ -278,7 +278,7 @@ static int has_entries_with_high_bit(struct repository *r, struct tree *t)
> struct tree_desc desc;
> struct name_entry entry;
>
> - init_tree_desc(&desc, t->buffer, t->size);
> + init_tree_desc(&desc, &t->object.oid, t->buffer, t->size);
> while (tree_entry(&desc, &entry)) {
> size_t i;
> for (i = 0; i < entry.pathlen; i++) {
>
> as one topic changed the function signature while the other topic
> added a new callsite.
>
> Everything else was pretty-much auto resolved, I think.
>
> Output from "git show --cc seen" matches my recollection. The above
> does appear as an evil merge.
Thanks, and I found all of this on your seen branch.
After tracking all of these down it appears all of the errors
came from my branch, I will be resending the patches as soon
as I finish going through the review comments.
Looking at the build errors pretty much all of the all of the
automatic test failures came from commit_tree_extended.
There was a strbuf that did
strbuf_init(&buf, 8192);
strbuf_init(&buf, 8192);
twice.
Plus there was another buffer that was allocated and not freed,
in commit_tree_extended.
The leaks were a bit tricky to track down as building with SANITIZE=leak
causes tests to fail somewhat randomly for me with "gcc (Debian
12.2.0-14) 12.2.0".
There was one smatch static-analysis error that suggested using
CALLOC_ARRAY instead of xcalloc.
The "win" build and "linux-gcc-default (ubuntu-lastest)" build failed
because of an over eager gcc warning -Werror=array-bounds.
Claiming:
In file included from /usr/include/string.h:535,
from git-compat-util.h:228,
from commit.c:1:
In function ‘memcpy’,
inlined from ‘oidcpy’ at hash-ll.h:272:2,
inlined from ‘commit_tree_extended’ at commit.c:1705:3:
##[error]/usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: ‘__builtin_memcpy’ offset [0, 31] is out of the bounds [0, 0] [-Werror=array-bounds]
29 | return __builtin___memcpy_chk (__dest, __src, __len,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
30 | __glibc_objsize0 (__dest));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
Previously in commit_tree_extended the structure was:
while (parents) {
struct commit *parent = pop_commit(&parents);
strbuf_addf(&buffer, "parent %s\n",
oid_to_hex(&parent->object.oid));
}
brian had changed it to:
nparents = commit_list_count(parents);
parent_buf = xcalloc(nparents, sizeof(*parent_buf));
for (i = 0; i < nparents; i++) {
struct commit *parent = pop_commit(&parents);
oidcpy(&parent_buf[i], &parent->object.oid);
}
Which is perfectly sound code.
I changed the structure of the loop to:
nparents = commit_list_count(parents);
parent_buf = xcalloc(nparents, sizeof(*parent_buf));
i = 0;
while (parents) {
struct commit *parent = pop_commit(&parents);
oidcpy(&parent_buf[i++], &parent->object.oid);
}
And the "array-bounds" warning had no problems with the code.
So it looks like the error was actually that array-bounds thought
there was a potential NULL pointer dereference at which point
it would not have array bounds, and then it complained about
the array bounds, instead of the NULL pointer dereference.
I am going to fix the patch. If array-bounds causes further
problems you may want to think about disabling it.
Eric
next prev parent reply other threads:[~2023-10-02 0:49 UTC|newest]
Thread overview: 104+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-27 19:49 [PATCH 00/30] Initial support for multiple hash functions Eric W. Biederman
2023-09-27 19:55 ` [PATCH 01/30] object-file-convert: Stubs for converting from one object format to another Eric W. Biederman
2023-09-27 20:42 ` Eric Sunshine
2023-10-02 1:22 ` Eric W. Biederman
2023-10-02 2:27 ` Eric Sunshine
2023-09-27 19:55 ` [PATCH 02/30] oid-array: Teach oid-array to handle multiple kinds of oids Eric W. Biederman
2023-09-27 23:20 ` Eric Sunshine
2023-09-27 19:55 ` [PATCH 03/30] object-names: Support input of oids in any supported hash Eric W. Biederman
2023-09-27 23:29 ` Eric Sunshine
2023-10-02 1:54 ` Eric W. Biederman
2023-09-27 19:55 ` [PATCH 04/30] repository: add a compatibility hash algorithm Eric W. Biederman
2023-09-27 19:55 ` [PATCH 05/30] loose: add a mapping between SHA-1 and SHA-256 for loose objects Eric W. Biederman
2023-09-28 7:14 ` Eric Sunshine
2023-10-02 2:11 ` Eric W. Biederman
2023-10-02 2:36 ` Eric Sunshine
2023-09-27 19:55 ` [PATCH 06/30] loose: Compatibilty short name support Eric W. Biederman
2023-09-27 19:55 ` [PATCH 07/30] object-file: Update the loose object map when writing loose objects Eric W. Biederman
2023-09-27 19:55 ` [PATCH 08/30] object-file: Add a compat_oid_in parameter to write_object_file_flags Eric W. Biederman
2023-09-27 19:55 ` [PATCH 09/30] commit: write commits for both hashes Eric W. Biederman
2023-09-27 19:55 ` [PATCH 10/30] commit: Convert mergetag before computing the signature of a commit Eric W. Biederman
2023-09-27 19:55 ` [PATCH 11/30] commit: Export add_header_signature to support handling signatures on tags Eric W. Biederman
2023-09-27 19:55 ` [PATCH 12/30] tag: sign both hashes Eric W. Biederman
2023-09-27 19:55 ` [PATCH 13/30] cache: add a function to read an OID of a specific algorithm Eric W. Biederman
2023-09-27 19:55 ` [PATCH 14/30] object: Factor out parse_mode out of fast-import and tree-walk into in object.h Eric W. Biederman
2023-09-27 19:55 ` [PATCH 15/30] object-file-convert: add a function to convert trees between algorithms Eric W. Biederman
2023-09-27 19:55 ` [PATCH 16/30] object-file-convert: convert tag objects when writing Eric W. Biederman
2023-09-27 19:55 ` [PATCH 17/30] object-file-convert: Don't leak when converting tag objects Eric W. Biederman
2023-09-27 19:55 ` [PATCH 18/30] object-file-convert: convert commit objects when writing Eric W. Biederman
2023-09-27 19:55 ` [PATCH 19/30] object-file-convert: Convert commits that embed signed tags Eric W. Biederman
2023-09-27 19:55 ` [PATCH 20/30] object-file: Update object_info_extended to reencode objects Eric W. Biederman
2023-09-27 19:55 ` [PATCH 21/30] repository: Implement extensions.compatObjectFormat Eric W. Biederman
2023-09-27 21:39 ` Junio C Hamano
2023-09-28 20:18 ` Junio C Hamano
2023-09-29 0:50 ` Eric Biederman
2023-09-29 16:59 ` Eric W. Biederman
2023-09-29 18:48 ` Junio C Hamano
2023-10-02 0:48 ` Eric W. Biederman [this message]
2023-10-02 1:31 ` Eric W. Biederman
2023-09-27 19:55 ` [PATCH 22/30] rev-parse: Add an --output-object-format parameter Eric W. Biederman
2023-09-27 19:55 ` [PATCH 23/30] builtin/cat-file: Let the oid determine the output algorithm Eric W. Biederman
2023-09-27 19:55 ` [PATCH 24/30] tree-walk: init_tree_desc take an oid to get the hash algorithm Eric W. Biederman
2023-09-27 19:55 ` [PATCH 25/30] object-file: Handle compat objects in check_object_signature Eric W. Biederman
2023-09-27 19:55 ` [PATCH 26/30] builtin/ls-tree: Let the oid determine the output algorithm Eric W. Biederman
2023-09-27 19:55 ` [PATCH 27/30] test-lib: Compute the compatibility hash so tests may use it Eric W. Biederman
2023-09-27 19:55 ` [PATCH 28/30] t1006: Rename sha1 to oid Eric W. Biederman
2023-09-27 19:55 ` [PATCH 29/30] t1006: Test oid compatibility with cat-file Eric W. Biederman
2023-09-27 19:55 ` [PATCH 30/30] t1016-compatObjectFormat: Add tests to verify the conversion between objects Eric W. Biederman
2023-09-27 21:31 ` [PATCH 00/30] Initial support for multiple hash functions Junio C Hamano
2023-10-02 2:39 ` [PATCH v2 00/30] initial " Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 01/30] object-file-convert: stubs for converting from one object format to another Eric W. Biederman
2024-02-08 8:23 ` Linus Arver
2024-02-15 11:21 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 02/30] oid-array: teach oid-array to handle multiple kinds of oids Eric W. Biederman
2024-02-13 8:16 ` Linus Arver
2024-02-15 6:22 ` Eric W. Biederman
2024-02-16 0:16 ` Linus Arver
2024-02-16 4:48 ` Eric W. Biederman
2024-02-17 1:59 ` Linus Arver
2024-02-13 8:31 ` Kristoffer Haugsbakk
2024-02-15 6:24 ` Eric W. Biederman
2024-02-15 11:21 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 03/30] object-names: support input of oids in any supported hash Eric W. Biederman
2024-02-13 9:33 ` Linus Arver
2024-02-15 11:21 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 04/30] repository: add a compatibility hash algorithm Eric W. Biederman
2024-02-13 10:02 ` Linus Arver
2024-02-15 11:22 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 05/30] loose: add a mapping between SHA-1 and SHA-256 for loose objects Eric W. Biederman
2024-02-14 7:20 ` Linus Arver
2024-02-15 5:33 ` Eric W. Biederman
2024-02-15 11:22 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 06/30] loose: compatibilty short name support Eric W. Biederman
2024-02-15 11:22 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 07/30] object-file: update the loose object map when writing loose objects Eric W. Biederman
2024-02-15 11:22 ` Patrick Steinhardt
2023-10-02 2:40 ` [PATCH v2 08/30] object-file: add a compat_oid_in parameter to write_object_file_flags Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 09/30] commit: write commits for both hashes Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 10/30] commit: convert mergetag before computing the signature of a commit Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 11/30] commit: export add_header_signature to support handling signatures on tags Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 12/30] tag: sign both hashes Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 13/30] cache: add a function to read an OID of a specific algorithm Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 14/30] object: factor out parse_mode out of fast-import and tree-walk into in object.h Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 15/30] object-file-convert: add a function to convert trees between algorithms Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 16/30] object-file-convert: convert tag objects when writing Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 17/30] object-file-convert: don't leak when converting tag objects Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 18/30] object-file-convert: convert commit objects when writing Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 19/30] object-file-convert: convert commits that embed signed tags Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 20/30] object-file: update object_info_extended to reencode objects Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 21/30] repository: implement extensions.compatObjectFormat Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 22/30] rev-parse: add an --output-object-format parameter Eric W. Biederman
2024-02-08 16:25 ` Jean-Noël Avila
2023-10-02 2:40 ` [PATCH v2 23/30] builtin/cat-file: let the oid determine the output algorithm Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 24/30] tree-walk: init_tree_desc take an oid to get the hash algorithm Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 25/30] object-file: handle compat objects in check_object_signature Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 26/30] builtin/ls-tree: let the oid determine the output algorithm Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 27/30] test-lib: compute the compatibility hash so tests may use it Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 28/30] t1006: rename sha1 to oid Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 29/30] t1006: test oid compatibility with cat-file Eric W. Biederman
2023-10-02 2:40 ` [PATCH v2 30/30] t1016-compatObjectFormat: add tests to verify the conversion between objects Eric W. Biederman
2024-02-07 22:18 ` [PATCH v2 00/30] initial support for multiple hash functions Junio C Hamano
2024-02-08 0:24 ` Linus Arver
2024-02-08 6:11 ` Patrick Steinhardt
2024-02-14 7:36 ` Linus Arver
2024-02-15 11:27 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r0mdetmv.fsf@gmail.froward.int.ebiederm.org \
--to=ebiederm@gmail$(echo .)com \
--cc=ebiederm@xmission$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=sandals@crustytoothpaste$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox