{"thread":{"id":"60467","subject":"[BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","startedAt":"2023-11-03T14:50:28Z","lastAt":"2023-11-04T04:59:29Z","messageCount":10,"participants":["rsbecker@nexbridge.com","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"484395","messageId":"007101da0e65$13e3c170$3bab4450$@nexbridge.com","threadId":"60467","inReplyTo":null,"subject":"[BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-11-03T14:50:19Z","receivedAt":"2023-11-03T14:50:28Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"In RC0, the following tests are failing (with verbose). They look like the\nsame root cause. Unpack(\"Q>\".... What version does git now require for perl?\nI have v5.30.3 available, nothing more recent.\n\nexpecting success of 4216.141 'Bloom reader notices too-small data chunk': \ncheck_corrupt_graph BDAT clear 00000000 &&\necho \"warning: ignoring too-small changed-path chunk\" \\\n\"(4 < 12) in commit-graph file\" >expect.err &&\ntest_cmp expect.err err\n\nInvalid type 'Q' in unpack at\n/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk\n-file.pl line 31.\nnot ok 141 - Bloom reader notices too-small data chunk\n#\n#check_corrupt_graph BDAT clear 00000000 &&\n#echo \"warning: ignoring too-small changed-path chunk\" \\\n#\"(4 < 12) in commit-graph file\" >expect.err &&\n#test_cmp expect.err err\n#\n\nexpecting success of 4216.142 'Bloom reader notices out-of-bounds filter\noffsets': \ncheck_corrupt_graph BIDX 12 FFFFFFFF &&\n# use grep to avoid depending on exact chunk size\ngrep \"warning: ignoring out-of-range offset (4294967295) for changed-path\nfilter at pos 3 of .git/objects/info/commit-graph\" err\n\nInvalid type 'Q' in unpack at\n/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk\n-file.pl line 31.\nnot ok 142 - Bloom reader notices out-of-bounds filter offsets\n#\n#check_corrupt_graph BIDX 12 FFFFFFFF &&\n## use grep to avoid depending on exact chunk size\n#grep \"warning: ignoring out-of-range offset (4294967295) for changed-path\nfilter at pos 3 of .git/objects/info/commit-graph\" err\n#\n\nexpecting success of 4216.143 'Bloom reader notices too-small index chunk': \n# replace the index with a single entry, making most\n# lookups out-of-bounds\ncheck_corrupt_graph BIDX clear 00000000 &&\necho \"warning: commit-graph changed-path index chunk\" \\\n\"is too small\" >expect.err &&\ntest_cmp expect.err err\n\nInvalid type 'Q' in unpack at\n/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk\n-file.pl line 31.\nnot ok 143 - Bloom reader notices too-small index chunk\n#\n## replace the index with a single entry, making most\n## lookups out-of-bounds\n#check_corrupt_graph BIDX clear 00000000 &&\n#echo \"warning: commit-graph changed-path index chunk\" \\\n#\"is too small\" >expect.err &&\n#test_cmp expect.err err\n#\n\nexpecting success of 4216.144 'Bloom reader notices out-of-order index\noffsets': \n# we do not know any real offsets, but we can pick\n# something plausible; we should not get to the point of\n# actually reading from the bogus offsets anyway.\ncorrupt_graph BIDX 4 0000000c00000005 &&\necho \"warning: ignoring decreasing changed-path index offsets\" \\\n\"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph\"\n>expect.err &&\ngit -c core.commitGraph=false log -- A/B/file2 >expect.out &&\ngit -c core.commitGraph=true log -- A/B/file2 >out 2>err &&\ntest_cmp expect.out out &&\ntest_cmp expect.err err\n\nInvalid type 'Q' in unpack at\n/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk\n-file.pl line 31.\nnot ok 144 - Bloom reader notices out-of-order index offsets\n#\n## we do not know any real offsets, but we can pick\n## something plausible; we should not get to the point of\n## actually reading from the bogus offsets anyway.\n#corrupt_graph BIDX 4 0000000c00000005 &&\n#echo \"warning: ignoring decreasing changed-path index offsets\" \\\n#\"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph\"\n>expect.err &&\n#git -c core.commitGraph=false log -- A/B/file2 >expect.out &&\n#git -c core.commitGraph=true log -- A/B/file2 >out 2>err &&\n#test_cmp expect.out out &&\n#test_cmp expect.err err\n#\n\n--\nBrief whoami: NonStop&UNIX developer since approximately\nUNIX(421664400)\nNonStop(211288444200000000)\n-- In real life, I talk too much.\n\n\n"},{"id":"484397","messageId":"007501da0e66$96f124d0$c4d36e70$@nexbridge.com","threadId":"60467","inReplyTo":"007101da0e65$13e3c170$3bab4450$@nexbridge.com","subject":"RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-11-03T15:01:08Z","receivedAt":"2023-11-03T15:01:16Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Friday, November 3, 2023 10:50 AM, I wrote:\n>In RC0, the following tests are failing (with verbose). They look like the\nsame root\n>cause. Unpack(\"Q>\".... What version does git now require for perl?\n>I have v5.30.3 available, nothing more recent.\n>\n>expecting success of 4216.141 'Bloom reader notices too-small data chunk':\n>check_corrupt_graph BDAT clear 00000000 && echo \"warning: ignoring\ntoo-small\n>changed-path chunk\" \\\n>\"(4 < 12) in commit-graph file\" >expect.err && test_cmp expect.err err\n>\n>Invalid type 'Q' in unpack at\n>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun\nk\n>-file.pl line 31.\n>not ok 141 - Bloom reader notices too-small data chunk #\n#check_corrupt_graph\n>BDAT clear 00000000 && #echo \"warning: ignoring too-small changed-path\nchunk\"\n>\\\n>#\"(4 < 12) in commit-graph file\" >expect.err && #test_cmp expect.err err #\n>\n>expecting success of 4216.142 'Bloom reader notices out-of-bounds filter\n>offsets':\n>check_corrupt_graph BIDX 12 FFFFFFFF &&\n># use grep to avoid depending on exact chunk size grep \"warning: ignoring\nout-of-\n>range offset (4294967295) for changed-path filter at pos 3 of\n>.git/objects/info/commit-graph\" err\n>\n>Invalid type 'Q' in unpack at\n>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun\nk\n>-file.pl line 31.\n>not ok 142 - Bloom reader notices out-of-bounds filter offsets #\n>#check_corrupt_graph BIDX 12 FFFFFFFF && ## use grep to avoid depending on\n>exact chunk size #grep \"warning: ignoring out-of-range offset (4294967295)\nfor\n>changed-path filter at pos 3 of .git/objects/info/commit-graph\" err #\n>\n>expecting success of 4216.143 'Bloom reader notices too-small index chunk':\n># replace the index with a single entry, making most # lookups\nout-of-bounds\n>check_corrupt_graph BIDX clear 00000000 && echo \"warning: commit-graph\n>changed-path index chunk\" \\ \"is too small\" >expect.err && test_cmp\nexpect.err err\n>\n>Invalid type 'Q' in unpack at\n>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun\nk\n>-file.pl line 31.\n>not ok 143 - Bloom reader notices too-small index chunk # ## replace the\nindex with\n>a single entry, making most ## lookups out-of-bounds #check_corrupt_graph\nBIDX\n>clear 00000000 && #echo \"warning: commit-graph changed-path index chunk\" \\\n>#\"is too small\" >expect.err && #test_cmp expect.err err #\n>\n>expecting success of 4216.144 'Bloom reader notices out-of-order index\n>offsets':\n># we do not know any real offsets, but we can pick # something plausible;\nwe\n>should not get to the point of # actually reading from the bogus offsets\nanyway.\n>corrupt_graph BIDX 4 0000000c00000005 && echo \"warning: ignoring decreasing\n>changed-path index offsets\" \\\n>\"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph\"\n>>expect.err &&\n>git -c core.commitGraph=false log -- A/B/file2 >expect.out && git -c\n>core.commitGraph=true log -- A/B/file2 >out 2>err && test_cmp expect.out\nout &&\n>test_cmp expect.err err\n>\n>Invalid type 'Q' in unpack at\n>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun\nk\n>-file.pl line 31.\n>not ok 144 - Bloom reader notices out-of-order index offsets # ## we do not\nknow\n>any real offsets, but we can pick ## something plausible; we should not get\nto the\n>point of ## actually reading from the bogus offsets anyway.\n>#corrupt_graph BIDX 4 0000000c00000005 && #echo \"warning: ignoring\n>decreasing changed-path index offsets\" \\\n>#\"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph\"\n>>expect.err &&\n>#git -c core.commitGraph=false log -- A/B/file2 >expect.out && #git -c\n>core.commitGraph=true log -- A/B/file2 >out 2>err && #test_cmp expect.out\nout\n>&& #test_cmp expect.err err #\n>\n\nThis same problem also happens in t5318, t5319, t5324\n--Randall\n\n"},{"id":"484400","messageId":"20231103155220.GA1470570@coredump.intra.peff.net","threadId":"60467","inReplyTo":"007501da0e66$96f124d0$c4d36e70$@nexbridge.com","subject":"Re: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-03T15:52:20Z","receivedAt":"2023-11-03T15:52:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:\n\n> In RC0, the following tests are failing (with verbose). They look like the\n> same root cause. Unpack(\"Q>\".... What version does git now require for perl?\n> I have v5.30.3 available, nothing more recent.\n\nThe perl used in the test suite is supposed to be vanilla enough to\nsupport any ancient version. The perl5 Git import doesn't have version\ntags that go back that far, but the quadwords in pack/unpack go back at\nleast to a commit from 1998.\n\nSo I suspect this is not a version issue, but rather a build-time config\none. The docs say:\n\n  Q  An unsigned quad value.\n      (Quads are available only if your system supports 64-bit integer\n      values _and_ if Perl has been compiled to support those.  Raises\n      an exception otherwise.)\n\nIt would probably be possible to rewrite the use of \"Q\" here to grab two\n32-bit values instead. But I'd guess that on your system it is not as\nsimple as a shift-and-add to then treat them as a 64-bit value, since\npresumably the problem is that perl's ints are all strictly 32-bit.\n\nWhat does this script produce for you:\n\n  perl -e '\n    my $bytes = \"\\1\\2\\3\\4\\5\\6\\7\\8\";\n    my $q = eval { unpack(\"Q>\", $bytes) };\n    print \"Q = \", defined($q) ? $q : \"($@)\", \"\\n\";\n    my ($n1, $n2) = unpack(\"NN\", $bytes);\n    print \"n1 = $n1\\n\";\n    print \"n2 = $n2\\n\";\n    print \"computed quad = \", ($n1 << 32) | $n2, \"\\n\";\n  '\n\nI get:\n\n  Q = 72623859790382904\n  n1 = 16909060\n  n2 = 84281144\n  computed quad = 72623859790382904\n\nbut I'm guessing you get an exception report for Q, and that the\ncomputed quad is probably equal to n2 (the shift of n1 goes totally off\nthe end).\n\nWe may not be without hope, though. These 64-bit values are file offsets\nwe're reading from the chunk files. The format naturally uses 64-bit\nvalues here to accommodate arbitrarily large files. But in our tests,\nthe offsets are all going to be relatively small. So our \"$n1\" in\npractice will always be 0.\n\n> This same problem also happens in t5318, t5319, t5324\n\nYep. The offending code is in lib-chunk.sh, so the new tests added in\nall of those scripts which use it will run into the same problem.\n\n-Peff\n"},{"id":"484401","messageId":"007c01da0e6f$014e2c30$03ea8490$@nexbridge.com","threadId":"60467","inReplyTo":"20231103155220.GA1470570@coredump.intra.peff.net","subject":"RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-11-03T16:01:22Z","receivedAt":"2023-11-03T16:01:38Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Jeff King <peff@peff.net> wrote:\n>On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:\n>> In RC0, the following tests are failing (with verbose). They look like\n>> the same root cause. Unpack(\"Q>\".... What version does git now require for perl?\n>> I have v5.30.3 available, nothing more recent.\n>\n>The perl used in the test suite is supposed to be vanilla enough to support any\n>ancient version. The perl5 Git import doesn't have version tags that go back that far,\n>but the quadwords in pack/unpack go back at least to a commit from 1998.\n>\n>So I suspect this is not a version issue, but rather a build-time config one. The docs\n>say:\n>\n>  Q  An unsigned quad value.\n>      (Quads are available only if your system supports 64-bit integer\n>      values _and_ if Perl has been compiled to support those.  Raises\n>      an exception otherwise.)\n>\n>It would probably be possible to rewrite the use of \"Q\" here to grab two 32-bit\n>values instead. But I'd guess that on your system it is not as simple as a shift-and-\n>add to then treat them as a 64-bit value, since presumably the problem is that perl's\n>ints are all strictly 32-bit.\n>\n>What does this script produce for you:\n>\n>  perl -e '\n>    my $bytes = \"\\1\\2\\3\\4\\5\\6\\7\\8\";\n>    my $q = eval { unpack(\"Q>\", $bytes) };\n>    print \"Q = \", defined($q) ? $q : \"($@)\", \"\\n\";\n>    my ($n1, $n2) = unpack(\"NN\", $bytes);\n>    print \"n1 = $n1\\n\";\n>    print \"n2 = $n2\\n\";\n>    print \"computed quad = \", ($n1 << 32) | $n2, \"\\n\";\n>  '\n>\n>I get:\n>\n>  Q = 72623859790382904\n>  n1 = 16909060\n>  n2 = 84281144\n>  computed quad = 72623859790382904\n>\n>but I'm guessing you get an exception report for Q, and that the computed quad is\n>probably equal to n2 (the shift of n1 goes totally off the end).\n>\n>We may not be without hope, though. These 64-bit values are file offsets we're\n>reading from the chunk files. The format naturally uses 64-bit values here to\n>accommodate arbitrarily large files. But in our tests, the offsets are all going to be\n>relatively small. So our \"$n1\" in practice will always be 0.\n>\n>> This same problem also happens in t5318, t5319, t5324\n>\n>Yep. The offending code is in lib-chunk.sh, so the new tests added in all of those\n>scripts which use it will run into the same problem.\n\nExplains a lot. We are only able to build git in 32-bit mode because of OS dependencies (only available in 32-bit). I did not know that 64-bit was now required for git. We will get there, but it will probably take years.\n\n"},{"id":"484402","messageId":"007f01da0e6f$d5031a40$7f094ec0$@nexbridge.com","threadId":"60467","inReplyTo":"20231103155220.GA1470570@coredump.intra.peff.net","subject":"RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-11-03T16:07:17Z","receivedAt":"2023-11-03T16:07:26Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On November 3, 2023, Jeff King <peff@peff.net> wrote:\n>On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:\n>\n>> In RC0, the following tests are failing (with verbose). They look like\n>> the same root cause. Unpack(\"Q>\".... What version does git now require for perl?\n>> I have v5.30.3 available, nothing more recent.\n>\n>The perl used in the test suite is supposed to be vanilla enough to support any\n>ancient version. The perl5 Git import doesn't have version tags that go back that far,\n>but the quadwords in pack/unpack go back at least to a commit from 1998.\n>\n>So I suspect this is not a version issue, but rather a build-time config one. The docs\n>say:\n>\n>  Q  An unsigned quad value.\n>      (Quads are available only if your system supports 64-bit integer\n>      values _and_ if Perl has been compiled to support those.  Raises\n>      an exception otherwise.)\n>\n>It would probably be possible to rewrite the use of \"Q\" here to grab two 32-bit\n>values instead. But I'd guess that on your system it is not as simple as a shift-and-\n>add to then treat them as a 64-bit value, since presumably the problem is that perl's\n>ints are all strictly 32-bit.\n>\n>What does this script produce for you:\n>\n>  perl -e '\n>    my $bytes = \"\\1\\2\\3\\4\\5\\6\\7\\8\";\n>    my $q = eval { unpack(\"Q>\", $bytes) };\n>    print \"Q = \", defined($q) ? $q : \"($@)\", \"\\n\";\n>    my ($n1, $n2) = unpack(\"NN\", $bytes);\n>    print \"n1 = $n1\\n\";\n>    print \"n2 = $n2\\n\";\n>    print \"computed quad = \", ($n1 << 32) | $n2, \"\\n\";\n>  '\n>\n>I get:\n>\n>  Q = 72623859790382904\n>  n1 = 16909060\n>  n2 = 84281144\n>  computed quad = 72623859790382904\n>\n>but I'm guessing you get an exception report for Q, and that the computed quad is\n>probably equal to n2 (the shift of n1 goes totally off the end).\n>\n>We may not be without hope, though. These 64-bit values are file offsets we're\n>reading from the chunk files. The format naturally uses 64-bit values here to\n>accommodate arbitrarily large files. But in our tests, the offsets are all going to be\n>relatively small. So our \"$n1\" in practice will always be 0.\n>\n>> This same problem also happens in t5318, t5319, t5324\n>\n>Yep. The offending code is in lib-chunk.sh, so the new tests added in all of those\n>scripts which use it will run into the same problem.\n\nWhat I get from Perl is \n$ perl -e '\n>     my $bytes = \"\\1\\2\\3\\4\\5\\6\\7\\8\";\n>     my $q = eval { unpack(\"Q>\", $bytes) };\n>     print \"Q = \", defined($q) ? $q : \"($@)\", \"\\n\";\n>     my ($n1, $n2) = unpack(\"NN\", $bytes);\n>     print \"n1 = $n1\\n\";\n>     print \"n2 = $n2\\n\";\n>     print \"computed quad = \", ($n1 << 32) | $n2, \"\\n\";  '\nQ = (Invalid type 'Q' in unpack at -e line 3.\n)\nn1 = 16909060\nn2 = 84281144\ncomputed quad = 84281144\n\nBecause perl itself is 32-bit, not 64-bit on this platform. So even moving git to 64-bit will not correct the issue.\n\n\n"},{"id":"484403","messageId":"20231103162019.GB1470570@coredump.intra.peff.net","threadId":"60467","inReplyTo":"007c01da0e6f$014e2c30$03ea8490$@nexbridge.com","subject":"[PATCH] t: avoid perl's pack/unpack \"Q\" specifier","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-03T16:20:19Z","receivedAt":"2023-11-03T16:20:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 03, 2023 at 12:01:22PM -0400, rsbecker@nexbridge.com wrote:\n\n> Explains a lot. We are only able to build git in 32-bit mode because\n> of OS dependencies (only available in 32-bit). I did not know that\n> 64-bit was now required for git. We will get there, but it will\n> probably take years.\n\nIt's not required for Git. And even on 32-bit systems there is usually\nsome compiler support for 64-bit types via \"uint64_t\", etc. And likewise\nperl can probably be built for 64-bit types even on a 32-bit system (the\ntests do pass on our Linux 32-bit build), but your perl simply wasn't.\n\nI think we can accommodate it, though. Did you try the snippet I showed?\n\nIf it behaves as I think it does, then this patch should hopefully fix\nthings for you:\n\n-- >8 --\nSubject: [PATCH] t: avoid perl's pack/unpack \"Q\" specifier\n\nThe perl script introduced by 86b008ee61 (t: add library for munging\nchunk-format files, 2023-10-09) uses pack(\"Q\") and unpack(\"Q\") to read\nand write 64-bit values (\"quadwords\" in perl parlance) from the on-disk\nchunk files. However, some builds of perl may not support 64-bit\nintegers at all, and throw an exception here. While some 32-bit\nplatforms may still support 64-bit integers in perl (such as our linux32\nCI environment), others reportedly don't (the NonStop 32-bit builds).\n\nWe can work around this by treating the 64-bit values as two 32-bit\nvalues. We can't ever combine them into a single 64-bit value, but in\npractice this is OK. These are representing file offsets, and our files\nare much smaller than 4GB. So the upper half of the 64-bit value will\nalways be 0.\n\nWe can just introduce a few helper functions which perform the\ntranslation and double-check our assumptions.\n\nReported-by: Randall S. Becker <randall.becker@nexbridge.ca>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nIf this does work, it should go on top of jk/chunk-bounds.\n\n t/lib-chunk/corrupt-chunk-file.pl | 30 +++++++++++++++++++++++++++---\n 1 file changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/t/lib-chunk/corrupt-chunk-file.pl b/t/lib-chunk/corrupt-chunk-file.pl\nindex cd6d386fef..b024bbdcb5 100644\n--- a/t/lib-chunk/corrupt-chunk-file.pl\n+++ b/t/lib-chunk/corrupt-chunk-file.pl\n@@ -21,14 +21,38 @@ sub copy {\n \treturn $buf;\n }\n \n+# Some platforms' perl builds don't support 64-bit integers, and hence do not\n+# allow packing/unpacking quadwords with \"Q\". The chunk format uses 64-bit file\n+# offsets to support files of any size, but in practice our test suite will\n+# only use small files. So we can fake it by asking for two 32-bit values and\n+# discarding the first (most significant) one, which is equivalent as long as\n+# it's just zero.\n+sub unpack_quad {\n+\tmy $bytes = shift;\n+\tmy ($n1, $n2) = unpack(\"NN\", $bytes);\n+\tdie \"quad value exceeds 32 bits\" if $n1;\n+\treturn $n2;\n+};\n+sub pack_quad {\n+\tmy $n = shift;\n+\tmy $ret = pack(\"NN\", 0, $n);\n+\t# double check that our original $n did not exceed the 32-bit limit.\n+\t# This is presumably impossible on a 32-bit system (which would have\n+\t# truncated much earlier), but would still alert us on a 64-bit build\n+\t# of a new test that would fail on a 32-bit build (though we'd\n+\t# presumably see the die() from unpack_quad() in such a case).\n+\tdie \"quad round-trip failed\" if unpack_quad($ret) != $n;\n+\treturn $ret;\n+}\n+\n # read until we find table-of-contents entry for chunk;\n # note that we cheat a bit by assuming 4-byte alignment and\n # that no ToC entry will accidentally look like a header.\n #\n # If we don't find the entry, copy() will hit EOF and exit\n # (which should cause the caller to fail the test).\n while (copy(4) ne $chunk) { }\n-my $offset = unpack(\"Q>\", copy(8));\n+my $offset = unpack_quad(copy(8));\n \n # In clear mode, our length will change. So figure out\n # the length by comparing to the offset of the next chunk, and\n@@ -38,11 +62,11 @@ sub copy {\n \tmy $id;\n \tdo {\n \t\t$id = copy(4);\n-\t\tmy $next = unpack(\"Q>\", get(8));\n+\t\tmy $next = unpack_quad(get(8));\n \t\tif (!defined $len) {\n \t\t\t$len = $next - $offset;\n \t\t}\n-\t\tprint pack(\"Q>\", $next - $len + length($bytes));\n+\t\tprint pack_quad($next - $len + length($bytes));\n \t} while (unpack(\"N\", $id));\n }\n \n-- \n2.42.0.1037.g1c5f731f9d\n\n"},{"id":"484404","messageId":"20231103162119.GC1470570@coredump.intra.peff.net","threadId":"60467","inReplyTo":"007f01da0e6f$d5031a40$7f094ec0$@nexbridge.com","subject":"Re: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-03T16:21:19Z","receivedAt":"2023-11-03T16:21:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 03, 2023 at 12:07:17PM -0400, rsbecker@nexbridge.com wrote:\n\n> What I get from Perl is \n> $ perl -e '\n> >     my $bytes = \"\\1\\2\\3\\4\\5\\6\\7\\8\";\n> >     my $q = eval { unpack(\"Q>\", $bytes) };\n> >     print \"Q = \", defined($q) ? $q : \"($@)\", \"\\n\";\n> >     my ($n1, $n2) = unpack(\"NN\", $bytes);\n> >     print \"n1 = $n1\\n\";\n> >     print \"n2 = $n2\\n\";\n> >     print \"computed quad = \", ($n1 << 32) | $n2, \"\\n\";  '\n> Q = (Invalid type 'Q' in unpack at -e line 3.\n> )\n> n1 = 16909060\n> n2 = 84281144\n> computed quad = 84281144\n\nOK, that matches what I expected. Hopefully the patch I just sent (our\nmails just crossed) will fix it for you.\n\n> Because perl itself is 32-bit, not 64-bit on this platform. So even\n> moving git to 64-bit will not correct the issue.\n\nYep, exactly.\n\n-Peff\n"},{"id":"484410","messageId":"00a801da0e8a$97acc950$c7065bf0$@nexbridge.com","threadId":"60467","inReplyTo":"20231103162119.GC1470570@coredump.intra.peff.net","subject":"RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-11-03T19:18:51Z","receivedAt":"2023-11-03T19:19:04Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On November 3, 2023 12:21 PM Jeff King wrote:\n>On Fri, Nov 03, 2023 at 12:07:17PM -0400, rsbecker@nexbridge.com wrote:\n>\n>> What I get from Perl is\n>> $ perl -e '\n>> >     my $bytes = \"\\1\\2\\3\\4\\5\\6\\7\\8\";\n>> >     my $q = eval { unpack(\"Q>\", $bytes) };\n>> >     print \"Q = \", defined($q) ? $q : \"($@)\", \"\\n\";\n>> >     my ($n1, $n2) = unpack(\"NN\", $bytes);\n>> >     print \"n1 = $n1\\n\";\n>> >     print \"n2 = $n2\\n\";\n>> >     print \"computed quad = \", ($n1 << 32) | $n2, \"\\n\";  '\n>> Q = (Invalid type 'Q' in unpack at -e line 3.\n>> )\n>> n1 = 16909060\n>> n2 = 84281144\n>> computed quad = 84281144\n>\n>OK, that matches what I expected. Hopefully the patch I just sent (our mails just\n>crossed) will fix it for you.\n>\n>> Because perl itself is 32-bit, not 64-bit on this platform. So even\n>> moving git to 64-bit will not correct the issue.\n>\n>Yep, exactly.\n\nYour patch worked! Thanks.\n\n"},{"id":"484422","messageId":"xmqq5y2ii91p.fsf@gitster.g","threadId":"60467","inReplyTo":"20231103162019.GB1470570@coredump.intra.peff.net","subject":"Re: [PATCH] t: avoid perl's pack/unpack \"Q\" specifier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-04T01:47:30Z","receivedAt":"2023-11-04T01:47:38Z","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> +# Some platforms' perl builds don't support 64-bit integers, and hence do not\n> +# allow packing/unpacking quadwords with \"Q\". The chunk format uses 64-bit file\n> +# offsets to support files of any size, but in practice our test suite will\n> +# only use small files. So we can fake it by asking for two 32-bit values and\n> +# discarding the first (most significant) one, which is equivalent as long as\n> +# it's just zero.\n> +sub unpack_quad {\n> +\tmy $bytes = shift;\n> +\tmy ($n1, $n2) = unpack(\"NN\", $bytes);\n> +\tdie \"quad value exceeds 32 bits\" if $n1;\n> +\treturn $n2;\n> +};\n\nIs this an unnecessary ';' at the end?\n\n> +sub pack_quad {\n> +\tmy $n = shift;\n> +\tmy $ret = pack(\"NN\", 0, $n);\n> +\t# double check that our original $n did not exceed the 32-bit limit.\n> +\t# This is presumably impossible on a 32-bit system (which would have\n> +\t# truncated much earlier), but would still alert us on a 64-bit build\n> +\t# of a new test that would fail on a 32-bit build (though we'd\n> +\t# presumably see the die() from unpack_quad() in such a case).\n> +\tdie \"quad round-trip failed\" if unpack_quad($ret) != $n;\n> +\treturn $ret;\n> +}\n\nNice.  Both sub are done carefully.\n\n>  # read until we find table-of-contents entry for chunk;\n>  # note that we cheat a bit by assuming 4-byte alignment and\n>  # that no ToC entry will accidentally look like a header.\n>  #\n>  # If we don't find the entry, copy() will hit EOF and exit\n>  # (which should cause the caller to fail the test).\n>  while (copy(4) ne $chunk) { }\n> -my $offset = unpack(\"Q>\", copy(8));\n> +my $offset = unpack_quad(copy(8));\n>  \n>  # In clear mode, our length will change. So figure out\n>  # the length by comparing to the offset of the next chunk, and\n> @@ -38,11 +62,11 @@ sub copy {\n>  \tmy $id;\n>  \tdo {\n>  \t\t$id = copy(4);\n> -\t\tmy $next = unpack(\"Q>\", get(8));\n> +\t\tmy $next = unpack_quad(get(8));\n>  \t\tif (!defined $len) {\n>  \t\t\t$len = $next - $offset;\n>  \t\t}\n> -\t\tprint pack(\"Q>\", $next - $len + length($bytes));\n> +\t\tprint pack_quad($next - $len + length($bytes));\n>  \t} while (unpack(\"N\", $id));\n>  }\n"},{"id":"484424","messageId":"20231104045926.GA1492014@coredump.intra.peff.net","threadId":"60467","inReplyTo":"xmqq5y2ii91p.fsf@gitster.g","subject":"Re: [PATCH] t: avoid perl's pack/unpack \"Q\" specifier","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-04T04:59:26Z","receivedAt":"2023-11-04T04:59:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 04, 2023 at 10:47:30AM +0900, Junio C Hamano wrote:\n\n> > +sub unpack_quad {\n> > +\tmy $bytes = shift;\n> > +\tmy ($n1, $n2) = unpack(\"NN\", $bytes);\n> > +\tdie \"quad value exceeds 32 bits\" if $n1;\n> > +\treturn $n2;\n> > +};\n> \n> Is this an unnecessary ';' at the end?\n\nOops, yes. I'm not sure how that snuck in there. (It is not breaking\nanything, but if you were to remove it while applying, I would be very\nhappy).\n\n-Peff\n"}]}