{"thread":{"id":"50359","subject":"t5702 failing under ASan on master","startedAt":"2019-01-30T08:59:09Z","lastAt":"2019-02-07T04:07:26Z","messageCount":13,"participants":["brian m. carlson","Duy Nguyen","Junio C Hamano","Jonathan Tan"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"368082","messageId":"20190130085855.GA24387@genre.crustytoothpaste.net","threadId":"50359","inReplyTo":null,"subject":"t5702 failing under ASan on master","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-01-30T08:58:56Z","receivedAt":"2019-01-30T08:59:09Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"It looks like t5702 is failing on master under ASan (output below). It\nbisects to the merge of my sha-256 series, but the error makes it look\nlike it's unrelated. I'm wondering if we just happened to have a\ndifferent memory layout with that series that's triggering this issue in\ncombination with one of the protocol v2 series from Jonathan Tan, and\nthe correct solution is something like this:\n\n----- %< -------\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 577faa6229..c9dda154da 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\t\trollback_lock_file(&shallow_lock);\n \t\t} else\n \t\t\tcommit_lock_file(&shallow_lock);\n+\t\talternate_shallow_file = \"\";\n \t\treturn;\n \t}\n \n@@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\t\t\t\t\t&alternate_shallow_file,\n \t\t\t\t\t\t&extra);\n \t\t\tcommit_lock_file(&shallow_lock);\n+\t\t\talternate_shallow_file = \"\";\n \t\t}\n \t\toid_array_clear(&extra);\n \t\treturn;\n@@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\tcommit_lock_file(&shallow_lock);\n \t\toid_array_clear(&extra);\n \t\toid_array_clear(&ref);\n+\t\talternate_shallow_file = \"\";\n \t\treturn;\n \t}\n \n----- %< -------\n\nThis does appear to pass the testsuite, but I'm unsure if it's correct.\nIt's also possible I've just broken something and am too dense to\nrealize it, so please point out if that's the case.\n\nOutput:\n\nexpecting success:\n rm -rf server client trace &&\n\n test_create_repo server &&\n test_commit -C server one &&\n test_commit -C server two &&\n test_commit -C server three &&\n git clone --shallow-exclude two \"file://$(pwd)/server\" client &&\n\n git -C server tag -a -m \"an annotated tag\" twotag two &&\n\n # Triggers tag following (thus, 2 fetches in one process)\n GIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n fetch --shallow-exclude one origin &&\n # Ensure that protocol v2 is used\n grep \"fetch< version 2\" trace\n\nInitialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5702-protocol-v2/server/.git/\n[master (root-commit) 1581e3e] one\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 one.t\n[master 5680d21] two\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 two.t\n[master 94705d7] three\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 three.t\nCloning into 'client'...\nremote: Enumerating objects: 5, done.\nremote: Counting objects: 100% (5/5), done.\nremote: Compressing objects: 100% (2/2), done.\nremote: Total 5 (delta 0), reused 0 (delta 0)\nReceiving objects: 100% (5/5), done.\nremote: Enumerating objects: 4, done.\nremote: Counting objects: 100% (4/4), done.\nremote: Compressing objects: 100% (3/3), done.\nremote: Total 3 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (3/3), done.\nremote: Enumerating objects: 1, done.\nremote: Counting objects: 100% (1/1), done.\nremote: Total 1 (delta 0), reused 0 (delta 0)\n=================================================================\n==628655==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000000a00 at pc 0x14c115e7e8cd bp 0x7ffe4f405a00 sp 0x7ffe4f4051b0\nREAD of size 2 at 0x60c000000a00 thread T0\n #0 0x14c115e7e8cc in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc)\n #1 0x5572f1a4ebe0 in xstrdup /home/bmc/checkouts/git/wrapper.c:44\n #2 0x5572f1773f33 in argv_array_push /home/bmc/checkouts/git/argv-array.c:26\n #3 0x5572f183b380 in get_pack /home/bmc/checkouts/git/fetch-pack.c:786\n #4 0x5572f183fb59 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1391\n #5 0x5572f183fb59 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620\n #6 0x5572f1a1172a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365\n #7 0x5572f1a1554d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296\n #8 0x5572f1623b84 in fetch_refs builtin/fetch.c:1016\n #9 0x5572f162b056 in backfill_tags builtin/fetch.c:1221\n #10 0x5572f162b056 in do_fetch builtin/fetch.c:1321\n #11 0x5572f162b056 in fetch_one builtin/fetch.c:1551\n #12 0x5572f162b056 in cmd_fetch builtin/fetch.c:1640\n #13 0x5572f15af748 in run_builtin /home/bmc/checkouts/git/git.c:422\n #14 0x5572f15af748 in handle_builtin /home/bmc/checkouts/git/git.c:648\n #15 0x5572f15b2651 in run_argv /home/bmc/checkouts/git/git.c:702\n #16 0x5572f15b2651 in cmd_main /home/bmc/checkouts/git/git.c:799\n #17 0x5572f15ad5e3 in main /home/bmc/checkouts/git/common-main.c:45\n #18 0x14c115a5e09a in __libc_start_main ../csu/libc-start.c:308\n #19 0x5572f15aef89 in _start (/home/bmc/checkouts/git/git+0x169f89)\n\n0x60c000000a00 is located 0 bytes inside of 124-byte region [0x60c000000a00,0x60c000000a7c)\nfreed by thread T0 here:\n #0 0x14c115f2cb70 in free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe8b70)\n #1 0x5572f19e56ae in strbuf_release /home/bmc/checkouts/git/strbuf.c:64\n #2 0x5572f1a04e85 in deactivate_tempfile /home/bmc/checkouts/git/tempfile.c:127\n #3 0x5572f1a0625a in rename_tempfile /home/bmc/checkouts/git/tempfile.c:305\n #4 0x5572f187e0b2 in commit_lock_file_to /home/bmc/checkouts/git/lockfile.h:293\n #5 0x5572f187e0b2 in commit_lock_file /home/bmc/checkouts/git/lockfile.c:206\n #6 0x5572f183f463 in update_shallow /home/bmc/checkouts/git/fetch-pack.c:1491\n #7 0x5572f183f463 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1643\n #8 0x5572f1a1172a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365\n #9 0x5572f1a1554d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296\n #10 0x5572f1623b84 in fetch_refs builtin/fetch.c:1016\n #11 0x5572f1629fbd in do_fetch builtin/fetch.c:1307\n #12 0x5572f1629fbd in fetch_one builtin/fetch.c:1551\n #13 0x5572f1629fbd in cmd_fetch builtin/fetch.c:1640\n #14 0x5572f15af748 in run_builtin /home/bmc/checkouts/git/git.c:422\n #15 0x5572f15af748 in handle_builtin /home/bmc/checkouts/git/git.c:648\n #16 0x5572f15b2651 in run_argv /home/bmc/checkouts/git/git.c:702\n #17 0x5572f15b2651 in cmd_main /home/bmc/checkouts/git/git.c:799\n #18 0x5572f15ad5e3 in main /home/bmc/checkouts/git/common-main.c:45\n #19 0x14c115a5e09a in __libc_start_main ../csu/libc-start.c:308\n\npreviously allocated by thread T0 here:\n #0 0x14c115f2d2e0 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe92e0)\n #1 0x5572f1a4ed01 in xrealloc /home/bmc/checkouts/git/wrapper.c:138\n #2 0x5572f19e5b60 in strbuf_grow /home/bmc/checkouts/git/strbuf.c:98\n #3 0x5572f19ea7e3 in strbuf_addch /home/bmc/checkouts/git/strbuf.h:231\n #4 0x5572f19ea7e3 in strbuf_add_absolute_path /home/bmc/checkouts/git/strbuf.c:780\n #5 0x5572f1a05da5 in create_tempfile /home/bmc/checkouts/git/tempfile.c:137\n #6 0x5572f187d12c in lock_file /home/bmc/checkouts/git/lockfile.c:82\n #7 0x5572f187db28 in lock_file_timeout /home/bmc/checkouts/git/lockfile.c:110\n #8 0x5572f187db28 in hold_lock_file_for_update_timeout /home/bmc/checkouts/git/lockfile.c:175\n #9 0x5572f19de0f1 in hold_lock_file_for_update /home/bmc/checkouts/git/lockfile.h:175\n #10 0x5572f19de0f1 in setup_alternate_shallow /home/bmc/checkouts/git/shallow.c:350\n #11 0x5572f1841499 in receive_shallow_info /home/bmc/checkouts/git/fetch-pack.c:1272\n #12 0x5572f1841499 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1384\n #13 0x5572f1841499 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620\n #14 0x5572f1a1172a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365\n #15 0x5572f1a1554d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296\n #16 0x5572f1623b84 in fetch_refs builtin/fetch.c:1016\n #17 0x5572f1629fbd in do_fetch builtin/fetch.c:1307\n #18 0x5572f1629fbd in fetch_one builtin/fetch.c:1551\n #19 0x5572f1629fbd in cmd_fetch builtin/fetch.c:1640\n #20 0x5572f15af748 in run_builtin /home/bmc/checkouts/git/git.c:422\n #21 0x5572f15af748 in handle_builtin /home/bmc/checkouts/git/git.c:648\n #22 0x5572f15b2651 in run_argv /home/bmc/checkouts/git/git.c:702\n #23 0x5572f15b2651 in cmd_main /home/bmc/checkouts/git/git.c:799\n #24 0x5572f15ad5e3 in main /home/bmc/checkouts/git/common-main.c:45\n #25 0x14c115a5e09a in __libc_start_main ../csu/libc-start.c:308\n\nSUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc) in strdup\nShadow bytes around the buggy address:\n 0x0c187fff80f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd\n 0x0c187fff8100: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa\n 0x0c187fff8110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa\n 0x0c187fff8120: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd\n 0x0c187fff8130: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa\n=>0x0c187fff8140:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd\n 0x0c187fff8150: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd\n 0x0c187fff8160: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa\n 0x0c187fff8170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa\n 0x0c187fff8180: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00\n 0x0c187fff8190: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa\nShadow byte legend (one shadow byte represents 8 application bytes):\n Addressable: 00\n Partially addressable: 01 02 03 04 05 06 07\n Heap left redzone: fa\n Freed heap region: fd\n Stack left redzone: f1\n Stack mid redzone: f2\n Stack right redzone: f3\n Stack after return: f5\n Stack use after scope: f8\n Global redzone: f9\n Global init order: f6\n Poisoned by user: f7\n Container overflow: fc\n Array cookie: ac\n Intra object redzone: bb\n ASan internal: fe\n Left alloca redzone: ca\n Right alloca redzone: cb\n==628655==ABORTING\nAborted\nnot ok 30 - ensure that multiple fetches in same process from a shallow repo works\n#\n# rm -rf server client trace &&\n#\n# test_create_repo server &&\n# test_commit -C server one &&\n# test_commit -C server two &&\n# test_commit -C server three &&\n# git clone --shallow-exclude two \"file://$(pwd)/server\" client &&\n#\n# git -C server tag -a -m \"an annotated tag\" twotag two &&\n#\n# # Triggers tag following (thus, 2 fetches in one process)\n# GIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n# fetch --shallow-exclude one origin &&\n# # Ensure that protocol v2 is used\n# grep \"fetch< version 2\" trace\n#\n\nexpecting success:\n rm -rf server client trace &&\n\n test_create_repo server &&\n test_commit -C server one &&\n test_commit -C server two &&\n test_commit -C server three &&\n git clone --depth 1 \"file://$(pwd)/server\" client &&\n test_commit -C server four &&\n\n # Sanity check that only \"three\" is downloaded\n git -C client log --pretty=tformat:%s master >actual &&\n echo three >expected &&\n test_cmp expected actual &&\n\n GIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n fetch --deepen=1 origin &&\n # Ensure that protocol v2 is used\n grep \"fetch< version 2\" trace &&\n\n git -C client log --pretty=tformat:%s origin/master >actual &&\n cat >expected <<-\\EOF &&\n four\n three\n two\n EOF\n test_cmp expected actual\n\nInitialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5702-protocol-v2/server/.git/\n[master (root-commit) 3735e1b] one\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 one.t\n[master 581c9db] two\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 two.t\n[master f6e0668] three\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 three.t\nCloning into 'client'...\nremote: Enumerating objects: 5, done.\nremote: Counting objects: 100% (5/5), done.\nremote: Compressing objects: 100% (2/2), done.\nremote: Total 5 (delta 0), reused 0 (delta 0)\nReceiving objects: 100% (5/5), done.\n[master e79f779] four\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 four.t\nremote: Enumerating objects: 6, done.\nremote: Counting objects: 100% (6/6), done.\nremote: Compressing objects: 100% (4/4), done.\nremote: Total 5 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (5/5), done.\nFrom file:///home/bmc/checkouts/git/t/trash directory.t5702-protocol-v2/server\n f6e0668..e79f779 master -> origin/master\n * [new tag] four -> four\n=================================================================\nremote: Total 0 (delta 0), reused 0 (delta 0)\n==628707==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000000ac0 at pc 0x1519f5dd78cd bp 0x7ffe67609620 sp 0x7ffe67608dd0\nREAD of size 2 at 0x60c000000ac0 thread T0\n #0 0x1519f5dd78cc in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc)\n #1 0x561ad07c7be0 in xstrdup /home/bmc/checkouts/git/wrapper.c:44\n #2 0x561ad04ecf33 in argv_array_push /home/bmc/checkouts/git/argv-array.c:26\n #3 0x561ad05b4380 in get_pack /home/bmc/checkouts/git/fetch-pack.c:786\n #4 0x561ad05b8b59 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1391\n #5 0x561ad05b8b59 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620\n #6 0x561ad078a72a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365\n #7 0x561ad078e54d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296\n #8 0x561ad039cb84 in fetch_refs builtin/fetch.c:1016\n #9 0x561ad03a4056 in backfill_tags builtin/fetch.c:1221\n #10 0x561ad03a4056 in do_fetch builtin/fetch.c:1321\n #11 0x561ad03a4056 in fetch_one builtin/fetch.c:1551\n #12 0x561ad03a4056 in cmd_fetch builtin/fetch.c:1640\n #13 0x561ad0328748 in run_builtin /home/bmc/checkouts/git/git.c:422\n #14 0x561ad0328748 in handle_builtin /home/bmc/checkouts/git/git.c:648\n #15 0x561ad032b651 in run_argv /home/bmc/checkouts/git/git.c:702\n #16 0x561ad032b651 in cmd_main /home/bmc/checkouts/git/git.c:799\n #17 0x561ad03265e3 in main /home/bmc/checkouts/git/common-main.c:45\n #18 0x1519f59b709a in __libc_start_main ../csu/libc-start.c:308\n #19 0x561ad0327f89 in _start (/home/bmc/checkouts/git/git+0x169f89)\n\n0x60c000000ac0 is located 0 bytes inside of 124-byte region [0x60c000000ac0,0x60c000000b3c)\nfreed by thread T0 here:\n #0 0x1519f5e85b70 in free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe8b70)\n #1 0x561ad075e6ae in strbuf_release /home/bmc/checkouts/git/strbuf.c:64\n #2 0x561ad077de85 in deactivate_tempfile /home/bmc/checkouts/git/tempfile.c:127\n #3 0x561ad077f25a in rename_tempfile /home/bmc/checkouts/git/tempfile.c:305\n #4 0x561ad05f70b2 in commit_lock_file_to /home/bmc/checkouts/git/lockfile.h:293\n #5 0x561ad05f70b2 in commit_lock_file /home/bmc/checkouts/git/lockfile.c:206\n #6 0x561ad05b8463 in update_shallow /home/bmc/checkouts/git/fetch-pack.c:1491\n #7 0x561ad05b8463 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1643\n #8 0x561ad078a72a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365\n #9 0x561ad078e54d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296\n #10 0x561ad039cb84 in fetch_refs builtin/fetch.c:1016\n #11 0x561ad03a2fbd in do_fetch builtin/fetch.c:1307\n #12 0x561ad03a2fbd in fetch_one builtin/fetch.c:1551\n #13 0x561ad03a2fbd in cmd_fetch builtin/fetch.c:1640\n #14 0x561ad0328748 in run_builtin /home/bmc/checkouts/git/git.c:422\n #15 0x561ad0328748 in handle_builtin /home/bmc/checkouts/git/git.c:648\n #16 0x561ad032b651 in run_argv /home/bmc/checkouts/git/git.c:702\n #17 0x561ad032b651 in cmd_main /home/bmc/checkouts/git/git.c:799\n #18 0x561ad03265e3 in main /home/bmc/checkouts/git/common-main.c:45\n #19 0x1519f59b709a in __libc_start_main ../csu/libc-start.c:308\n\npreviously allocated by thread T0 here:\n #0 0x1519f5e862e0 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe92e0)\n #1 0x561ad07c7d01 in xrealloc /home/bmc/checkouts/git/wrapper.c:138\n #2 0x561ad075eb60 in strbuf_grow /home/bmc/checkouts/git/strbuf.c:98\n #3 0x561ad07637e3 in strbuf_addch /home/bmc/checkouts/git/strbuf.h:231\n #4 0x561ad07637e3 in strbuf_add_absolute_path /home/bmc/checkouts/git/strbuf.c:780\n #5 0x561ad077eda5 in create_tempfile /home/bmc/checkouts/git/tempfile.c:137\n #6 0x561ad05f612c in lock_file /home/bmc/checkouts/git/lockfile.c:82\n #7 0x561ad05f6b28 in lock_file_timeout /home/bmc/checkouts/git/lockfile.c:110\n #8 0x561ad05f6b28 in hold_lock_file_for_update_timeout /home/bmc/checkouts/git/lockfile.c:175\n #9 0x561ad07570f1 in hold_lock_file_for_update /home/bmc/checkouts/git/lockfile.h:175\n #10 0x561ad07570f1 in setup_alternate_shallow /home/bmc/checkouts/git/shallow.c:350\n #11 0x561ad05ba499 in receive_shallow_info /home/bmc/checkouts/git/fetch-pack.c:1272\n #12 0x561ad05ba499 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1384\n #13 0x561ad05ba499 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620\n #14 0x561ad078a72a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365\n #15 0x561ad078e54d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296\n #16 0x561ad039cb84 in fetch_refs builtin/fetch.c:1016\n #17 0x561ad03a2fbd in do_fetch builtin/fetch.c:1307\n #18 0x561ad03a2fbd in fetch_one builtin/fetch.c:1551\n #19 0x561ad03a2fbd in cmd_fetch builtin/fetch.c:1640\n #20 0x561ad0328748 in run_builtin /home/bmc/checkouts/git/git.c:422\n #21 0x561ad0328748 in handle_builtin /home/bmc/checkouts/git/git.c:648\n #22 0x561ad032b651 in run_argv /home/bmc/checkouts/git/git.c:702\n #23 0x561ad032b651 in cmd_main /home/bmc/checkouts/git/git.c:799\n #24 0x561ad03265e3 in main /home/bmc/checkouts/git/common-main.c:45\n #25 0x1519f59b709a in __libc_start_main ../csu/libc-start.c:308\n\nSUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc) in strdup\nShadow bytes around the buggy address:\n 0x0c187fff8100: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa\n 0x0c187fff8110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa\n 0x0c187fff8120: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd\n 0x0c187fff8130: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa\n 0x0c187fff8140: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd\n=>0x0c187fff8150: fa fa fa fa fa fa fa fa[fd]fd fd fd fd fd fd fd\n 0x0c187fff8160: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa\n 0x0c187fff8170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd\n 0x0c187fff8180: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd\n 0x0c187fff8190: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa\n 0x0c187fff81a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd\nShadow byte legend (one shadow byte represents 8 application bytes):\n Addressable: 00\n Partially addressable: 01 02 03 04 05 06 07\n Heap left redzone: fa\n Freed heap region: fd\n Stack left redzone: f1\n Stack mid redzone: f2\n Stack right redzone: f3\n Stack after return: f5\n Stack use after scope: f8\n Global redzone: f9\n Global init order: f6\n Poisoned by user: f7\n Container overflow: fc\n Array cookie: ac\n Intra object redzone: bb\n ASan internal: fe\n Left alloca redzone: ca\n Right alloca redzone: cb\n==628707==ABORTING\nAborted\nnot ok 31 - deepen-relative\n#\n# rm -rf server client trace &&\n#\n# test_create_repo server &&\n# test_commit -C server one &&\n# test_commit -C server two &&\n# test_commit -C server three &&\n# git clone --depth 1 \"file://$(pwd)/server\" client &&\n# test_commit -C server four &&\n#\n# # Sanity check that only \"three\" is downloaded\n# git -C client log --pretty=tformat:%s master >actual &&\n# echo three >expected &&\n# test_cmp expected actual &&\n#\n# GIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n# fetch --deepen=1 origin &&\n# # Ensure that protocol v2 is used\n# grep \"fetch< version 2\" trace &&\n#\n# git -C client log --pretty=tformat:%s origin/master >actual &&\n# cat >expected <<-\\EOF &&\n# four\n# three\n# two\n# EOF\n# test_cmp expected actual\n#\n\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368105","messageId":"CACsJy8AVoETLkB5rLft27Rz7V9xPFN9fk=8g9-fCR61fVi4vjw@mail.gmail.com","threadId":"50359","inReplyTo":"20190130085855.GA24387@genre.crustytoothpaste.net","subject":"Re: t5702 failing under ASan on master","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-30T10:07:20Z","receivedAt":"2019-01-30T10:07:49Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jan 30, 2019 at 3:59 PM brian m. carlson\n wrote:\n>\n> It looks like t5702 is failing on master under ASan (output below). It\n> bisects to the merge of my sha-256 series, but the error makes it look\n> like it's unrelated. I'm wondering if we just happened to have a\n> different memory layout with that series that's triggering this issue in\n> combination with one of the protocol v2 series from Jonathan Tan, and\n> the correct solution is something like this:\n>\n> ----- %< -------\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 577faa6229..c9dda154da 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> rollback_lock_file(&shallow_lock);\n> } else\n> commit_lock_file(&shallow_lock);\n> + alternate_shallow_file = \"\";\n> return;\n> }\n>\n> @@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> &alternate_shallow_file,\n> &extra);\n> commit_lock_file(&shallow_lock);\n> + alternate_shallow_file = \"\";\n> }\n> oid_array_clear(&extra);\n> return;\n> @@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> commit_lock_file(&shallow_lock);\n> oid_array_clear(&extra);\n> oid_array_clear(&ref);\n> + alternate_shallow_file = \"\";\n> return;\n> }\n>\n> ----- %< -------\n>\n> This does appear to pass the testsuite, but I'm unsure if it's correct.\n> It's also possible I've just broken something and am too dense to\n> realize it, so please point out if that's the case.\n\nIf I understand ASan report correctly alternate_shallow_file memory is\nalready gone after the first fetch, when we update the shallow file.\nBut we're doing two fetches in the same process (the tag backfill\nthingy), the second fetch reuses the dangling alternate_shallow_file\npointer and ASan caught it. Resetting the variable seems like the\nright way to go.\n\nBut should we reset it to an empty string? We would pass\n\"--shallow-file=\" to \"git index-pack\", which is treated as \"no shallow\nfile\" (i.e. complete repo). This sounds wrong because this is still a\nshallow repository.\n\nI suppose setting alternate_shallow_file to NULL would be ok. \"git\nindex-pack\" will just go back to reading $GIT_DIR/info/shallow, which\nhas been updated and contains correct info.\n\nPS. No idea how ASan blames your series for this. Yeah maybe memory\nlayout and stuff. But it does spot a real problem.\n-- \nDuy\n"},{"id":"368111","messageId":"20190130114016.GD24387@genre.crustytoothpaste.net","threadId":"50359","inReplyTo":"CACsJy8AVoETLkB5rLft27Rz7V9xPFN9fk=8g9-fCR61fVi4vjw@mail.gmail.com","subject":"Re: t5702 failing under ASan on master","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-01-30T11:40:16Z","receivedAt":"2019-01-30T11:40:33Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Jan 30, 2019 at 05:07:20PM +0700, Duy Nguyen wrote:\n> If I understand ASan report correctly alternate_shallow_file memory is\n> already gone after the first fetch, when we update the shallow file.\n> But we're doing two fetches in the same process (the tag backfill\n> thingy), the second fetch reuses the dangling alternate_shallow_file\n> pointer and ASan caught it. Resetting the variable seems like the\n> right way to go.\n\nAh, I think I was missing the fact that we're doing a tag backfill. That\nexplains a lot.\n\n> But should we reset it to an empty string? We would pass\n> \"--shallow-file=\" to \"git index-pack\", which is treated as \"no shallow\n> file\" (i.e. complete repo). This sounds wrong because this is still a\n> shallow repository.\n> \n> I suppose setting alternate_shallow_file to NULL would be ok. \"git\n> index-pack\" will just go back to reading $GIT_DIR/info/shallow, which\n> has been updated and contains correct info.\n\nYeah, that sounds like a better choice. I'll send a complete patch which\ndoes this.\n\n> PS. No idea how ASan blames your series for this. Yeah maybe memory\n> layout and stuff. But it does spot a real problem.\n\nI don't doubt this is a problem. We'll definitely want to fix it before\nthe release, since if I see it in development, somebody will likely see\nit in production.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368251","messageId":"xmqqsgx83ddx.fsf@gitster-ct.c.googlers.com","threadId":"50359","inReplyTo":"20190130114016.GD24387@genre.crustytoothpaste.net","subject":"Re: t5702 failing under ASan on master","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-31T18:12:10Z","receivedAt":"2019-01-31T18:12:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On Wed, Jan 30, 2019 at 05:07:20PM +0700, Duy Nguyen wrote:\n>> If I understand ASan report correctly alternate_shallow_file memory is\n>> already gone after the first fetch, when we update the shallow file.\n>> But we're doing two fetches in the same process (the tag backfill\n>> thingy), the second fetch reuses the dangling alternate_shallow_file\n>> pointer and ASan caught it. Resetting the variable seems like the\n>> right way to go.\n>\n> Ah, I think I was missing the fact that we're doing a tag backfill. That\n> explains a lot.\n>\n>> But should we reset it to an empty string? We would pass\n>> \"--shallow-file=\" to \"git index-pack\", which is treated as \"no shallow\n>> file\" (i.e. complete repo). This sounds wrong because this is still a\n>> shallow repository.\n>> \n>> I suppose setting alternate_shallow_file to NULL would be ok. \"git\n>> index-pack\" will just go back to reading $GIT_DIR/info/shallow, which\n>> has been updated and contains correct info.\n>\n> Yeah, that sounds like a better choice. I'll send a complete patch which\n> does this.\n\nThanks, both.\n"},{"id":"368444","messageId":"20190204000650.686175-1-sandals@crustytoothpaste.net","threadId":"50359","inReplyTo":"20190130085855.GA24387@genre.crustytoothpaste.net","subject":"[PATCH] fetch-pack: clear alternate shallow when complete","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-04T00:06:50Z","receivedAt":"2019-02-04T00:07:01Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we write an alternate shallow file in update_shallow, we write it\ninto the lock file. The string stored in alternate_shallow_file is\ncopied from the lock file path, but it is freed the moment that the lock\nfile is closed, since we call strbuf_release to free that path.\n\nThis used to work, since we did not invoke git index-pack more than\nonce, but now that we do, we reuse the freed memory. Ensure we reset the\nvalue to NULL to avoid using freed memory. git index-pack will read the\nrepository's shallow file, which will have been updated with the correct\ninformation.\n\nSigned-off-by: brian m. carlson \n---\n fetch-pack.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 577faa6229..2d76287674 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\t\trollback_lock_file(&shallow_lock);\n \t\t} else\n \t\t\tcommit_lock_file(&shallow_lock);\n+\t\talternate_shallow_file = NULL;\n \t\treturn;\n \t}\n \n@@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\t\t\t\t\t&alternate_shallow_file,\n \t\t\t\t\t\t&extra);\n \t\t\tcommit_lock_file(&shallow_lock);\n+\t\t\talternate_shallow_file = NULL;\n \t\t}\n \t\toid_array_clear(&extra);\n \t\treturn;\n@@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\tcommit_lock_file(&shallow_lock);\n \t\toid_array_clear(&extra);\n \t\toid_array_clear(&ref);\n+\t\talternate_shallow_file = NULL;\n \t\treturn;\n \t}\n \n"},{"id":"368448","messageId":"CACsJy8CvVPr+OJ04aLGDeAS-o5__x1+3cKQNuq2+qba5gw5W+g@mail.gmail.com","threadId":"50359","inReplyTo":"20190204000650.686175-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] fetch-pack: clear alternate shallow when complete","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-04T10:34:38Z","receivedAt":"2019-02-04T10:35:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson\n wrote:\n>\n> When we write an alternate shallow file in update_shallow, we write it\n> into the lock file. The string stored in alternate_shallow_file is\n> copied from the lock file path, but it is freed the moment that the lock\n> file is closed, since we call strbuf_release to free that path.\n>\n> This used to work, since we did not invoke git index-pack more than\n> once, but now that we do, we reuse the freed memory. Ensure we reset the\n> value to NULL to avoid using freed memory. git index-pack will read the\n> repository's shallow file, which will have been updated with the correct\n> information.\n\nIt may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow\nlock unnecessarily - 2019-01-10). I believe this is the same problem\nand a full solution was suggested but not implemented in that commit.\n\nThe problem with dangling alternate_shallow_file is also from that\ncommit. When line_received is false at the end of\nreceive_shallow_info(), we should clear alternate_shallow_file. I'm\nstill debating myself whether we should clear alternate_shallow_file\nin receive_shallow_info() in addition to your changes (which is good\nhygiene anyway) to keep the setup steps of do_fetch_pack() and\ndo_fetch_pack_v2() aligned.\n\npack protocol v1 does this clearing alternate_shallow_file (near the\nend of do_fetch_pack()) so it will not have the same dangling pointer\nproblem even if we do two fetches in the same process. But from the\nTan's commit description, I thought v1 probably faces the same \"lock\ntwice not supported\". But luckily it's never triggered in practice.\nWhen backfill_tags() is called, if I read it correctly, we either\ndisable shallow (TRANS_OPT_DEPTH set to zero) or we start a second\nprocess. In either case, the lock will not be acquired twice by the\nsame process.\n\n> Signed-off-by: brian m. carlson \n> ---\n> fetch-pack.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 577faa6229..2d76287674 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> rollback_lock_file(&shallow_lock);\n> } else\n> commit_lock_file(&shallow_lock);\n> + alternate_shallow_file = NULL;\n> return;\n> }\n>\n> @@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> &alternate_shallow_file,\n> &extra);\n> commit_lock_file(&shallow_lock);\n> + alternate_shallow_file = NULL;\n> }\n> oid_array_clear(&extra);\n> return;\n> @@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> commit_lock_file(&shallow_lock);\n> oid_array_clear(&extra);\n> oid_array_clear(&ref);\n> + alternate_shallow_file = NULL;\n> return;\n> }\n>\n\n\n-- \nDuy\n"},{"id":"368529","messageId":"20190205162636.128901-1-jonathantanmy@google.com","threadId":"50359","inReplyTo":"CACsJy8CvVPr+OJ04aLGDeAS-o5__x1+3cKQNuq2+qba5gw5W+g@mail.gmail.com","subject":"Re: [PATCH] fetch-pack: clear alternate shallow when complete","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-02-05T16:26:36Z","receivedAt":"2019-02-05T16:26:44Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson\n> wrote:\n> >\n> > When we write an alternate shallow file in update_shallow, we write it\n> > into the lock file. The string stored in alternate_shallow_file is\n> > copied from the lock file path, but it is freed the moment that the lock\n> > file is closed, since we call strbuf_release to free that path.\n> >\n> > This used to work, since we did not invoke git index-pack more than\n> > once, but now that we do, we reuse the freed memory. Ensure we reset the\n> > value to NULL to avoid using freed memory. git index-pack will read the\n> > repository's shallow file, which will have been updated with the correct\n> > information.\n> \n> It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow\n> lock unnecessarily - 2019-01-10). I believe this is the same problem\n> and a full solution was suggested but not implemented in that commit.\n\nFor reference, the full solution is [1], linked from that commit's email\n[2]. (Looking back, I probably should have included all the information\nbelow the \"---\" in the commit message proper.) The full solution is more\nrelated to shallow locks, though, not alternate_shallow_file.\n\n[1] https://public-inbox.org/git/20181218010811.143608-1-jonathantanmy@google.com/\n[2] https://public-inbox.org/git/20190110193645.34080-1-jonathantanmy@google.com/\n\n> The problem with dangling alternate_shallow_file is also from that\n> commit.\n\nYou're right - thanks for noticing this.\n\n> When line_received is false at the end of\n> receive_shallow_info(), we should clear alternate_shallow_file. I'm\n> still debating myself whether we should clear alternate_shallow_file\n> in receive_shallow_info() in addition to your changes (which is good\n> hygiene anyway) to keep the setup steps of do_fetch_pack() and\n> do_fetch_pack_v2() aligned.\n\nClearing alternate_shallow_file when line_received is false at the end\nof receive_shallow_info() sounds like a good idea to me.\n"},{"id":"368654","messageId":"20190206232641.GE527887@genre.crustytoothpaste.net","threadId":"50359","inReplyTo":"20190205162636.128901-1-jonathantanmy@google.com","subject":"Re: [PATCH] fetch-pack: clear alternate shallow when complete","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-06T23:26:41Z","receivedAt":"2019-02-06T23:26:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Feb 05, 2019 at 08:26:36AM -0800, Jonathan Tan wrote:\n> > It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow\n> > lock unnecessarily - 2019-01-10). I believe this is the same problem\n> > and a full solution was suggested but not implemented in that commit.\n> \n> For reference, the full solution is [1], linked from that commit's email\n> [2]. (Looking back, I probably should have included all the information\n> below the \"---\" in the commit message proper.) The full solution is more\n> related to shallow locks, though, not alternate_shallow_file.\n> \n> [1] https://public-inbox.org/git/20181218010811.143608-1-jonathantanmy@google.com/\n> [2] https://public-inbox.org/git/20190110193645.34080-1-jonathantanmy@google.com/\n> \n> > The problem with dangling alternate_shallow_file is also from that\n> > commit.\n> \n> You're right - thanks for noticing this.\n> \n> > When line_received is false at the end of\n> > receive_shallow_info(), we should clear alternate_shallow_file. I'm\n> > still debating myself whether we should clear alternate_shallow_file\n> > in receive_shallow_info() in addition to your changes (which is good\n> > hygiene anyway) to keep the setup steps of do_fetch_pack() and\n> > do_fetch_pack_v2() aligned.\n> \n> Clearing alternate_shallow_file when line_received is false at the end\n> of receive_shallow_info() sounds like a good idea to me.\n\nI'll reroll with that change and a commit message update. I think it's\nimportant that we get this issue fixed for the release, and then we can\nchoose to implement a more comprehensive solution later.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368655","messageId":"20190206235937.201663-1-sandals@crustytoothpaste.net","threadId":"50359","inReplyTo":"20190204000650.686175-1-sandals@crustytoothpaste.net","subject":"[PATCH v2] fetch-pack: clear alternate shallow when complete","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-06T23:59:37Z","receivedAt":"2019-02-06T23:59:48Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we write an alternate shallow file in update_shallow, we write it\ninto the lock file. The string stored in alternate_shallow_file is\ncopied from the lock file path, but it is freed the moment that the lock\nfile is closed, since we call strbuf_release to free that path.\n\nThis used to work, since we did not invoke git index-pack more than\nonce. However, we now do, and starting with bd0b42aed3 (fetch-pack: do\nnot take shallow lock unnecessarily - 2019-01-10), we no longer reset\nthis value unconditionally; consequently, we reuse the freed memory.\nEnsure we reset the value to NULL to avoid using freed memory. git\nindex-pack will read the repository's shallow file, which will have been\nupdated with the correct information.\n\nSigned-off-by: brian m. carlson \n---\n fetch-pack.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 577faa6229..a92621a388 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1272,6 +1272,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,\n \t\tsetup_alternate_shallow(&shallow_lock, &alternate_shallow_file,\n \t\t\t\t\tNULL);\n \t\targs->deepen = 1;\n+\t} else {\n+\t\talternate_shallow_file = NULL;\n \t}\n }\n \n@@ -1489,6 +1491,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\t\trollback_lock_file(&shallow_lock);\n \t\t} else\n \t\t\tcommit_lock_file(&shallow_lock);\n+\t\talternate_shallow_file = NULL;\n \t\treturn;\n \t}\n \n@@ -1512,6 +1515,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\t\t\t\t\t&alternate_shallow_file,\n \t\t\t\t\t\t&extra);\n \t\t\tcommit_lock_file(&shallow_lock);\n+\t\t\talternate_shallow_file = NULL;\n \t\t}\n \t\toid_array_clear(&extra);\n \t\treturn;\n@@ -1551,6 +1555,7 @@ static void update_shallow(struct fetch_pack_args *args,\n \t\tcommit_lock_file(&shallow_lock);\n \t\toid_array_clear(&extra);\n \t\toid_array_clear(&ref);\n+\t\talternate_shallow_file = NULL;\n \t\treturn;\n \t}\n \n"},{"id":"368658","messageId":"CACsJy8Ae0ZZjh8+f+TwbfJ-V9SzrHpDFyfjwScwKJj+kc7FDTQ@mail.gmail.com","threadId":"50359","inReplyTo":"20190206235937.201663-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] fetch-pack: clear alternate shallow when complete","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-07T02:32:21Z","receivedAt":"2019-02-07T02:32:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 7, 2019 at 7:00 AM brian m. carlson\n wrote:\n>\n> When we write an alternate shallow file in update_shallow, we write it\n> into the lock file. The string stored in alternate_shallow_file is\n> copied from the lock file path, but it is freed the moment that the lock\n> file is closed, since we call strbuf_release to free that path.\n>\n> This used to work, since we did not invoke git index-pack more than\n> once. However, we now do, and starting with bd0b42aed3 (fetch-pack: do\n> not take shallow lock unnecessarily - 2019-01-10), we no longer reset\n> this value unconditionally; consequently, we reuse the freed memory.\n> Ensure we reset the value to NULL to avoid using freed memory. git\n> index-pack will read the repository's shallow file, which will have been\n> updated with the correct information.\n\nThe patch looks good to me.\n\n>\n> Signed-off-by: brian m. carlson \n> ---\n> fetch-pack.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 577faa6229..a92621a388 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -1272,6 +1272,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,\n> setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,\n> NULL);\n> args->deepen = 1;\n> + } else {\n> + alternate_shallow_file = NULL;\n> }\n> }\n>\n> @@ -1489,6 +1491,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> rollback_lock_file(&shallow_lock);\n> } else\n> commit_lock_file(&shallow_lock);\n> + alternate_shallow_file = NULL;\n> return;\n> }\n>\n> @@ -1512,6 +1515,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> &alternate_shallow_file,\n> &extra);\n> commit_lock_file(&shallow_lock);\n> + alternate_shallow_file = NULL;\n> }\n> oid_array_clear(&extra);\n> return;\n> @@ -1551,6 +1555,7 @@ static void update_shallow(struct fetch_pack_args *args,\n> commit_lock_file(&shallow_lock);\n> oid_array_clear(&extra);\n> oid_array_clear(&ref);\n> + alternate_shallow_file = NULL;\n> return;\n> }\n>\n\n\n-- \nDuy\n"},{"id":"368659","messageId":"xmqq4l9gs49w.fsf@gitster-ct.c.googlers.com","threadId":"50359","inReplyTo":"CACsJy8Ae0ZZjh8+f+TwbfJ-V9SzrHpDFyfjwScwKJj+kc7FDTQ@mail.gmail.com","subject":"Re: [PATCH v2] fetch-pack: clear alternate shallow when complete","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-07T02:47:55Z","receivedAt":"2019-02-07T02:48:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Thu, Feb 7, 2019 at 7:00 AM brian m. carlson\n> wrote:\n>>\n>> When we write an alternate shallow file in update_shallow, we write it\n>> into the lock file. The string stored in alternate_shallow_file is\n>> copied from the lock file path, but it is freed the moment that the lock\n>> file is closed, since we call strbuf_release to free that path.\n>>\n>> This used to work, since we did not invoke git index-pack more than\n>> once. However, we now do, and starting with bd0b42aed3 (fetch-pack: do\n>> not take shallow lock unnecessarily - 2019-01-10), we no longer reset\n>> this value unconditionally; consequently, we reuse the freed memory.\n>> Ensure we reset the value to NULL to avoid using freed memory. git\n>> index-pack will read the repository's shallow file, which will have been\n>> updated with the correct information.\n>\n> The patch looks good to me.\n\nThanks, all.\n"},{"id":"368660","messageId":"xmqqzhr8qpfl.fsf@gitster-ct.c.googlers.com","threadId":"50359","inReplyTo":"xmqq4l9gs49w.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] fetch-pack: clear alternate shallow when complete","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-07T02:53:50Z","receivedAt":"2019-02-07T02:53:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> The patch looks good to me.\n>\n> Thanks, all.\n\nOops, spoke a bit too fast. As the previous one is already in\n'next', let's do this instead.\n\n-- >8 --\nFrom: \"brian m. carlson\" \nDate: Wed, 6 Feb 2019 23:59:37 +0000\nSubject: [PATCH] fetch-pack: clear alternate shallow in one more place\n\nThe previous one did not clear the variable in one codepath,\nbut we should aim to be complete.\n\nSigned-off-by: brian m. carlson \n[jc: made a reroll into incremental, as the previous one already is\n in the next branch]\nSigned-off-by: Junio C Hamano \n---\n fetch-pack.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 2d76287674..a92621a388 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1272,6 +1272,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,\n \t\tsetup_alternate_shallow(&shallow_lock, &alternate_shallow_file,\n \t\t\t\t\tNULL);\n \t\targs->deepen = 1;\n+\t} else {\n+\t\talternate_shallow_file = NULL;\n \t}\n }\n \n-- \n2.20.1-519-g8feddda32c\n\n"},{"id":"368661","messageId":"20190207040717.GA11927@genre.crustytoothpaste.net","threadId":"50359","inReplyTo":"xmqqzhr8qpfl.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] fetch-pack: clear alternate shallow when complete","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-07T04:07:17Z","receivedAt":"2019-02-07T04:07:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Feb 06, 2019 at 06:53:50PM -0800, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> >> The patch looks good to me.\n> >\n> > Thanks, all.\n> \n> Oops, spoke a bit too fast. As the previous one is already in\n> 'next', let's do this instead.\n\nThat's fine with me. Sorry about the delay getting the reroll in.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"}]}