{"thread":{"id":"62819","subject":"[BUG] git crashes with a SIGBUS on sparc64 during pull","startedAt":"2025-01-17T03:30:21Z","lastAt":"2025-01-21T16:37:27Z","messageCount":22,"participants":["Koakuma","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"510753","messageId":"xd6Mn7uXpDxaT8zF28BwOIdV98lnvm3S0rBgxmm97nuRW3z0GQSUi_KeYe0fUuRfz8rCgo6Uialw5rnraqp5tjzMFQkFH0MZV_n4fEQg8go=@protonmail.com","threadId":"62819","inReplyTo":null,"subject":"[BUG] git crashes with a SIGBUS on sparc64 during pull","fromName":"Koakuma","fromEmail":"koachan@protonmail.com","sentAt":"2025-01-17T03:30:09Z","receivedAt":"2025-01-17T03:30:21Z","isPatch":false,"sender":{"key":"koachan@protonmail.com","avatar":null},"body":"Thank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\n1. Compile git with clang (this doesn't seem to happen with GCC, oddly)\n2. Do a `git pull` on a repository that will trigger the `unpack-objects` code\n\nWhat did you expect to happen? (Expected behavior)\n\nPull succeeds\n\nWhat happened instead? (Actual behavior)\n\nPull fails with this error:\nremote: Enumerating objects: 7, done.\nremote: Counting objects: 100% (5/5), done.\nerror: unpack-objects died of signal 10\nfatal: unpack-objects failed\n\nWhat's different between what you expected and what actually happened?\n\nThe pull process crashes with a SIGBUS.\n\nAnything else you want to add:\n\nre: unpack-objects, I don't know how to specifically trigger that code,\nbut I know that once `unpack-objects` is triggered it will reliably crash\nwith the error above.\n\nAccording to gdb, the crash happens on this line:\n#0  0x000001000019ca18 in cmd_unpack_objects (argc=<optimized out>, argv=0x1000063a4e0, prefix=<optimized out>, repo=<optimized out>) at builtin/unpack-objects.c:653\n653\t\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n\nOveraligning the `buffer` declaration in the same file to try to get around\npossible alignment issues seems to be able to prevent the crash,\nbut I don't know if it would be a proper fix for it.\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.48.1.262.g85cc9f2d1e\ncpu: sparc64\nbuilt from commit: 85cc9f2d1ee4d65cb1edb00d4f56863185a53e0f\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nlibcurl: 8.10.1\nOpenSSL: OpenSSL 3.3.2 3 Sep 2024\nzlib: 1.3.1\nuname: Linux 6.6.30-sparc64-clang+ #1 SMP Sat Oct 26 21:22:10 WIB 2024 sparc64\ncompiler info: clang: 19.1.4\nlibc info: glibc: 2.40\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\n\n"},{"id":"510796","messageId":"20250117121121.GA2356599@coredump.intra.peff.net","threadId":"62819","inReplyTo":"xd6Mn7uXpDxaT8zF28BwOIdV98lnvm3S0rBgxmm97nuRW3z0GQSUi_KeYe0fUuRfz8rCgo6Uialw5rnraqp5tjzMFQkFH0MZV_n4fEQg8go=@protonmail.com","subject":"Re: [BUG] git crashes with a SIGBUS on sparc64 during pull","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-17T12:11:21Z","receivedAt":"2025-01-17T12:11:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 17, 2025 at 03:30:09AM +0000, Koakuma wrote:\n\n> re: unpack-objects, I don't know how to specifically trigger that code,\n> but I know that once `unpack-objects` is triggered it will reliably crash\n> with the error above.\n\nPerhaps:\n\n  git init repo\n  cd repo\n  git commit --allow-empty -m foo\n  git repack -ad\n  pack=$(ls .git/objects/pack/*.pack)\n  dd if=$pack of=no-header.pack bs=1 skip=12\n  # don't bother parsing the first 12 bytes; we know it is\n  # a version 2 pack with 2 objects\n  git unpack-objects --pack_header=2,2 <no-header.pack\n\nwould be a minimal reproduction?\n\n> According to gdb, the crash happens on this line:\n> #0  0x000001000019ca18 in cmd_unpack_objects (argc=<optimized out>, argv=0x1000063a4e0, prefix=<optimized out>, repo=<optimized out>) at builtin/unpack-objects.c:653\n> 653\t\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n> \n> Overaligning the `buffer` declaration in the same file to try to get around\n> possible alignment issues seems to be able to prevent the crash,\n> but I don't know if it would be a proper fix for it.\n\nInteresting. We are pretty cavalier about casting mmap'd buffers to\nstructs in order to read pack headers (since the format is designed to\nbe 4-byte aligned).\n\nBut in this case we are _writing_ into a buffer through the cast\nstructure (our caller has already parsed the header, so we are\nreconstructing it). So we are relying on the alignment of a static\n\"unsigned char\" in the data section.\n\nWhich certainly seems sketchy, though it is kind of interesting that it\nhas never been a problem before (and the code has been this way for\ndecades). At any rate, the fix is probably this (and we'd want the same\nin index-pack, too, I'd think):\n\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 2197d6d933..288cecf98f 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -645,18 +645,20 @@ int cmd_unpack_objects(int argc,\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tstruct pack_header *hdr;\n+\t\t\t\tunsigned char *hdr = buffer;\n \t\t\t\tchar *c;\n \n-\t\t\t\thdr = (struct pack_header *)buffer;\n-\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\tput_be32(hdr, PACK_SIGNATURE);\n+\t\t\t\thdr += 4;\n+\t\t\t\tput_be32(hdr, strtoul(arg + 14, &c, 10));\n+\t\t\t\thdr += 4;\n \t\t\t\tif (*c != ',')\n \t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\t\t\t\tput_be32(hdr, strtoul(c + 1, &c, 10));\n+\t\t\t\thdr += 4;\n \t\t\t\tif (*c)\n \t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\tlen = sizeof(*hdr);\n+\t\t\t\tlen = hdr - buffer;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (skip_prefix(arg, \"--max-input-size=\", &arg)) {\n"},{"id":"510802","messageId":"20250117125207.GB2356599@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250117121121.GA2356599@coredump.intra.peff.net","subject":"Re: [BUG] git crashes with a SIGBUS on sparc64 during pull","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-17T12:52:07Z","receivedAt":"2025-01-17T12:52:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 17, 2025 at 07:11:21AM -0500, Jeff King wrote:\n\n> Which certainly seems sketchy, though it is kind of interesting that it\n> has never been a problem before (and the code has been this way for\n> decades). At any rate, the fix is probably this (and we'd want the same\n> in index-pack, too, I'd think):\n\nAssuming that helps, here's a series which does that fix plus some\nassociated cleanups.\n\nI'm curious if it's enough. After we write to this unaligned buffer,\nnaturally the next thing we'll do is read from it, and the reading\nroutines will do the same cast (see unpack_all() in unpack-objects).\nBut maybe your platform allows unaligned reads but not writes? Probably\nI am being too optimistic. :)\n\n  [1/3]: packfile: factor out --pack_header argument parsing\n  [2/3]: parse_pack_header_option(): avoid unaligned memory writes\n  [3/3]: index-pack, unpack-objects: use skip_prefix to avoid magic number\n\n builtin/index-pack.c     | 18 +++++-------------\n builtin/unpack-objects.c | 18 +++++-------------\n packfile.c               | 22 ++++++++++++++++++++++\n packfile.h               |  6 ++++++\n 4 files changed, 38 insertions(+), 26 deletions(-)\n\n-Peff\n"},{"id":"510803","messageId":"20250117125459.GA2893666@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250117125207.GB2356599@coredump.intra.peff.net","subject":"[PATCH 1/3] packfile: factor out --pack_header argument parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-17T12:54:59Z","receivedAt":"2025-01-17T12:55:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Both index-pack and unpack-objects accept a --pack_header argument. This\nis an undocumented internal argument used by receive-pack and fetch to\npass along information about the header of the pack, which they've\nalready read from the incoming stream.\n\nIn preparation for a bugfix, let's factor the duplicated code into a\ncommon helper.\n\nThe callers are still responsible for identifying the option. While this\ncould likewise be factored out, it is more flexible this way (e.g., if\nthey ever started using parse-options and wanted to handle both the\nstuck and unstuck forms).\n\nLikewise, the callers are responsible for reporting errors, though they\nboth just call die(). I've tweaked unpack-objects to match index-pack in\nmarking the error for translation.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe generating side of this option is duplicate, too, between\nreceive-pack and fetch-pack. But it's so much simpler that I didn't\nthink it was worth factoring out. We could always do it later.\n\n builtin/index-pack.c     | 14 +++-----------\n builtin/unpack-objects.c | 16 ++++------------\n packfile.c               | 17 +++++++++++++++++\n packfile.h               |  6 ++++++\n 4 files changed, 30 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 0b62b2589f..75b84f78f4 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1955,18 +1955,10 @@ int cmd_index_pack(int argc,\n \t\t\t\t\tnr_threads = 1;\n \t\t\t\t}\n \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tstruct pack_header *hdr;\n-\t\t\t\tchar *c;\n-\n-\t\t\t\thdr = (struct pack_header *)input_buffer;\n-\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n-\t\t\t\tif (*c != ',')\n-\t\t\t\t\tdie(_(\"bad %s\"), arg);\n-\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n-\t\t\t\tif (*c)\n+\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\t\t\t\t\t     input_buffer,\n+\t\t\t\t\t\t\t     &input_len) < 0)\n \t\t\t\t\tdie(_(\"bad %s\"), arg);\n-\t\t\t\tinput_len = sizeof(*hdr);\n \t\t\t} else if (!strcmp(arg, \"-v\")) {\n \t\t\t\tverbose = 1;\n \t\t\t} else if (!strcmp(arg, \"--progress-title\")) {\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 2197d6d933..cf2bc5c531 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -18,6 +18,7 @@\n #include \"progress.h\"\n #include \"decorate.h\"\n #include \"fsck.h\"\n+#include \"packfile.h\"\n \n static int dry_run, quiet, recover, has_errors, strict;\n static const char unpack_usage[] = \"git unpack-objects [-n] [-q] [-r] [--strict]\";\n@@ -645,18 +646,9 @@ int cmd_unpack_objects(int argc,\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tstruct pack_header *hdr;\n-\t\t\t\tchar *c;\n-\n-\t\t\t\thdr = (struct pack_header *)buffer;\n-\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n-\t\t\t\tif (*c != ',')\n-\t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n-\t\t\t\tif (*c)\n-\t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\tlen = sizeof(*hdr);\n+\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\t\t\t\t\t     buffer, &len) < 0)\n+\t\t\t\t\tdie(_(\"bad %s\"), arg);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (skip_prefix(arg, \"--max-input-size=\", &arg)) {\ndiff --git a/packfile.c b/packfile.c\nindex cc7ab6403a..2bf9e57330 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2315,3 +2315,20 @@ int is_promisor_object(struct repository *r, const struct object_id *oid)\n \t}\n \treturn oidset_contains(&promisor_objects, oid);\n }\n+\n+int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len)\n+{\n+\tstruct pack_header *hdr;\n+\tchar *c;\n+\n+\thdr = (struct pack_header *)out;\n+\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n+\thdr->hdr_version = htonl(strtoul(in, &c, 10));\n+\tif (*c != ',')\n+\t\treturn -1;\n+\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\tif (*c)\n+\t\treturn -1;\n+\t*len = sizeof(*hdr);\n+\treturn 0;\n+}\ndiff --git a/packfile.h b/packfile.h\nindex 58104fa009..00ada7a938 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -216,4 +216,10 @@ int is_promisor_object(struct repository *r, const struct object_id *oid);\n int load_idx(const char *path, const unsigned int hashsz, void *idx_map,\n \t     size_t idx_size, struct packed_git *p);\n \n+/*\n+ * Parse a --pack_header option as accepted by index-pack and unpack-objects,\n+ * turning it into the matching bytes we'd find in a pack.\n+ */\n+int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len);\n+\n #endif\n-- \n2.48.1.438.g7fbf7b046e\n\n"},{"id":"510804","messageId":"20250117125530.GB2893666@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250117125207.GB2356599@coredump.intra.peff.net","subject":"[PATCH 2/3] parse_pack_header_option(): avoid unaligned memory writes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-17T12:55:30Z","receivedAt":"2025-01-17T12:55:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In order to recreate a pack header in our in-memory buffer, we cast the\nbuffer to a \"struct pack_header\" and assign the individual fields. This\nis reported to cause SIGBUS on sparc64 due to alignment issues.\n\nWe can work around this by using put_be32() which will write individual\nbytes into the buffer.\n\nReported-by: Koakuma <koachan@protonmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nFingers crossed that this is sufficient, and we don't have more\nalignment problems.\n\n packfile.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 2bf9e57330..2d80d80cb3 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2318,17 +2318,20 @@ int is_promisor_object(struct repository *r, const struct object_id *oid)\n \n int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len)\n {\n-\tstruct pack_header *hdr;\n+\tunsigned char *hdr;\n \tchar *c;\n \n-\thdr = (struct pack_header *)out;\n-\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\thdr->hdr_version = htonl(strtoul(in, &c, 10));\n+\thdr = out;\n+\tput_be32(hdr, PACK_SIGNATURE);\n+\thdr += 4;\n+\tput_be32(hdr, strtoul(in, &c, 10));\n+\thdr += 4;\n \tif (*c != ',')\n \t\treturn -1;\n-\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\tput_be32(hdr, strtoul(c + 1, &c, 10));\n+\thdr += 4;\n \tif (*c)\n \t\treturn -1;\n-\t*len = sizeof(*hdr);\n+\t*len = hdr - out;\n \treturn 0;\n }\n-- \n2.48.1.438.g7fbf7b046e\n\n"},{"id":"510805","messageId":"20250117125647.GC2893666@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250117125207.GB2356599@coredump.intra.peff.net","subject":"[PATCH 3/3] index-pack, unpack-objects: use skip_prefix to avoid magic number","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-17T12:56:47Z","receivedAt":"2025-01-17T12:56:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When parsing --pack_header=, we manually skip 14 bytes to the data.\nLet's use skip_prefix() to do this automatically.\n\nNote that we overwrite our pointer to the front of the string, so we\nhave to add more context to the error message. We could avoid this by\ndeclaring an extra pointer to hold the value, but I think the modified\nmessage is actually preferable. It should give translators a bit more\ncontext.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nJust a cleanup I noticed in the area. It's possible that I'm overly\nannoyed by manual string counting and this is just churn, though. ;)\n\n builtin/index-pack.c     | 6 +++---\n builtin/unpack-objects.c | 6 +++---\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 75b84f78f4..0561eec00a 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1954,11 +1954,11 @@ int cmd_index_pack(int argc,\n \t\t\t\t\twarning(_(\"no threads support, ignoring %s\"), arg);\n \t\t\t\t\tnr_threads = 1;\n \t\t\t\t}\n-\t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\t} else if (skip_prefix(arg, \"--pack_header=\", &arg)) {\n+\t\t\t\tif (parse_pack_header_option(arg,\n \t\t\t\t\t\t\t     input_buffer,\n \t\t\t\t\t\t\t     &input_len) < 0)\n-\t\t\t\t\tdie(_(\"bad %s\"), arg);\n+\t\t\t\t\tdie(_(\"bad --pack_header: %s\"), arg);\n \t\t\t} else if (!strcmp(arg, \"-v\")) {\n \t\t\t\tverbose = 1;\n \t\t\t} else if (!strcmp(arg, \"--progress-title\")) {\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex cf2bc5c531..06d517dfb6 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -645,10 +645,10 @@ int cmd_unpack_objects(int argc,\n \t\t\t\tfsck_set_msg_types(&fsck_options, arg);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\tif (skip_prefix(arg, \"--pack_header=\", &arg)) {\n+\t\t\t\tif (parse_pack_header_option(arg,\n \t\t\t\t\t\t\t     buffer, &len) < 0)\n-\t\t\t\t\tdie(_(\"bad %s\"), arg);\n+\t\t\t\t\tdie(_(\"bad --pack_header: %s\"), arg);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (skip_prefix(arg, \"--max-input-size=\", &arg)) {\n-- \n2.48.1.438.g7fbf7b046e\n"},{"id":"510813","messageId":"nwg0r52UT4d_kBbQa7V7xCWKxE1xtHiwXKtBvzEwxtJa-3nKpXcWG2DRGUENDSJQ9QOkk4jrEaXYjAbiSAZds2ynH1TZPcX3x6T_t4zm8s4=@protonmail.com","threadId":"62819","inReplyTo":"20250117125207.GB2356599@coredump.intra.peff.net","subject":"Re: [BUG] git crashes with a SIGBUS on sparc64 during pull","fromName":"Koakuma","fromEmail":"koachan@protonmail.com","sentAt":"2025-01-17T15:55:53Z","receivedAt":"2025-01-17T15:56:05Z","isPatch":false,"sender":{"key":"koachan@protonmail.com","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> Perhaps:\n> \n> git init repo\n> cd repo\n> git commit --allow-empty -m foo\n> git repack -ad\n> pack=$(ls .git/objects/pack/*.pack)\n> dd if=$pack of=no-header.pack bs=1 skip=12\n> # don't bother parsing the first 12 bytes; we know it is\n> # a version 2 pack with 2 objects\n> git unpack-objects --pack_header=2,2 <no-header.pack\n> \n> would be a minimal reproduction?\n\nThank you! This reliably triggers it every time.\n\n> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\n> index 2197d6d933..288cecf98f 100644\n> --- a/builtin/unpack-objects.c\n> +++ b/builtin/unpack-objects.c\n> @@ -645,18 +645,20 @@ int cmd_unpack_objects(int argc,\n> continue;\n> }\n> if (starts_with(arg, \"--pack_header=\")) {\n> - struct pack_header *hdr;\n> + unsigned char *hdr = buffer;\n> char *c;\n> \n> - hdr = (struct pack_header *)buffer;\n> - hdr->hdr_signature = htonl(PACK_SIGNATURE);\n> \n> - hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n> \n> + put_be32(hdr, PACK_SIGNATURE);\n> + hdr += 4;\n> + put_be32(hdr, strtoul(arg + 14, &c, 10));\n> + hdr += 4;\n> if (*c != ',')\n> die(\"bad %s\", arg);\n> - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n> \n> + put_be32(hdr, strtoul(c + 1, &c, 10));\n> + hdr += 4;\n> if (*c)\n> die(\"bad %s\", arg);\n> - len = sizeof(*hdr);\n> + len = hdr - buffer;\n> continue;\n> }\n> if (skip_prefix(arg, \"--max-input-size=\", &arg)) {\n\nThis diff does fix the issue in `cmd_unpack_objects`, however...\n\n> I'm curious if it's enough. After we write to this unaligned buffer,\n> naturally the next thing we'll do is read from it, and the reading\n> routines will do the same cast (see unpack_all() in unpack-objects).\n\nIt crashes in `unpack_all`, just as you guessed:\n\n#0  unpack_all () at builtin/unpack-objects.c:583\n583\t\tnr_objects = ntohl(hdr->hdr_entries);\n\nSo I suppose the reading part needs to be adjusted as well?\n\n> But maybe your platform allows unaligned reads but not writes? Probably\n> I am being too optimistic. :)\n\nAs far as I understand, sparc64 traps on any unaligned accesses,\nbe it reads or writes.\n"},{"id":"510852","messageId":"xmqqo705hxfz.fsf@gitster.g","threadId":"62819","inReplyTo":"20250117125459.GA2893666@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] packfile: factor out --pack_header argument parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-17T22:45:04Z","receivedAt":"2025-01-17T22:45:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>  \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n> -\t\t\t\tstruct pack_header *hdr;\n> -\t\t\t\tchar *c;\n> -\n> -\t\t\t\thdr = (struct pack_header *)input_buffer;\n> -\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n> -\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n\nInteresting.  So the file-scope static input_buffer[] sits in the\nBSS and happens to be well aligned not to cause the problem, but ...\n\n> @@ -645,18 +646,9 @@ int cmd_unpack_objects(int argc,\n>  \t\t\t\tcontinue;\n>  \t\t\t}\n>  \t\t\tif (starts_with(arg, \"--pack_header=\")) {\n> -\t\t\t\tstruct pack_header *hdr;\n> -\t\t\t\tchar *c;\n> -\n> -\t\t\t\thdr = (struct pack_header *)buffer;\n> -\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n> -\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n\n... the same file-scope static buffer[] that also sits in the BSS\nwas not well aligned by chance?\n\nOtherwise these should be identical code.  Very interesting.\n\nAnd of course the fix in the [2/3] is absolutely the right thing to\ndo.\n\nThanks.\n\n"},{"id":"510862","messageId":"xmqq7c6s52ti.fsf@gitster.g","threadId":"62819","inReplyTo":"20250117125530.GB2893666@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] parse_pack_header_option(): avoid unaligned memory writes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-18T01:27:21Z","receivedAt":"2025-01-18T01:27:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +\tput_be32(hdr, PACK_SIGNATURE);\n\nTonight's comedy.  PACK_SIGNATURE is defined as 0x5041434b (in pack.h)\nIn <compat/bswap.h> we want to take advantage of the fact that\nassigning any unsigned integer to *(unsigned char *) would assign\nthe integer's least significant 8 bits.\n\nstatic inline void put_be32(void *ptr, uint32_t value)\n{\n\tunsigned char *p = ptr;\n\tp[0] = value >> 24;\n\tp[1] = value >> 16;\n\tp[2] = value >>  8;\n\tp[3] = value >>  0;\n}\n\nBut sparse seems not to like that.\n\ncompat/bswap.h:175:22: error: cast truncates bits from constant value (5041 becomes 41)\ncompat/bswap.h:176:22: error: cast truncates bits from constant value (504143 becomes 43)\ncompat/bswap.h:177:22: error: cast truncates bits from constant value (5041434b becomes 4b)\n\nOf course we could do the mask, but should we have to?\n\nI think the real compiler would be clever ehough to produce the\nidentical binary with the following patch that is only needed to\nsquelch this error, but I feel dirty after writing this.\n\nBy the way, a \"git grep\" finds \n\n\tput_be32(&hdr_version, INDEX_EXTENSION_VERSION2);\n\nin the fsmonitor.c file, which does not get flagged only because the\nCPP macro expands to a small integer (2).  That is doubly insulting.\n\n\n compat/bswap.h | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git c/compat/bswap.h w/compat/bswap.h\nindex 512f6f4b99..b34054f2bd 100644\n--- c/compat/bswap.h\n+++ w/compat/bswap.h\n@@ -171,23 +171,23 @@ static inline uint64_t get_be64(const void *ptr)\n static inline void put_be32(void *ptr, uint32_t value)\n {\n \tunsigned char *p = ptr;\n-\tp[0] = value >> 24;\n-\tp[1] = value >> 16;\n-\tp[2] = value >>  8;\n-\tp[3] = value >>  0;\n+\tp[0] = (value >> 24) & 0xff;\n+\tp[1] = (value >> 16) & 0xff;\n+\tp[2] = (value >>  8) & 0xff;\n+\tp[3] = (value >>  0) & 0xff;\n }\n \n static inline void put_be64(void *ptr, uint64_t value)\n {\n \tunsigned char *p = ptr;\n-\tp[0] = value >> 56;\n-\tp[1] = value >> 48;\n-\tp[2] = value >> 40;\n-\tp[3] = value >> 32;\n-\tp[4] = value >> 24;\n-\tp[5] = value >> 16;\n-\tp[6] = value >>  8;\n-\tp[7] = value >>  0;\n+\tp[0] = (value >> 56) & 0xff;\n+\tp[1] = (value >> 48) & 0xff;\n+\tp[2] = (value >> 40) & 0xff;\n+\tp[3] = (value >> 32) & 0xff;\n+\tp[4] = (value >> 24) & 0xff;\n+\tp[5] = (value >> 16) & 0xff;\n+\tp[6] = (value >>  8) & 0xff;\n+\tp[7] = (value >>  0) & 0xff;\n }\n \n #endif /* COMPAT_BSWAP_H */\n\n"},{"id":"510865","messageId":"20250118092048.GA3474411@coredump.intra.peff.net","threadId":"62819","inReplyTo":"nwg0r52UT4d_kBbQa7V7xCWKxE1xtHiwXKtBvzEwxtJa-3nKpXcWG2DRGUENDSJQ9QOkk4jrEaXYjAbiSAZds2ynH1TZPcX3x6T_t4zm8s4=@protonmail.com","subject":"Re: [BUG] git crashes with a SIGBUS on sparc64 during pull","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-18T09:20:48Z","receivedAt":"2025-01-18T09:20:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 17, 2025 at 03:55:53PM +0000, Koakuma wrote:\n\n> This diff does fix the issue in `cmd_unpack_objects`, however...\n> \n> > I'm curious if it's enough. After we write to this unaligned buffer,\n> > naturally the next thing we'll do is read from it, and the reading\n> > routines will do the same cast (see unpack_all() in unpack-objects).\n> \n> It crashes in `unpack_all`, just as you guessed:\n> \n> #0  unpack_all () at builtin/unpack-objects.c:583\n> 583\t\tnr_objects = ntohl(hdr->hdr_entries);\n> \n> So I suppose the reading part needs to be adjusted as well?\n\nI guess that's not too surprising. Probably an application of get_be32()\nwould solve it. But I do wonder if it would be simpler just to make sure\nthe buffer is aligned. You mentioned that you tried that before and it\nworked. How did you do it? With a pragma/attribute, or with a union (as\nbelow)?\n\nThe union thing should be portable, I'd think, but unfortunately has a\nlot of fallout through the code because the name of \"buffer\" changes (we\ncould also declare the storage separately and make \"buffer\" a pointer to\nit, but we'd have to be careful about calls to sizeof()).\n\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 2197d6d933..65db435b46 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -23,7 +23,10 @@ static int dry_run, quiet, recover, has_errors, strict;\n static const char unpack_usage[] = \"git unpack-objects [-n] [-q] [-r] [--strict]\";\n \n /* We always read in 4kB chunks. */\n-static unsigned char buffer[4096];\n+static union {\n+\tstruct pack_header hdr;\n+\tunsigned char bytes[4096];\n+} buffer;\n static unsigned int offset, len;\n static off_t consumed_bytes;\n static off_t max_input_size;\n@@ -65,24 +68,24 @@ static void add_object_buffer(struct object *object, char *buffer, unsigned long\n static void *fill(int min)\n {\n \tif (min <= len)\n-\t\treturn buffer + offset;\n-\tif (min > sizeof(buffer))\n+\t\treturn buffer.bytes + offset;\n+\tif (min > sizeof(buffer.bytes))\n \t\tdie(\"cannot fill %d bytes\", min);\n \tif (offset) {\n-\t\tthe_hash_algo->update_fn(&ctx, buffer, offset);\n-\t\tmemmove(buffer, buffer + offset, len);\n+\t\tthe_hash_algo->update_fn(&ctx, buffer.bytes, offset);\n+\t\tmemmove(buffer.bytes, buffer.bytes + offset, len);\n \t\toffset = 0;\n \t}\n \tdo {\n-\t\tssize_t ret = xread(0, buffer + len, sizeof(buffer) - len);\n+\t\tssize_t ret = xread(0, buffer.bytes + len, sizeof(buffer.bytes) - len);\n \t\tif (ret <= 0) {\n \t\t\tif (!ret)\n \t\t\t\tdie(\"early EOF\");\n \t\t\tdie_errno(\"read error on input\");\n \t\t}\n \t\tlen += ret;\n \t} while (len < min);\n-\treturn buffer;\n+\treturn buffer.bytes;\n }\n \n static void use(int bytes)\n@@ -645,18 +648,16 @@ int cmd_unpack_objects(int argc,\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n \n-\t\t\t\thdr = (struct pack_header *)buffer;\n-\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\tbuffer.hdr.hdr_signature = htonl(PACK_SIGNATURE);\n+\t\t\t\tbuffer.hdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));\n \t\t\t\tif (*c != ',')\n \t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\t\t\t\tbuffer.hdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));\n \t\t\t\tif (*c)\n \t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\tlen = sizeof(*hdr);\n+\t\t\t\tlen = sizeof(buffer.hdr);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (skip_prefix(arg, \"--max-input-size=\", &arg)) {\n@@ -671,7 +672,7 @@ int cmd_unpack_objects(int argc,\n \t}\n \tthe_hash_algo->init_fn(&ctx);\n \tunpack_all();\n-\tthe_hash_algo->update_fn(&ctx, buffer, offset);\n+\tthe_hash_algo->update_fn(&ctx, buffer.bytes, offset);\n \tthe_hash_algo->init_fn(&tmp_ctx);\n \tthe_hash_algo->clone_fn(&tmp_ctx, &ctx);\n \tthe_hash_algo->final_oid_fn(&oid, &tmp_ctx);\n@@ -686,7 +687,7 @@ int cmd_unpack_objects(int argc,\n \tuse(the_hash_algo->rawsz);\n \n \t/* Write the last part of the buffer to stdout */\n-\twrite_in_full(1, buffer + offset, len);\n+\twrite_in_full(1, buffer.bytes + offset, len);\n \n \t/* All done */\n \treturn has_errors;\n"},{"id":"510866","messageId":"20250118092351.GB3474411@coredump.intra.peff.net","threadId":"62819","inReplyTo":"xmqqo705hxfz.fsf@gitster.g","subject":"Re: [PATCH 1/3] packfile: factor out --pack_header argument parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-18T09:23:51Z","receivedAt":"2025-01-18T09:23:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 17, 2025 at 02:45:04PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >  \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n> > -\t\t\t\tstruct pack_header *hdr;\n> > -\t\t\t\tchar *c;\n> > -\n> > -\t\t\t\thdr = (struct pack_header *)input_buffer;\n> > -\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n> > -\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n> \n> Interesting.  So the file-scope static input_buffer[] sits in the\n> BSS and happens to be well aligned not to cause the problem, but ...\n\nI suspect it _is_ a problem, but either:\n\n  - The OP's test case was small enough to trigger unpack-objects, not\n    index-pack. Possibly:\n\n      git index-pack --stdin --pack_header=2,2 <no-header.pack\n\n    would fail for them.\n\n  - We simply got lucky with alignment based on the other things in BSS,\n    the whim of the compiler, etc. But it is an accident waiting to\n    happen.\n\n-Peff\n"},{"id":"510867","messageId":"20250118093612.GC3474411@coredump.intra.peff.net","threadId":"62819","inReplyTo":"xmqq7c6s52ti.fsf@gitster.g","subject":"Re: [PATCH 2/3] parse_pack_header_option(): avoid unaligned memory writes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-18T09:36:12Z","receivedAt":"2025-01-18T09:36:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 17, 2025 at 05:27:21PM -0800, Junio C Hamano wrote:\n\n> static inline void put_be32(void *ptr, uint32_t value)\n> {\n> \tunsigned char *p = ptr;\n> \tp[0] = value >> 24;\n> \tp[1] = value >> 16;\n> \tp[2] = value >>  8;\n> \tp[3] = value >>  0;\n> }\n> \n> But sparse seems not to like that.\n> \n> compat/bswap.h:175:22: error: cast truncates bits from constant value (5041 becomes 41)\n> compat/bswap.h:176:22: error: cast truncates bits from constant value (504143 becomes 43)\n> compat/bswap.h:177:22: error: cast truncates bits from constant value (5041434b becomes 4b)\n> \n> Of course we could do the mask, but should we have to?\n\nCute. I think the above is well defined in terms of the C standard. But\nI could see how a linter might want to remind you that you're truncating\na constant.\n\nIt is kind of lame that it only flags the call with a constant. If you\nwant to warn people that they are accidentally truncating, surely it's\nobvious in the code above that truncation is _possible_ depending on the\nvalue. It seems like it's either worth flagging as a dangerous\nconstruct, or not; but doing it only for a constant is not super\nhelpful.\n\n> I think the real compiler would be clever ehough to produce the\n> identical binary with the following patch that is only needed to\n> squelch this error, but I feel dirty after writing this.\n\nI checked with \"gcc -s\" and it produces the same asm before and after\nyour patch, with both -O0 and -O2. So I don't think there's a practical\ndownside. As far as feeling dirty, I dunno. It is basically telling any\nlinter \"yes, I know we are truncating here\". Since it is contained\nwithin put_be32() and won't spread across the code base, I'm not too\noffended by it.\n\nI guess the other option is to pass -Wno-cast-truncate to sparse.\n\n> By the way, a \"git grep\" finds \n> \n> \tput_be32(&hdr_version, INDEX_EXTENSION_VERSION2);\n> \n> in the fsmonitor.c file, which does not get flagged only because the\n> CPP macro expands to a small integer (2).  That is doubly insulting.\n\nHeh. Yeah, that goes back to my \"kind of lame\" comment above. ;)\n\n-Peff\n"},{"id":"510873","messageId":"WnIqKOrqOA2N9RtHmebZaMv3F-XW203leJkiT-5GFU61g3xHbkoJgXwkLfr5G-09fjbU-AX6Eji8kBMUDtjngAw0yRxQuy-rna6Eiov_E4I=@protonmail.com","threadId":"62819","inReplyTo":"20250118092048.GA3474411@coredump.intra.peff.net","subject":"Re: [BUG] git crashes with a SIGBUS on sparc64 during pull","fromName":"Koakuma","fromEmail":"koachan@protonmail.com","sentAt":"2025-01-18T16:50:01Z","receivedAt":"2025-01-18T16:50:13Z","isPatch":false,"sender":{"key":"koachan@protonmail.com","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> The union thing should be portable, I'd think, but unfortunately has a\n> lot of fallout through the code because the name of \"buffer\" changes (we\n> could also declare the storage separately and make \"buffer\" a pointer to\n> it, but we'd have to be careful about calls to sizeof()).\n> \n> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\n> index 2197d6d933..65db435b46 100644\n> --- a/builtin/unpack-objects.c\n> +++ b/builtin/unpack-objects.c\n> @@ -23,7 +23,10 @@ static int dry_run, quiet, recover, has_errors, strict;\n> static const char unpack_usage[] = \"git unpack-objects [-n] [-q] [-r] [--strict]\";\n> \n> /* We always read in 4kB chunks. */\n> -static unsigned char buffer[4096];\n> +static union {\n> + struct pack_header hdr;\n> + unsigned char bytes[4096];\n> +} buffer;\n> static unsigned int offset, len;\n> static off_t consumed_bytes;\n> static off_t max_input_size;\n> @@ -65,24 +68,24 @@ static void add_object_buffer(struct object *object, char *buffer, unsigned long\n> static void *fill(int min)\n> {\n> if (min <= len)\n> - return buffer + offset;\n> - if (min > sizeof(buffer))\n> \n> + return buffer.bytes + offset;\n> + if (min > sizeof(buffer.bytes))\n> \n> die(\"cannot fill %d bytes\", min);\n> if (offset) {\n> - the_hash_algo->update_fn(&ctx, buffer, offset);\n> \n> - memmove(buffer, buffer + offset, len);\n> + the_hash_algo->update_fn(&ctx, buffer.bytes, offset);\n> \n> + memmove(buffer.bytes, buffer.bytes + offset, len);\n> offset = 0;\n> }\n> do {\n> - ssize_t ret = xread(0, buffer + len, sizeof(buffer) - len);\n> + ssize_t ret = xread(0, buffer.bytes + len, sizeof(buffer.bytes) - len);\n> if (ret <= 0) {\n> if (!ret)\n> die(\"early EOF\");\n> die_errno(\"read error on input\");\n> }\n> len += ret;\n> } while (len < min);\n> - return buffer;\n> + return buffer.bytes;\n> }\n> \n> static void use(int bytes)\n> @@ -645,18 +648,16 @@ int cmd_unpack_objects(int argc,\n> continue;\n> }\n> if (starts_with(arg, \"--pack_header=\")) {\n> - struct pack_header *hdr;\n> char *c;\n> \n> - hdr = (struct pack_header *)buffer;\n> - hdr->hdr_signature = htonl(PACK_SIGNATURE);\n> \n> - hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n> \n> + buffer.hdr.hdr_signature = htonl(PACK_SIGNATURE);\n> + buffer.hdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));\n> if (*c != ',')\n> die(\"bad %s\", arg);\n> - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n> \n> + buffer.hdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));\n> if (*c)\n> die(\"bad %s\", arg);\n> - len = sizeof(*hdr);\n> + len = sizeof(buffer.hdr);\n> continue;\n> }\n> if (skip_prefix(arg, \"--max-input-size=\", &arg)) {\n> @@ -671,7 +672,7 @@ int cmd_unpack_objects(int argc,\n> }\n> the_hash_algo->init_fn(&ctx);\n> \n> unpack_all();\n> - the_hash_algo->update_fn(&ctx, buffer, offset);\n> \n> + the_hash_algo->update_fn(&ctx, buffer.bytes, offset);\n> \n> the_hash_algo->init_fn(&tmp_ctx);\n> \n> the_hash_algo->clone_fn(&tmp_ctx, &ctx);\n> \n> the_hash_algo->final_oid_fn(&oid, &tmp_ctx);\n> \n> @@ -686,7 +687,7 @@ int cmd_unpack_objects(int argc,\n> use(the_hash_algo->rawsz);\n> \n> \n> /* Write the last part of the buffer to stdout /\n> - write_in_full(1, buffer + offset, len);\n> + write_in_full(1, buffer.bytes + offset, len);\n> \n> / All done */\n> return has_errors;\n\nThis patch works well here, yes.\n\n> I guess that's not too surprising. Probably an application of get_be32()\n> would solve it. But I do wonder if it would be simpler just to make sure\n> the buffer is aligned. You mentioned that you tried that before and it\n> worked. How did you do it? With a pragma/attribute, or with a union (as\n> below)?\n\nI did it by adding an attribute, though probably a better implementation would\nquery for the alignment of `structack_hdr` instead of hardcoding it.\n\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 842a90353a..98f270ec0c 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -23,7 +23,7 @@ static int dry_run, quiet, recover, has_errors, strict;\n static const char unpack_usage[] = \"git unpack-objects [-n] [-q] [-r] [--strict]\";\n \n /* We always read in 4kB chunks. */\n-static unsigned char buffer[4096];\n+static unsigned char buffer[4096] __attribute__((aligned(16)));\n static unsigned int offset, len;\n static off_t consumed_bytes;\n static off_t max_input_size;\n\n"},{"id":"510874","messageId":"oyahQoMDgyHL5zm-JuAtwlkUM2gHqO67zs83WP5Cm2h07S0x_xt8m4CNiShDFe2epznxlJF3kC-X_g414dRhDdrko44xo_yfsDXq1esVYVo=@protonmail.com","threadId":"62819","inReplyTo":"20250118092351.GB3474411@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] packfile: factor out --pack_header argument parsing","fromName":"Koakuma","fromEmail":"koachan@protonmail.com","sentAt":"2025-01-18T16:57:02Z","receivedAt":"2025-01-18T16:57:14Z","isPatch":true,"sender":{"key":"koachan@protonmail.com","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> Junio C Hamano wrote:\n> > Jeff King peff@peff.net writes:\n> > Interesting. So the file-scope static input_buffer[] sits in the\n> > BSS and happens to be well aligned not to cause the problem, but ...\n> \n> I suspect it is a problem, but either:\n> \n> - The OP's test case was small enough to trigger unpack-objects, not\n> index-pack. Possibly:\n> \n> git index-pack --stdin --pack_header=2,2 <no-header.pack\n> \n> would fail for them.\n> \n> - We simply got lucky with alignment based on the other things in BSS,\n> the whim of the compiler, etc. But it is an accident waiting to\n> happen.\n> \n> -Peff\n\nThat particular command doesn't seem to crash for me right now, but, as you\nsaid, that is probably just a lucky coincidence that the compiler happens to\nplace the buffer on aligned memory.\n"},{"id":"510904","messageId":"20250119131224.GA1541095@coredump.intra.peff.net","threadId":"62819","inReplyTo":"nwg0r52UT4d_kBbQa7V7xCWKxE1xtHiwXKtBvzEwxtJa-3nKpXcWG2DRGUENDSJQ9QOkk4jrEaXYjAbiSAZds2ynH1TZPcX3x6T_t4zm8s4=@protonmail.com","subject":"[PATCH v2 0/5] git crashes with a SIGBUS on sparc64 during pull","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-19T13:12:24Z","receivedAt":"2025-01-19T13:12:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's a v2 which fixes the reading side, as well. I think this should\nlet you get through a full run of unpack-objects, but please confirm. :)\n\nThis also includes Junio's put_be32() tweak to silence sparse.\n\n  [1/5]: bswap.h: squelch potential sparse -Wcast-truncate warnings\n  [2/5]: packfile: factor out --pack_header argument parsing\n  [3/5]: parse_pack_header_option(): avoid unaligned memory writes\n  [4/5]: index-pack, unpack-objects: use get_be32() for reading pack header\n  [5/5]: index-pack, unpack-objects: use skip_prefix to avoid magic number\n\n builtin/index-pack.c     | 30 ++++++++++++------------------\n builtin/unpack-objects.c | 31 ++++++++++++-------------------\n compat/bswap.h           | 24 ++++++++++++------------\n pack.h                   |  3 ++-\n packfile.c               | 20 ++++++++++++++++++++\n packfile.h               |  6 ++++++\n 6 files changed, 64 insertions(+), 50 deletions(-)\n\n-Peff\n"},{"id":"510905","messageId":"20250119132308.GA1552263@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250119131224.GA1541095@coredump.intra.peff.net","subject":"[PATCH v2 1/5] bswap.h: squelch potential sparse -Wcast-truncate warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-19T13:23:08Z","receivedAt":"2025-01-19T13:23:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nIn put_be32(), we right-shift a uint32_t value various amounts and then\nassign the low 8-bits to individual \"unsigned char\" bytes, throwing away\nthe high bits. For shifts smaller than 24 bits, those thrown away bits\nwill be arbitrary bits from the original uint32_t.\n\nThis works exactly as we want, but if you feed a constant, then sparse\ncomplains. For example if we write this (which we plan to do in a future\npatch):\n\n  put_be32(hdr, PACK_SIGNATURE);\n\nthen \"make sparse\" produces:\n\n  compat/bswap.h:175:22: error: cast truncates bits from constant value (5041 becomes 41)\n  compat/bswap.h:176:22: error: cast truncates bits from constant value (504143 becomes 43)\n  compat/bswap.h:177:22: error: cast truncates bits from constant value (5041434b becomes 4b)\n\nAnd the same issue exists in the other put_be*() functions, when used\nwith a constant.\n\nWe can silence this warning by explicitly masking off the truncated\nbits. The compiler is smart enough to know the result is the same, and\nthe asm generated by gcc (with both -O0 and -O2) is identical.\n\nCuriously this line already exists:\n\n\tput_be32(&hdr_version, INDEX_EXTENSION_VERSION2);\n\nin the fsmonitor.c file, but it does not get flagged because the CPP\nmacro expands to a small integer (2).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI tweaked the commit message to make more sense in context. Probably\nshould get Junio's signoff. :)\n\nI noticed that reftable/basics.c has its own version of these macros,\nand it also does the masking.\n\n compat/bswap.h | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 512f6f4b99..b34054f2bd 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -171,23 +171,23 @@ static inline uint64_t get_be64(const void *ptr)\n static inline void put_be32(void *ptr, uint32_t value)\n {\n \tunsigned char *p = ptr;\n-\tp[0] = value >> 24;\n-\tp[1] = value >> 16;\n-\tp[2] = value >>  8;\n-\tp[3] = value >>  0;\n+\tp[0] = (value >> 24) & 0xff;\n+\tp[1] = (value >> 16) & 0xff;\n+\tp[2] = (value >>  8) & 0xff;\n+\tp[3] = (value >>  0) & 0xff;\n }\n \n static inline void put_be64(void *ptr, uint64_t value)\n {\n \tunsigned char *p = ptr;\n-\tp[0] = value >> 56;\n-\tp[1] = value >> 48;\n-\tp[2] = value >> 40;\n-\tp[3] = value >> 32;\n-\tp[4] = value >> 24;\n-\tp[5] = value >> 16;\n-\tp[6] = value >>  8;\n-\tp[7] = value >>  0;\n+\tp[0] = (value >> 56) & 0xff;\n+\tp[1] = (value >> 48) & 0xff;\n+\tp[2] = (value >> 40) & 0xff;\n+\tp[3] = (value >> 32) & 0xff;\n+\tp[4] = (value >> 24) & 0xff;\n+\tp[5] = (value >> 16) & 0xff;\n+\tp[6] = (value >>  8) & 0xff;\n+\tp[7] = (value >>  0) & 0xff;\n }\n \n #endif /* COMPAT_BSWAP_H */\n-- \n2.48.1.466.g22b8cfd1fa\n\n"},{"id":"510906","messageId":"20250119132337.GB1552263@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250119131224.GA1541095@coredump.intra.peff.net","subject":"[PATCH v2 2/5] packfile: factor out --pack_header argument parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-19T13:23:37Z","receivedAt":"2025-01-19T13:23:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Both index-pack and unpack-objects accept a --pack_header argument. This\nis an undocumented internal argument used by receive-pack and fetch to\npass along information about the header of the pack, which they've\nalready read from the incoming stream.\n\nIn preparation for a bugfix, let's factor the duplicated code into a\ncommon helper.\n\nThe callers are still responsible for identifying the option. While this\ncould likewise be factored out, it is more flexible this way (e.g., if\nthey ever started using parse-options and wanted to handle both the\nstuck and unstuck forms).\n\nLikewise, the callers are responsible for reporting errors, though they\nboth just call die(). I've tweaked unpack-objects to match index-pack in\nmarking the error for translation.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSame as before.\n\n builtin/index-pack.c     | 14 +++-----------\n builtin/unpack-objects.c | 16 ++++------------\n packfile.c               | 17 +++++++++++++++++\n packfile.h               |  6 ++++++\n 4 files changed, 30 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 0b62b2589f..75b84f78f4 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1955,18 +1955,10 @@ int cmd_index_pack(int argc,\n \t\t\t\t\tnr_threads = 1;\n \t\t\t\t}\n \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tstruct pack_header *hdr;\n-\t\t\t\tchar *c;\n-\n-\t\t\t\thdr = (struct pack_header *)input_buffer;\n-\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n-\t\t\t\tif (*c != ',')\n-\t\t\t\t\tdie(_(\"bad %s\"), arg);\n-\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n-\t\t\t\tif (*c)\n+\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\t\t\t\t\t     input_buffer,\n+\t\t\t\t\t\t\t     &input_len) < 0)\n \t\t\t\t\tdie(_(\"bad %s\"), arg);\n-\t\t\t\tinput_len = sizeof(*hdr);\n \t\t\t} else if (!strcmp(arg, \"-v\")) {\n \t\t\t\tverbose = 1;\n \t\t\t} else if (!strcmp(arg, \"--progress-title\")) {\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 2197d6d933..cf2bc5c531 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -18,6 +18,7 @@\n #include \"progress.h\"\n #include \"decorate.h\"\n #include \"fsck.h\"\n+#include \"packfile.h\"\n \n static int dry_run, quiet, recover, has_errors, strict;\n static const char unpack_usage[] = \"git unpack-objects [-n] [-q] [-r] [--strict]\";\n@@ -645,18 +646,9 @@ int cmd_unpack_objects(int argc,\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tstruct pack_header *hdr;\n-\t\t\t\tchar *c;\n-\n-\t\t\t\thdr = (struct pack_header *)buffer;\n-\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n-\t\t\t\tif (*c != ',')\n-\t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n-\t\t\t\tif (*c)\n-\t\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\t\tlen = sizeof(*hdr);\n+\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\t\t\t\t\t     buffer, &len) < 0)\n+\t\t\t\t\tdie(_(\"bad %s\"), arg);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (skip_prefix(arg, \"--max-input-size=\", &arg)) {\ndiff --git a/packfile.c b/packfile.c\nindex cc7ab6403a..2bf9e57330 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2315,3 +2315,20 @@ int is_promisor_object(struct repository *r, const struct object_id *oid)\n \t}\n \treturn oidset_contains(&promisor_objects, oid);\n }\n+\n+int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len)\n+{\n+\tstruct pack_header *hdr;\n+\tchar *c;\n+\n+\thdr = (struct pack_header *)out;\n+\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n+\thdr->hdr_version = htonl(strtoul(in, &c, 10));\n+\tif (*c != ',')\n+\t\treturn -1;\n+\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\tif (*c)\n+\t\treturn -1;\n+\t*len = sizeof(*hdr);\n+\treturn 0;\n+}\ndiff --git a/packfile.h b/packfile.h\nindex 58104fa009..00ada7a938 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -216,4 +216,10 @@ int is_promisor_object(struct repository *r, const struct object_id *oid);\n int load_idx(const char *path, const unsigned int hashsz, void *idx_map,\n \t     size_t idx_size, struct packed_git *p);\n \n+/*\n+ * Parse a --pack_header option as accepted by index-pack and unpack-objects,\n+ * turning it into the matching bytes we'd find in a pack.\n+ */\n+int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len);\n+\n #endif\n-- \n2.48.1.466.g22b8cfd1fa\n\n"},{"id":"510907","messageId":"20250119132344.GC1552263@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250119131224.GA1541095@coredump.intra.peff.net","subject":"[PATCH v2 3/5] parse_pack_header_option(): avoid unaligned memory writes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-19T13:23:44Z","receivedAt":"2025-01-19T13:23:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In order to recreate a pack header in our in-memory buffer, we cast the\nbuffer to a \"struct pack_header\" and assign the individual fields. This\nis reported to cause SIGBUS on sparc64 due to alignment issues.\n\nWe can work around this by using put_be32() which will write individual\nbytes into the buffer.\n\nReported-by: Koakuma <koachan@protonmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSame as before.\n\n packfile.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 2bf9e57330..2d80d80cb3 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2318,17 +2318,20 @@ int is_promisor_object(struct repository *r, const struct object_id *oid)\n \n int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len)\n {\n-\tstruct pack_header *hdr;\n+\tunsigned char *hdr;\n \tchar *c;\n \n-\thdr = (struct pack_header *)out;\n-\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\thdr->hdr_version = htonl(strtoul(in, &c, 10));\n+\thdr = out;\n+\tput_be32(hdr, PACK_SIGNATURE);\n+\thdr += 4;\n+\tput_be32(hdr, strtoul(in, &c, 10));\n+\thdr += 4;\n \tif (*c != ',')\n \t\treturn -1;\n-\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\tput_be32(hdr, strtoul(c + 1, &c, 10));\n+\thdr += 4;\n \tif (*c)\n \t\treturn -1;\n-\t*len = sizeof(*hdr);\n+\t*len = hdr - out;\n \treturn 0;\n }\n-- \n2.48.1.466.g22b8cfd1fa\n\n"},{"id":"510908","messageId":"20250119132547.GD1552263@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250119131224.GA1541095@coredump.intra.peff.net","subject":"[PATCH v2 4/5] index-pack, unpack-objects: use get_be32() for reading pack header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-19T13:25:47Z","receivedAt":"2025-01-19T13:25:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Both of these commands read the incoming pack into a static unsigned\nchar buffer in BSS, and then parse it by casting the start of the buffer\nto a struct pack_header. This can result in SIGBUS on some platforms if\nthe compiler doesn't place the buffer in a position that is properly\naligned for 4-byte integers.\n\nThis reportedly happens with unpack-objects (but not index-pack) on\nsparc64 when compiled with clang (but not gcc). But we are definitely in\nthe wrong in both spots; since the buffer's type is unsigned char, we\ncan't depend on larger alignment. When it works it is only because we\nare lucky.\n\nWe'll fix this by switching to get_be32() to read the headers (just like\nthe last few commits similarly switched us to put_be32() for writing\ninto the same buffer).\n\nIt would be nice to factor this out into a common helper function, but\nthe interface ends up quite awkward. Either the caller needs to hardcode\nhow many bytes we'll need, or it needs to pass us its fill()/use()\nfunctions as pointers. So I've just fixed both spots in the same way;\nthis is not code that is likely to be repeated a third time (most of the\npack reading code uses an mmap'd buffer, which should be properly\naligned).\n\nI did make one tweak to the shared code: our pack_version_ok() macro\nexpects us to pass the big-endian value we'd get by casting. We can\nintroduce a \"native\" variant which uses the host integer ordering.\n\nReported-by: Koakuma <koachan@protonmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nNew in v2.\n\nI did write the function-pointer version, and it's not even _too_ bad to\nread. But while trying to write a comment documenting the helper, it\nwas just too gross to justify.\n\n builtin/index-pack.c     | 12 +++++++-----\n builtin/unpack-objects.c | 13 +++++++------\n pack.h                   |  3 ++-\n 3 files changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 75b84f78f4..d6fd4bbde6 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -379,16 +379,18 @@ static const char *open_pack_file(const char *pack_name)\n \n static void parse_pack_header(void)\n {\n-\tstruct pack_header *hdr = fill(sizeof(struct pack_header));\n+\tunsigned char *hdr = fill(sizeof(struct pack_header));\n \n \t/* Header consistency check */\n-\tif (hdr->hdr_signature != htonl(PACK_SIGNATURE))\n+\tif (get_be32(hdr) != PACK_SIGNATURE)\n \t\tdie(_(\"pack signature mismatch\"));\n-\tif (!pack_version_ok(hdr->hdr_version))\n+\thdr += 4;\n+\tif (!pack_version_ok_native(get_be32(hdr)))\n \t\tdie(_(\"pack version %\"PRIu32\" unsupported\"),\n-\t\t\tntohl(hdr->hdr_version));\n+\t\t    get_be32(hdr));\n+\thdr += 4;\n \n-\tnr_objects = ntohl(hdr->hdr_entries);\n+\tnr_objects = get_be32(hdr);\n \tuse(sizeof(struct pack_header));\n }\n \ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex cf2bc5c531..76c6a9031b 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -579,15 +579,16 @@ static void unpack_one(unsigned nr)\n static void unpack_all(void)\n {\n \tint i;\n-\tstruct pack_header *hdr = fill(sizeof(struct pack_header));\n+\tunsigned char *hdr = fill(sizeof(struct pack_header));\n \n-\tnr_objects = ntohl(hdr->hdr_entries);\n-\n-\tif (ntohl(hdr->hdr_signature) != PACK_SIGNATURE)\n+\tif (get_be32(hdr) != PACK_SIGNATURE)\n \t\tdie(\"bad pack file\");\n-\tif (!pack_version_ok(hdr->hdr_version))\n+\thdr += 4;\n+\tif (!pack_version_ok_native(get_be32(hdr)))\n \t\tdie(\"unknown pack file version %\"PRIu32,\n-\t\t\tntohl(hdr->hdr_version));\n+\t\t    get_be32(hdr));\n+\thdr += 4;\n+\tnr_objects = get_be32(hdr);\n \tuse(sizeof(struct pack_header));\n \n \tif (!quiet)\ndiff --git a/pack.h b/pack.h\nindex a8da040629..78f8d8b213 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -13,7 +13,8 @@ struct repository;\n  */\n #define PACK_SIGNATURE 0x5041434b\t/* \"PACK\" */\n #define PACK_VERSION 2\n-#define pack_version_ok(v) ((v) == htonl(2) || (v) == htonl(3))\n+#define pack_version_ok(v) pack_version_ok_native(ntohl(v))\n+#define pack_version_ok_native(v) ((v) == 2 || (v) == 3)\n struct pack_header {\n \tuint32_t hdr_signature;\n \tuint32_t hdr_version;\n-- \n2.48.1.466.g22b8cfd1fa\n\n"},{"id":"510909","messageId":"20250119132553.GE1552263@coredump.intra.peff.net","threadId":"62819","inReplyTo":"20250119131224.GA1541095@coredump.intra.peff.net","subject":"[PATCH v2 5/5] index-pack, unpack-objects: use skip_prefix to avoid magic number","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-19T13:25:53Z","receivedAt":"2025-01-19T13:25:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When parsing --pack_header=, we manually skip 14 bytes to the data.\nLet's use skip_prefix() to do this automatically.\n\nNote that we overwrite our pointer to the front of the string, so we\nhave to add more context to the error message. We could avoid this by\ndeclaring an extra pointer to hold the value, but I think the modified\nmessage is actually preferable; it should give translators a bit more\ncontext.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/index-pack.c     | 6 +++---\n builtin/unpack-objects.c | 6 +++---\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex d6fd4bbde6..c632d9f88b 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1956,11 +1956,11 @@ int cmd_index_pack(int argc,\n \t\t\t\t\twarning(_(\"no threads support, ignoring %s\"), arg);\n \t\t\t\t\tnr_threads = 1;\n \t\t\t\t}\n-\t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\t} else if (skip_prefix(arg, \"--pack_header=\", &arg)) {\n+\t\t\t\tif (parse_pack_header_option(arg,\n \t\t\t\t\t\t\t     input_buffer,\n \t\t\t\t\t\t\t     &input_len) < 0)\n-\t\t\t\t\tdie(_(\"bad %s\"), arg);\n+\t\t\t\t\tdie(_(\"bad --pack_header: %s\"), arg);\n \t\t\t} else if (!strcmp(arg, \"-v\")) {\n \t\t\t\tverbose = 1;\n \t\t\t} else if (!strcmp(arg, \"--progress-title\")) {\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 76c6a9031b..51a856d823 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -646,10 +646,10 @@ int cmd_unpack_objects(int argc,\n \t\t\t\tfsck_set_msg_types(&fsck_options, arg);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--pack_header=\")) {\n-\t\t\t\tif (parse_pack_header_option(arg + 14,\n+\t\t\tif (skip_prefix(arg, \"--pack_header=\", &arg)) {\n+\t\t\t\tif (parse_pack_header_option(arg,\n \t\t\t\t\t\t\t     buffer, &len) < 0)\n-\t\t\t\t\tdie(_(\"bad %s\"), arg);\n+\t\t\t\t\tdie(_(\"bad --pack_header: %s\"), arg);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (skip_prefix(arg, \"--max-input-size=\", &arg)) {\n-- \n2.48.1.466.g22b8cfd1fa\n"},{"id":"510952","messageId":"OZMxBc6aAmluYw1_O-0ML1kM7cHNUEuFuhQwX-jurD482L-tjh65TOSWSq-hRzVXvQ2Bnz5SRqzWphewMPH_j-KKSdRtI-f_aNCeIkDLVSI=@protonmail.com","threadId":"62819","inReplyTo":"20250119131224.GA1541095@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/5] git crashes with a SIGBUS on sparc64 during pull","fromName":"Koakuma","fromEmail":"koachan@protonmail.com","sentAt":"2025-01-20T15:20:35Z","receivedAt":"2025-01-20T15:28:08Z","isPatch":true,"sender":{"key":"koachan@protonmail.com","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> Here's a v2 which fixes the reading side, as well. I think this should\n> let you get through a full run of unpack-objects, but please confirm. :)\n> \n> This also includes Junio's put_be32() tweak to silence sparse.\n> \n> [1/5]: bswap.h: squelch potential sparse -Wcast-truncate warnings\n> [2/5]: packfile: factor out --pack_header argument parsing\n> [3/5]: parse_pack_header_option(): avoid unaligned memory writes\n> [4/5]: index-pack, unpack-objects: use get_be32() for reading pack header\n> [5/5]: index-pack, unpack-objects: use skip_prefix to avoid magic number\n> \n> builtin/index-pack.c | 30 ++++++++++++------------------\n> builtin/unpack-objects.c | 31 ++++++++++++-------------------\n> compat/bswap.h | 24 ++++++++++++------------\n> pack.h | 3 ++-\n> packfile.c | 20 ++++++++++++++++++++\n> packfile.h | 6 ++++++\n> 6 files changed, 64 insertions(+), 50 deletions(-)\n> \n> -Peff\n\nOkay, just tested the patchset here.\nBoth the testcase from upthread and actual pulls seem to work well now,\nwithout any crashes happening.\nThanks a lot!\n"},{"id":"511004","messageId":"xmqqed0w2ke3.fsf@gitster.g","threadId":"62819","inReplyTo":"OZMxBc6aAmluYw1_O-0ML1kM7cHNUEuFuhQwX-jurD482L-tjh65TOSWSq-hRzVXvQ2Bnz5SRqzWphewMPH_j-KKSdRtI-f_aNCeIkDLVSI=@protonmail.com","subject":"Re: [PATCH v2 0/5] git crashes with a SIGBUS on sparc64 during pull","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-21T16:37:24Z","receivedAt":"2025-01-21T16:37:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Koakuma <koachan@protonmail.com> writes:\n\n> Jeff King <peff@peff.net> wrote:\n>> Here's a v2 which fixes the reading side, as well. I think this should\n>> let you get through a full run of unpack-objects, but please confirm. :)\n>> ...\n>\n> Okay, just tested the patchset here.\n> Both the testcase from upthread and actual pulls seem to work well now,\n> without any crashes happening.\n> Thanks a lot!\n\nThanks, both.  Will queue.\n"}]}