git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [BUG] git crashes with a SIGBUS on sparc64 during pull

From
KKoakuma <koachan@protonmail.com>
Date
Jan 18, 2025, 16:50 UTC
Message-ID
<WnIqKOrqOA2N9RtHmebZaMv3F-XW203leJkiT-5GFU61g3xHbkoJgXwkLfr5G-09fjbU-AX6Eji8kBMUDtjngAw0yRxQuy-rna6Eiov_E4I=@protonmail.com>
In-Reply-To
<20250118092048.GA3474411@coredump.intra.peff.net>
Jeff King <peff@peff.net> wrote:
Show 107 quoted lines
> The union thing should be portable, I'd think, but unfortunately has a
> lot of fallout through the code because the name of "buffer" changes (we
> could also declare the storage separately and make "buffer" a pointer to
> it, but we'd have to be careful about calls to sizeof()).
> 
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 2197d6d933..65db435b46 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -23,7 +23,10 @@ static int dry_run, quiet, recover, has_errors, strict;
> static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict]";
> 
> /* We always read in 4kB chunks. */
> -static unsigned char buffer[4096];
> +static union {
> + struct pack_header hdr;
> + unsigned char bytes[4096];
> +} buffer;
> static unsigned int offset, len;
> static off_t consumed_bytes;
> static off_t max_input_size;
> @@ -65,24 +68,24 @@ static void add_object_buffer(struct object *object, char *buffer, unsigned long
> static void *fill(int min)
> {
> if (min <= len)
> - return buffer + offset;
> - if (min > sizeof(buffer))
> 
> + return buffer.bytes + offset;
> + if (min > sizeof(buffer.bytes))
> 
> die("cannot fill %d bytes", min);
> if (offset) {
> - the_hash_algo->update_fn(&ctx, buffer, offset);
> 
> - memmove(buffer, buffer + offset, len);
> + the_hash_algo->update_fn(&ctx, buffer.bytes, offset);
> 
> + memmove(buffer.bytes, buffer.bytes + offset, len);
> offset = 0;
> }
> do {
> - ssize_t ret = xread(0, buffer + len, sizeof(buffer) - len);
> + ssize_t ret = xread(0, buffer.bytes + len, sizeof(buffer.bytes) - len);
> if (ret <= 0) {
> if (!ret)
> die("early EOF");
> die_errno("read error on input");
> }
> len += ret;
> } while (len < min);
> - return buffer;
> + return buffer.bytes;
> }
> 
> static void use(int bytes)
> @@ -645,18 +648,16 @@ int cmd_unpack_objects(int argc,
> continue;
> }
> if (starts_with(arg, "--pack_header=")) {
> - struct pack_header *hdr;
> char *c;
> 
> - hdr = (struct pack_header *)buffer;
> - hdr->hdr_signature = htonl(PACK_SIGNATURE);
> 
> - hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));
> 
> + buffer.hdr.hdr_signature = htonl(PACK_SIGNATURE);
> + buffer.hdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));
> if (*c != ',')
> die("bad %s", arg);
> - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));
> 
> + buffer.hdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));
> if (*c)
> die("bad %s", arg);
> - len = sizeof(*hdr);
> + len = sizeof(buffer.hdr);
> continue;
> }
> if (skip_prefix(arg, "--max-input-size=", &arg)) {
> @@ -671,7 +672,7 @@ int cmd_unpack_objects(int argc,
> }
> the_hash_algo->init_fn(&ctx);
> 
> unpack_all();
> - the_hash_algo->update_fn(&ctx, buffer, offset);
> 
> + the_hash_algo->update_fn(&ctx, buffer.bytes, offset);
> 
> the_hash_algo->init_fn(&tmp_ctx);
> 
> the_hash_algo->clone_fn(&tmp_ctx, &ctx);
> 
> the_hash_algo->final_oid_fn(&oid, &tmp_ctx);
> 
> @@ -686,7 +687,7 @@ int cmd_unpack_objects(int argc,
> use(the_hash_algo->rawsz);
> 
> 
> /* Write the last part of the buffer to stdout /
> - write_in_full(1, buffer + offset, len);
> + write_in_full(1, buffer.bytes + offset, len);
> 
> / All done */
> return has_errors;
This patch works well here, yes.
Show 5 quoted lines
> I guess that's not too surprising. Probably an application of get_be32()
> would solve it. But I do wonder if it would be simpler just to make sure
> the buffer is aligned. You mentioned that you tried that before and it
> worked. How did you do it? With a pragma/attribute, or with a union (as
> below)?

I did it by adding an attribute, though probably a better implementation would query for the alignment of `structack_hdr` instead of hardcoding it.

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 842a90353a..98f270ec0c 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -23,7 +23,7 @@ static int dry_run, quiet, recover, has_errors, strict;
 static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict]";
 
 /* We always read in 4kB chunks. */
-static unsigned char buffer[4096];
+static unsigned char buffer[4096] __attribute__((aligned(16)));
 static unsigned int offset, len;
 static off_t consumed_bytes;
 static off_t max_input_size;
Previous: Jeff KingNext: Jeff King
Message 14 of 22 in “[BUG] git crashes with a SIGBUS on sparc64 during pull”
  1. KoakumaJan 17, 2025
  2. Jeff KingJan 17, 2025
  3. Jeff KingJan 17, 2025
  4. 1/3 packfile: factor out --pack_header argument parsingJeff King, Jan 17, 2025
  5. Junio C HamanoJan 17, 2025
  6. Jeff KingJan 18, 2025
  7. KoakumaJan 18, 2025
  8. 2/3 parse_pack_header_option(): avoid unaligned memory writesJeff King, Jan 17, 2025
  9. Junio C HamanoJan 18, 2025
  10. Jeff KingJan 18, 2025
  11. 3/3 index-pack, unpack-objects: use skip_prefix to avoid magic numberJeff King, Jan 17, 2025
  12. KoakumaJan 17, 2025
  13. Jeff KingJan 18, 2025
  14. KoakumaJan 18, 2025
  15. 0/5 git crashes with a SIGBUS on sparc64 during pullJeff King, Jan 19, 2025
  16. 1/5 bswap.h: squelch potential sparse -Wcast-truncate warningsJeff King, Jan 19, 2025
  17. 2/5 packfile: factor out --pack_header argument parsingJeff King, Jan 19, 2025
  18. 3/5 parse_pack_header_option(): avoid unaligned memory writesJeff King, Jan 19, 2025
  19. 4/5 index-pack, unpack-objects: use get_be32() for reading pack headerJeff King, Jan 19, 2025
  20. 5/5 index-pack, unpack-objects: use skip_prefix to avoid magic numberJeff King, Jan 19, 2025
  21. KoakumaJan 20, 2025
  22. Junio C HamanoJan 21, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.