{"thread":{"id":"63365","subject":"[BUG] rebase: can write reflog with uninit. `action` string","startedAt":"2025-04-28T19:42:14Z","lastAt":"2025-05-01T17:53:26Z","messageCount":12,"participants":["Kristoffer Haugsbakk","Phillip Wood","Jeff King","phillip.wood123@gmail.com","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"516907","messageId":"20250428194048.149348-1-code@khaugsbakk.name","threadId":"63365","inReplyTo":null,"subject":"[BUG] rebase: can write reflog with uninit. `action` string","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2025-04-28T19:40:43Z","receivedAt":"2025-04-28T19:42:14Z","isPatch":false,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"From: code@khaugsbakk.name\n\nWe did `git rebase --rebase-merges` on a branch with merge commits.  Including\nback merges.  The reflog after that showed some weird symbols for\ncertain merge commits (only merges):\n\n    e9c962f2ea0 HEAD@{8}: <binary>�: Merged in <branch> (pull request #4441)\n\nSome merge commits were normal.\n\nNo backmerges were affected.\n\nWe have a main branch.  This other branch was created from the main\nbranch.  It had been kept up to date with backmerges.  Then someone did\na rebase on it once it was supposed to go into the main branch soonish.\n\nIt looks like the string is uninit.  The values are different each time.\n\n§ Bisection\n\nBisects to d188a60d722 (sequencer: stop exporting GIT_REFLOG_ACTION,\n2022-11-09).\n\n§ Reproduction on latest code\n\nReproduced on `master`, on f65182a99e5 (The ninth batch, 2025-04-24).\n\n§ Gdb\n\nThis is the backtrace when I get the apparently uninit. string:\n\n```\n#0  run_git_commit (defmsg=0x555555babe70 \"<repo path>/MERGE_MSG\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n#1  0x00005555558740d9 in do_merge (r=0x555555a67020 <the_repo>, commit=0x555555b75cb0,\n    arg=0x555555b2164d \"<branch stuff> # Merged in <branch> { (pull request #4464)\\nlabel branch-point-9\\npick 4026b5ced849724bd3857283b6ad50c8609b6d33 only sh\"..., arg_len=125, flags=0, check_todo=0x7fffffffb1e0, opts=0x7fffffffc070) at sequencer.c:4380\n#2  0x0000555555876629 in pick_commits (r=0x555555a67020 <the_repo>, todo_list=0x7fffffffbf50, opts=0x7fffffffc070) at sequencer.c:5048\n#3  0x0000555555877eeb in sequencer_continue (r=0x555555a67020 <the_repo>, opts=0x7fffffffc070) at sequencer.c:5480\n#4  0x000055555563a491 in run_sequencer_rebase (opts=0x7fffffffc330) at builtin/rebase.c:369\n#5  0x000055555563bc74 in run_specific_rebase (opts=0x7fffffffc330) at builtin/rebase.c:746\n#6  0x000055555563fe2a in cmd_rebase (argc=0, argv=0x555555a73890, prefix=0x0, repo=0x555555a67020 <the_repo>) at builtin/rebase.c:1878\n#7  0x0000555555574c0d in run_builtin (p=0x555555a34908 <commands+2280>, argc=2, argv=0x555555a73890, repo=0x555555a67020 <the_repo>) at git.c:480\n#8  0x00005555555750ca in handle_builtin (args=0x7fffffffd8a0) at git.c:743\n#9  0x000055555557538c in run_argv (args=0x7fffffffd8a0) at git.c:810\n#10 0x00005555555759e2 in cmd_main (argc=2, argv=0x7fffffffda30) at git.c:950\n#11 0x000055555569b0c3 in main (argc=5, argv=0x7fffffffda18) at common-main.c:9\n```\n\n§ No reproduction script\n\nI was unable to reproduce with a simple repo. setup.  I tried:\n\n1. Creating a side branch which had a merge\n2. The side branch conflicted with the other branch\n3. Rebased with `--rebase-merges`\n\n-----\n\nNormal bugreport questionaire follows.\n\n-----\n\n    Thank you for filling out a Git bug report!\n    Please answer the following questions to help us understand your issue.\n\n    What did you do before the bug happened? (Steps to reproduce your issue)\n\n`git rebase --rebase-merges` on a branch with merge commits.  Including\nback merges.\n\n    What did you expect to happen? (Expected behavior)\n\nNormal “action” string for the reflog like for example `continue`:\n\n    3f90f6ab14d (HEAD -> <branch>) HEAD@{1}: rebase (continue): Merged in <branch> (pull request #4507)\n\nWhat happened instead? (Actual behavior)\n\nThe “action” (or whatever it is) string is arbitrary bytes.  Like some\nuninit. memory.\n\n```\ne9c962f2ea0 HEAD@{8}: <binary>�: Merged in <branch> (pull request #4441)\n```\n\n    What's different between what you expected and what actually happened?\n\nApparently uninit. string.\n\n     Anything else you want to add:\n\n     Please review the rest of the bug report below.\n     You can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.49.0.459.gf65182a99e5\ncpu: x86_64\nbuilt from commit: f65182a99e545d2f2bc22e6c1c2da192133b16a3\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nlibcurl: 7.81.0\nOpenSSL: OpenSSL 3.0.2 15 Mar 2022\nzlib: 1.2.11\nSHA-1: SHA1_DC\nSHA-256: SHA256_BLK\nuname: Linux 6.8.0-58-generic #60~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 28 16:09:21 UTC 2 x86_64\ncompiler info: gnuc: 11.4\nlibc info: glibc: 2.35\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\n"},{"id":"516944","messageId":"ce0f41e4-7d90-4398-a0e9-e8ba69791e57@gmail.com","threadId":"63365","inReplyTo":"20250428194048.149348-1-code@khaugsbakk.name","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-04-29T09:22:14Z","receivedAt":"2025-04-29T09:22:24Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Kristoffer\n\nOn 28/04/2025 20:40, Kristoffer Haugsbakk wrote:\n> From: code@khaugsbakk.name\n> \n> We did `git rebase --rebase-merges` on a branch with merge commits.  Including\n> back merges.  The reflog after that showed some weird symbols for\n> certain merge commits (only merges):\n> \n>      e9c962f2ea0 HEAD@{8}: <binary>�: Merged in <branch> (pull request #4441)\n\nThanks for reporting this. I'm afraid I'm rather confused. In \nrun_git_commit() we set GIT_REFLOG_ACTION with\n\n\tstrvec_pushf(&cmd.env, GIT_REFLOG_ACTION \"=%s\", ctx->reflog_message);\n\nctx is allocated with calloc() so ctx->reflog_message is initially NULL. \nAt the start of pick_commits() we store the result of \nsequencer_reflog_action() in ctx->reflog_message. That function returns \na heap allocation that is not freed until we call replay_opts_release() \nat the end of the rebase. If we're doing a normal pick rather than a \nmerge we store the result of reflog_message() in ctx->reflog_message. \nThat function returns a string stored in a static strbuf. So when we \ncall do_merge() from pick_commits() ctx->reflog_message should point to \na valid string though that string will not hold the correct reflog \nmessage for the merge because do_merge() does not call reflog_message().\n\n> This is the backtrace when I get the apparently uninit. string:\n> \n> ```\n> #0  run_git_commit (defmsg=0x555555babe70 \"<repo path>/MERGE_MSG\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n\nThanks for the backtrace. It would be useful to know what's stored in \nopts->ctx->reflog_message at this point if it's not too much trouble \nplease can you run \"print *opts\" and \"print *opts->ctx\" here.\n\nThanks\n\nPhillip\n\n> #1  0x00005555558740d9 in do_merge (r=0x555555a67020 <the_repo>, commit=0x555555b75cb0,\n>      arg=0x555555b2164d \"<branch stuff> # Merged in <branch> { (pull request #4464)\\nlabel branch-point-9\\npick 4026b5ced849724bd3857283b6ad50c8609b6d33 only sh\"..., arg_len=125, flags=0, check_todo=0x7fffffffb1e0, opts=0x7fffffffc070) at sequencer.c:4380\n> #2  0x0000555555876629 in pick_commits (r=0x555555a67020 <the_repo>, todo_list=0x7fffffffbf50, opts=0x7fffffffc070) at sequencer.c:5048\n> #3  0x0000555555877eeb in sequencer_continue (r=0x555555a67020 <the_repo>, opts=0x7fffffffc070) at sequencer.c:5480\n> #4  0x000055555563a491 in run_sequencer_rebase (opts=0x7fffffffc330) at builtin/rebase.c:369\n> #5  0x000055555563bc74 in run_specific_rebase (opts=0x7fffffffc330) at builtin/rebase.c:746\n> #6  0x000055555563fe2a in cmd_rebase (argc=0, argv=0x555555a73890, prefix=0x0, repo=0x555555a67020 <the_repo>) at builtin/rebase.c:1878\n> #7  0x0000555555574c0d in run_builtin (p=0x555555a34908 <commands+2280>, argc=2, argv=0x555555a73890, repo=0x555555a67020 <the_repo>) at git.c:480\n> #8  0x00005555555750ca in handle_builtin (args=0x7fffffffd8a0) at git.c:743\n> #9  0x000055555557538c in run_argv (args=0x7fffffffd8a0) at git.c:810\n> #10 0x00005555555759e2 in cmd_main (argc=2, argv=0x7fffffffda30) at git.c:950\n> #11 0x000055555569b0c3 in main (argc=5, argv=0x7fffffffda18) at common-main.c:9\n> ```\n> \n> § No reproduction script\n> \n> I was unable to reproduce with a simple repo. setup.  I tried:\n> \n> 1. Creating a side branch which had a merge\n> 2. The side branch conflicted with the other branch\n> 3. Rebased with `--rebase-merges`\n> \n> -----\n> \n> Normal bugreport questionaire follows.\n> \n> -----\n> \n>      Thank you for filling out a Git bug report!\n>      Please answer the following questions to help us understand your issue.\n> \n>      What did you do before the bug happened? (Steps to reproduce your issue)\n> \n> `git rebase --rebase-merges` on a branch with merge commits.  Including\n> back merges.\n> \n>      What did you expect to happen? (Expected behavior)\n> \n> Normal “action” string for the reflog like for example `continue`:\n> \n>      3f90f6ab14d (HEAD -> <branch>) HEAD@{1}: rebase (continue): Merged in <branch> (pull request #4507)\n> \n> What happened instead? (Actual behavior)\n> \n> The “action” (or whatever it is) string is arbitrary bytes.  Like some\n> uninit. memory.\n> \n> ```\n> e9c962f2ea0 HEAD@{8}: <binary>�: Merged in <branch> (pull request #4441)\n> ```\n> \n>      What's different between what you expected and what actually happened?\n> \n> Apparently uninit. string.\n> \n>       Anything else you want to add:\n> \n>       Please review the rest of the bug report below.\n>       You can delete any lines you don't wish to share.\n> \n> \n> [System Info]\n> git version:\n> git version 2.49.0.459.gf65182a99e5\n> cpu: x86_64\n> built from commit: f65182a99e545d2f2bc22e6c1c2da192133b16a3\n> sizeof-long: 8\n> sizeof-size_t: 8\n> shell-path: /bin/sh\n> libcurl: 7.81.0\n> OpenSSL: OpenSSL 3.0.2 15 Mar 2022\n> zlib: 1.2.11\n> SHA-1: SHA1_DC\n> SHA-256: SHA256_BLK\n> uname: Linux 6.8.0-58-generic #60~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 28 16:09:21 UTC 2 x86_64\n> compiler info: gnuc: 11.4\n> libc info: glibc: 2.35\n> $SHELL (typically, interactive shell): /bin/bash\n> \n> \n> [Enabled Hooks]\n> \n\n"},{"id":"516983","messageId":"fbc97d6a-2022-4a64-a2ba-5a7255cd81a6@app.fastmail.com","threadId":"63365","inReplyTo":"ce0f41e4-7d90-4398-a0e9-e8ba69791e57@gmail.com","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2025-04-29T19:40:13Z","receivedAt":"2025-04-29T19:40:39Z","isPatch":false,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi Phillip\n\nOn Tue, Apr 29, 2025, at 11:22, Phillip Wood wrote:\n>> #0  run_git_commit (defmsg=0x555555babe70 \"<repo path>/MERGE_MSG\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n>\n> Thanks for the backtrace. It would be useful to know what's stored in\n> opts->ctx->reflog_message at this point if it's not too much trouble\n> please can you run \"print *opts\" and \"print *opts->ctx\" here.\n\nToday I ran on f65182a99e5 (The ninth batch, 2025-04-24) at\nsequencer.c:1148.  I was never able to reproduce this\n`opts->ctx->reflog_message` having a weird value with GDB today.  The\nreflog was also fine.\n\nThen I ran without GDB and I got the weird reflog that I expected.\n\nSo I don’t know what `*opts` or `*opts->ctx` looks like here.  But I did\nfind just two minutes ago some old notes about `ctx->reflog_message`:\n\n```\nThread 1 \"git\" hit Breakpoint 1, run_git_commit (defmsg=0x555555babe70 \"<merge msg path>\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n1158\t\tstrvec_pushf(&cmd.env, GIT_REFLOG_ACTION \"=%s\", ctx->reflog_message);\n@(gdb) p ctx->reflog_message\n$23 = 0x555555ba0d50 \"\\250y\\267UUU\"\n@(gdb)\n```\n\nIt’s line 1158 because of my debug code apparently.\n"},{"id":"516989","messageId":"20250429215155.GA36727@coredump.intra.peff.net","threadId":"63365","inReplyTo":"fbc97d6a-2022-4a64-a2ba-5a7255cd81a6@app.fastmail.com","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-04-29T21:51:55Z","receivedAt":"2025-04-29T21:58:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 29, 2025 at 09:40:13PM +0200, Kristoffer Haugsbakk wrote:\n\n> On Tue, Apr 29, 2025, at 11:22, Phillip Wood wrote:\n> >> #0  run_git_commit (defmsg=0x555555babe70 \"<repo path>/MERGE_MSG\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n> >\n> > Thanks for the backtrace. It would be useful to know what's stored in\n> > opts->ctx->reflog_message at this point if it's not too much trouble\n> > please can you run \"print *opts\" and \"print *opts->ctx\" here.\n> \n> Today I ran on f65182a99e5 (The ninth batch, 2025-04-24) at\n> sequencer.c:1148.  I was never able to reproduce this\n> `opts->ctx->reflog_message` having a weird value with GDB today.  The\n> reflog was also fine.\n> \n> Then I ran without GDB and I got the weird reflog that I expected.\n\nHave you tried building with \"make SANITIZE=address,undefined\"?\n\nThis is a wild guess, but since ctx->reflog_message is pointing to a\nstatic strbuf, it could be a use after free if the strbuf is reallocated\ndue to another call to reflog_message(), but we are still holding the\nold pointer via ctx->reflog_message.\n\n-Peff\n"},{"id":"517024","messageId":"6743a9fc-11ca-45ac-bc40-4148f5d85d27@app.fastmail.com","threadId":"63365","inReplyTo":"20250429215155.GA36727@coredump.intra.peff.net","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2025-04-30T15:17:38Z","receivedAt":"2025-04-30T15:18:07Z","isPatch":false,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Apr 29, 2025, at 23:51, Jeff King wrote:\n> On Tue, Apr 29, 2025 at 09:40:13PM +0200, Kristoffer Haugsbakk wrote:\n>\n>> On Tue, Apr 29, 2025, at 11:22, Phillip Wood wrote:\n>> >> #0  run_git_commit (defmsg=0x555555babe70 \"<repo path>/MERGE_MSG\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n>> >\n>> > Thanks for the backtrace. It would be useful to know what's stored in\n>> > opts->ctx->reflog_message at this point if it's not too much trouble\n>> > please can you run \"print *opts\" and \"print *opts->ctx\" here.\n>>\n>> Today I ran on f65182a99e5 (The ninth batch, 2025-04-24) at\n>> sequencer.c:1148.  I was never able to reproduce this\n>> `opts->ctx->reflog_message` having a weird value with GDB today.  The\n>> reflog was also fine.\n>>\n>> Then I ran without GDB and I got the weird reflog that I expected.\n>\n> Have you tried building with \"make SANITIZE=address,undefined\"?\n\nNo I haven’t.  Thank you.  The following is with that `make`.\n\nStill on f65182a99e5 (The ninth batch, 2025-04-24).  I eventually[1]\ngot this:\n\n[1] I run through 19 merge conflicts which I `--continue` (using rerere)\n    until the rebase is done\n\n```\ndetached HEAD 5d96584c836] Merge branch '<branch>' into <something else>\n Author: [author]\n=================================================================\n==87324==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300001daa0 at pc 0x79371ca5df89 bp 0x7fff8e215a50 sp 0x7fff8e2151c8\nREAD of size 2 at 0x60300001daa0 thread T0\n    #0 0x79371ca5df88 in printf_common ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:553\n    #1 0x79371ca5fbd5 in __interceptor_vsnprintf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1668\n    #2 0x5702aa6bd851 in strbuf_vaddf /home/kristoffer/programming/git/strbuf.c:415\n    #3 0x5702aa6d2384 in strvec_pushf /home/kristoffer/programming/git/strvec.c:35\n    #4 0x5702aa629087 in run_git_commit /home/kristoffer/programming/git/sequencer.c:1148\n    #5 0x5702aa64c652 in do_merge /home/kristoffer/programming/git/sequencer.c:4363\n    #6 0x5702aa655714 in pick_commits /home/kristoffer/programming/git/sequencer.c:5029\n    #7 0x5702aa659a68 in sequencer_continue /home/kristoffer/programming/git/sequencer.c:5461\n    #8 0x5702a9d0f5ce in run_sequencer_rebase builtin/rebase.c:370\n    #9 0x5702a9d14cd9 in run_specific_rebase builtin/rebase.c:747\n    #10 0x5702a9d23798 in cmd_rebase builtin/rebase.c:1887\n    #11 0x5702a9a3c26c in run_builtin /home/kristoffer/programming/git/git.c:480\n    #12 0x5702a9a3d3d5 in handle_builtin /home/kristoffer/programming/git/git.c:744\n    #13 0x5702a9a3dc2c in run_argv /home/kristoffer/programming/git/git.c:811\n    #14 0x5702a9a3f17c in cmd_main /home/kristoffer/programming/git/git.c:951\n    #15 0x5702a9e77aa3 in main /home/kristoffer/programming/git/common-main.c:9\n    #16 0x79371be29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n    #17 0x79371be29e3f in __libc_start_main_impl ../csu/libc-start.c:392\n    #18 0x5702a9a35384 in _start (/home/kristoffer/programming/git/git+0x12a0384)\n\n0x60300001daa0 is located 0 bytes inside of 24-byte region [0x60300001daa0,0x60300001dab8)\nfreed by thread T0 here:\n    #0 0x79371cab4c38 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164\n    #1 0x5702aa7ecdca in xrealloc /home/kristoffer/programming/git/wrapper.c:140\n    #2 0x5702aa6b83b7 in strbuf_grow /home/kristoffer/programming/git/strbuf.c:114\n    #3 0x5702aa6bd8ca in strbuf_vaddf /home/kristoffer/programming/git/strbuf.c:420\n    #4 0x5702aa6476dc in reflog_message /home/kristoffer/programming/git/sequencer.c:3948\n    #5 0x5702aa648e42 in do_reset /home/kristoffer/programming/git/sequencer.c:4059\n    #6 0x5702aa65545e in pick_commits /home/kristoffer/programming/git/sequencer.c:5026\n    #7 0x5702aa659a68 in sequencer_continue /home/kristoffer/programming/git/sequencer.c:5461\n    #8 0x5702a9d0f5ce in run_sequencer_rebase builtin/rebase.c:370\n    #9 0x5702a9d14cd9 in run_specific_rebase builtin/rebase.c:747\n    #10 0x5702a9d23798 in cmd_rebase builtin/rebase.c:1887\n    #11 0x5702a9a3c26c in run_builtin /home/kristoffer/programming/git/git.c:480\n    #12 0x5702a9a3d3d5 in handle_builtin /home/kristoffer/programming/git/git.c:744\n    #13 0x5702a9a3dc2c in run_argv /home/kristoffer/programming/git/git.c:811\n    #14 0x5702a9a3f17c in cmd_main /home/kristoffer/programming/git/git.c:951\n    #15 0x5702a9e77aa3 in main /home/kristoffer/programming/git/common-main.c:9\n    #16 0x79371be29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n\npreviously allocated by thread T0 here:\n    #0 0x79371cab4c38 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164\n    #1 0x5702aa7ecdca in xrealloc /home/kristoffer/programming/git/wrapper.c:140\n    #2 0x5702aa6b83b7 in strbuf_grow /home/kristoffer/programming/git/strbuf.c:114\n    #3 0x5702aa6bbeae in strbuf_add /home/kristoffer/programming/git/strbuf.c:313\n    #4 0x5702aa61da33 in strbuf_addstr /home/kristoffer/programming/git/strbuf.h:310\n    #5 0x5702aa64766a in reflog_message /home/kristoffer/programming/git/sequencer.c:3943\n    #6 0x5702aa659637 in sequencer_continue /home/kristoffer/programming/git/sequencer.c:5426\n    #7 0x5702a9d0f5ce in run_sequencer_rebase builtin/rebase.c:370\n    #8 0x5702a9d14cd9 in run_specific_rebase builtin/rebase.c:747\n    #9 0x5702a9d23798 in cmd_rebase builtin/rebase.c:1887\n    #10 0x5702a9a3c26c in run_builtin /home/kristoffer/programming/git/git.c:480\n    #11 0x5702a9a3d3d5 in handle_builtin /home/kristoffer/programming/git/git.c:744\n    #12 0x5702a9a3dc2c in run_argv /home/kristoffer/programming/git/git.c:811\n    #13 0x5702a9a3f17c in cmd_main /home/kristoffer/programming/git/git.c:951\n    #14 0x5702a9e77aa3 in main /home/kristoffer/programming/git/common-main.c:9\n    #15 0x79371be29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n\nSUMMARY: AddressSanitizer: heap-use-after-free ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:553 in printf_common\nShadow bytes around the buggy address:\n  0x0c067fffbb00: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fa\n  0x0c067fffbb10: fa fa fd fd fd fd fa fa fd fd fd fa fa fa fd fd\n  0x0c067fffbb20: fd fd fa fa fd fd fd fd fa fa fd fd fd fd fa fa\n  0x0c067fffbb30: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd\n  0x0c067fffbb40: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd\n=>0x0c067fffbb50: fd fa fa fa[fd]fd fd fa fa fa fd fd fd fa fa fa\n  0x0c067fffbb60: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa\n  0x0c067fffbb70: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd\n  0x0c067fffbb80: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa\n  0x0c067fffbb90: fd fd fd fd fa fa fd fd fd fa fa fa fd fd fd fd\n  0x0c067fffbba0: fa fa fd fd fd fa fa fa fd fd fd fd fa fa 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  Shadow gap:              cc\n==87324==ABORTING\n```\n\n> This is a wild guess, but since ctx->reflog_message is pointing to a\n> static strbuf, it could be a use after free if the strbuf is reallocated\n> due to another call to reflog_message(), but we are still holding the\n> old pointer via ctx->reflog_message.\n"},{"id":"517042","messageId":"20250501131751.GA1725607@coredump.intra.peff.net","threadId":"63365","inReplyTo":"6743a9fc-11ca-45ac-bc40-4148f5d85d27@app.fastmail.com","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-01T13:17:51Z","receivedAt":"2025-05-01T13:17:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2025 at 05:17:38PM +0200, Kristoffer Haugsbakk wrote:\n\n> > Have you tried building with \"make SANITIZE=address,undefined\"?\n> \n> No I haven’t.  Thank you.  The following is with that `make`.\n> \n> Still on f65182a99e5 (The ninth batch, 2025-04-24).  I eventually[1]\n> got this:\n> \n> [1] I run through 19 merge conflicts which I `--continue` (using rerere)\n>     until the rebase is done\n> \n> ```\n> detached HEAD 5d96584c836] Merge branch '<branch>' into <something else>\n>  Author: [author]\n> =================================================================\n> ==87324==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300001daa0 at pc 0x79371ca5df89 bp 0x7fff8e215a50 sp 0x7fff8e2151c8\n\nMakes sense. Presumably it triggers in your case but not others because\nsomething about your particular reflog messages causes the strbuf to\nreallocate (I guess one of them is a lot longer than the others).\n\nBut we should be able to trigger it reliably with this:\n\ndiff --git a/sequencer.c b/sequencer.c\nindex b5c4043757..43db0ce66b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3939,7 +3939,7 @@ static const char *reflog_message(struct replay_opts *opts,\n \tstatic struct strbuf buf = STRBUF_INIT;\n \n \tva_start(ap, fmt);\n-\tstrbuf_reset(&buf);\n+\tstrbuf_release(&buf); /* guarantees reallocation */\n \tstrbuf_addstr(&buf, sequencer_reflog_action(opts));\n \tif (sub_action)\n \t\tstrbuf_addf(&buf, \" (%s)\", sub_action);\n\nAnd indeed, building with that patch and SANITIZE=address seems to show\nthe problem reliably with the existing tests in t3430 (it might also\nshow up without ASan, but probably not as reliably).\n\nProbably the smallest solution is for ctx->reflog_message to copy the\nresult and always own the memory (and then remember to free it, both at\ncleanup and if it is ever overwritten).\n\nBut I think the way reflog_message() returns the \"buf\" member of a\nstatic strbuf is kind of an anti-pattern, exactly because you can get\nthis kind of subtle re-use. It probably should just return a non-const\npointer, handing over memory ownership to the caller. That would require\nadjusting its other callers, too.\n\nSo the \"smallest\" version is perhaps something like this, totally\nuntested except for confirming that t3430 no longer complains:\n\ndiff --git a/sequencer.c b/sequencer.c\nindex b5c4043757..07aa3b3731 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -228,7 +228,7 @@ struct replay_ctx {\n \t * Stores the reflog message that will be used when creating a\n \t * commit. Points to a static buffer and should not be free()'d.\n \t */\n-\tconst char *reflog_message;\n+\tchar *reflog_message;\n \t/*\n \t * The number of completed fixup and squash commands in the\n \t * current chain.\n@@ -411,6 +411,7 @@ static void replay_ctx_release(struct replay_ctx *ctx)\n {\n \tstrbuf_release(&ctx->current_fixups);\n \tstrbuf_release(&ctx->message);\n+\tfree(ctx->reflog_message);\n }\n \n void replay_opts_release(struct replay_opts *opts)\n@@ -3939,7 +3940,7 @@ static const char *reflog_message(struct replay_opts *opts,\n \tstatic struct strbuf buf = STRBUF_INIT;\n \n \tva_start(ap, fmt);\n-\tstrbuf_reset(&buf);\n+\tstrbuf_release(&buf); /* guarantees realloaction */\n \tstrbuf_addstr(&buf, sequencer_reflog_action(opts));\n \tif (sub_action)\n \t\tstrbuf_addf(&buf, \" (%s)\", sub_action);\n@@ -4886,9 +4887,11 @@ static int pick_one_commit(struct repository *r,\n \tint res;\n \tstruct todo_item *item = todo_list->items + todo_list->current;\n \tconst char *arg = todo_item_get_arg(todo_list, item);\n-\tif (is_rebase_i(opts))\n-\t\tctx->reflog_message = reflog_message(\n-\t\t\topts, command_to_string(item->command), NULL);\n+\tif (is_rebase_i(opts)) {\n+\t\tfree(ctx->reflog_message);\n+\t\tctx->reflog_message = xstrdup(reflog_message(\n+\t\t\topts, command_to_string(item->command), NULL));\n+\t}\n \n \tres = do_pick_commit(r, item, opts, is_final_fixup(todo_list),\n \t\t\t     check_todo);\n@@ -4947,7 +4950,8 @@ static int pick_commits(struct repository *r,\n \tstruct replay_ctx *ctx = opts->ctx;\n \tint res = 0, reschedule = 0;\n \n-\tctx->reflog_message = sequencer_reflog_action(opts);\n+\tfree(ctx->reflog_message);\n+\tctx->reflog_message = xstrdup(sequencer_reflog_action(opts));\n \tif (opts->allow_ff)\n \t\tASSERT(!(opts->signoff || opts->no_commit ||\n \t\t\t opts->record_origin || should_edit(opts) ||\n@@ -5423,7 +5427,8 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)\n \t\t\tunlink(rebase_path_dropped());\n \t\t}\n \n-\t\tctx->reflog_message = reflog_message(opts, \"continue\", NULL);\n+\t\tfree(ctx->reflog_message);\n+\t\tctx->reflog_message = xstrdup(reflog_message(opts, \"continue\", NULL));\n \t\tif (commit_staged_changes(r, opts, &todo_list)) {\n \t\t\tres = -1;\n \t\t\tgoto release_todo_list;\n@@ -5475,7 +5480,8 @@ static int single_pick(struct repository *r,\n \t\t\tTODO_PICK : TODO_REVERT;\n \titem.commit = cmit;\n \n-\topts->ctx->reflog_message = sequencer_reflog_action(opts);\n+\tfree(opts->ctx->reflog_message);\n+\topts->ctx->reflog_message = xstrdup(sequencer_reflog_action(opts));\n \treturn do_pick_commit(r, &item, opts, 0, &check_todo);\n }\n \n\nI'm hoping your or Phillip can decide on the best fix from here.\n\n-Peff\n"},{"id":"517045","messageId":"a834c708-8904-44c5-9481-5f796cf0054e@gmail.com","threadId":"63365","inReplyTo":"20250429215155.GA36727@coredump.intra.peff.net","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-05-01T14:10:44Z","receivedAt":"2025-05-01T14:11:02Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Peff\n\nOn 29/04/2025 22:51, Jeff King wrote:\n> On Tue, Apr 29, 2025 at 09:40:13PM +0200, Kristoffer Haugsbakk wrote:\n> \n>> On Tue, Apr 29, 2025, at 11:22, Phillip Wood wrote:\n>>>> #0  run_git_commit (defmsg=0x555555babe70 \"<repo path>/MERGE_MSG\", opts=0x7fffffffc070, flags=0) at sequencer.c:1158\n>>>\n>>> Thanks for the backtrace. It would be useful to know what's stored in\n>>> opts->ctx->reflog_message at this point if it's not too much trouble\n>>> please can you run \"print *opts\" and \"print *opts->ctx\" here.\n>>\n>> Today I ran on f65182a99e5 (The ninth batch, 2025-04-24) at\n>> sequencer.c:1148.  I was never able to reproduce this\n>> `opts->ctx->reflog_message` having a weird value with GDB today.  The\n>> reflog was also fine.\n>>\n>> Then I ran without GDB and I got the weird reflog that I expected.\n> \n> Have you tried building with \"make SANITIZE=address,undefined\"?\n> \n> This is a wild guess, but since ctx->reflog_message is pointing to a\n> static strbuf, it could be a use after free if the strbuf is reallocated\n> due to another call to reflog_message(), but we are still holding the\n> old pointer via ctx->reflog_message.\n\nOh, nice insight. I'd forgotten we had callers of reflog_message() that \ndidn't store the result in ctx->reflog_message. One of those callers is \nin do_reset() which due to the way the todo list gets constructed is \nlikely to be called just before do_merge().\n\nThanks\n\nPhillip\n"},{"id":"517046","messageId":"e4ea1e44-af7e-4c24-b0ee-f87091200b90@gmail.com","threadId":"63365","inReplyTo":"20250501131751.GA1725607@coredump.intra.peff.net","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-05-01T14:36:13Z","receivedAt":"2025-05-01T14:36:31Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Peff and Kristoffer\n\nOn 01/05/2025 14:17, Jeff King wrote:\n> On Wed, Apr 30, 2025 at 05:17:38PM +0200, Kristoffer Haugsbakk wrote:\n> \n> Probably the smallest solution is for ctx->reflog_message to copy the\n> result and always own the memory (and then remember to free it, both at\n> cleanup and if it is ever overwritten).\n> \n> But I think the way reflog_message() returns the \"buf\" member of a\n> static strbuf is kind of an anti-pattern, exactly because you can get\n> this kind of subtle re-use. It probably should just return a non-const\n> pointer, handing over memory ownership to the caller. That would require\n> adjusting its other callers, too.\n\nGetting rid of the static buffer would certainly protect us from the \nuse-after-free. The bug here is that we're not calling reflog_message() \nand storing the result in ctx->reflog_message() to create the correct \nmessage in do_merge(). Looking at your patch, having to remember to copy \nthe string returned from reflog_message() is a bit of a pain. I wonder \nif we could change ctx->reflog_message to be an strbuf and update \nreflog_message() like so\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ad0ab75c8d4..59d80ddf0cc 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3941,11 +3941,10 @@ static const char \n*sequencer_reflog_action(struct replay_opts *opts)\n  }\n\n  __attribute__((format (printf, 3, 4)))\n-static const char *reflog_message(struct replay_opts *opts,\n+static void reflog_message(struct replay_opts *opts,\n  \tconst char *sub_action, const char *fmt, ...)\n  {\n  \tva_list ap;\n-\tstatic struct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf *buf = &opts->ctx->reflog_message;\n\n  \tva_start(ap, fmt);\n  \tstrbuf_reset(&buf);\n@@ -3957,8 +3956,6 @@ static const char *reflog_message(struct \nreplay_opts *opts,\n  \t\tstrbuf_vaddf(&buf, fmt, ap);\n  \t}\n  \tva_end(ap);\n-\n-\treturn buf.buf;\n  }\n\n  static struct commit *lookup_label(struct repository *r, const char \n*label,\n\nAll of the other callers should can then use use ctx->reflog_message.buf \nwhere they were using the return value of reflog_message() before. That \nwould protect us from the use-after-free.\n\nI'll try and put a proper patch together next week that removes the \nstatic buffer and starts calling reflog_message() when we're merging.\n\nBest Wishes\n\nPhillip\n\n\n> So the \"smallest\" version is perhaps something like this, totally\n> untested except for confirming that t3430 no longer complains:\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index b5c4043757..07aa3b3731 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -228,7 +228,7 @@ struct replay_ctx {\n>   \t * Stores the reflog message that will be used when creating a\n>   \t * commit. Points to a static buffer and should not be free()'d.\n>   \t */\n> -\tconst char *reflog_message;\n> +\tchar *reflog_message;\n>   \t/*\n>   \t * The number of completed fixup and squash commands in the\n>   \t * current chain.\n> @@ -411,6 +411,7 @@ static void replay_ctx_release(struct replay_ctx *ctx)\n>   {\n>   \tstrbuf_release(&ctx->current_fixups);\n>   \tstrbuf_release(&ctx->message);\n> +\tfree(ctx->reflog_message);\n>   }\n>   \n>   void replay_opts_release(struct replay_opts *opts)\n> @@ -3939,7 +3940,7 @@ static const char *reflog_message(struct replay_opts *opts,\n>   \tstatic struct strbuf buf = STRBUF_INIT;\n>   \n>   \tva_start(ap, fmt);\n> -\tstrbuf_reset(&buf);\n> +\tstrbuf_release(&buf); /* guarantees realloaction */\n>   \tstrbuf_addstr(&buf, sequencer_reflog_action(opts));\n>   \tif (sub_action)\n>   \t\tstrbuf_addf(&buf, \" (%s)\", sub_action);\n> @@ -4886,9 +4887,11 @@ static int pick_one_commit(struct repository *r,\n>   \tint res;\n>   \tstruct todo_item *item = todo_list->items + todo_list->current;\n>   \tconst char *arg = todo_item_get_arg(todo_list, item);\n> -\tif (is_rebase_i(opts))\n> -\t\tctx->reflog_message = reflog_message(\n> -\t\t\topts, command_to_string(item->command), NULL);\n> +\tif (is_rebase_i(opts)) {\n> +\t\tfree(ctx->reflog_message);\n> +\t\tctx->reflog_message = xstrdup(reflog_message(\n> +\t\t\topts, command_to_string(item->command), NULL));\n> +\t}\n>   \n>   \tres = do_pick_commit(r, item, opts, is_final_fixup(todo_list),\n>   \t\t\t     check_todo);\n> @@ -4947,7 +4950,8 @@ static int pick_commits(struct repository *r,\n>   \tstruct replay_ctx *ctx = opts->ctx;\n>   \tint res = 0, reschedule = 0;\n>   \n> -\tctx->reflog_message = sequencer_reflog_action(opts);\n> +\tfree(ctx->reflog_message);\n> +\tctx->reflog_message = xstrdup(sequencer_reflog_action(opts));\n>   \tif (opts->allow_ff)\n>   \t\tASSERT(!(opts->signoff || opts->no_commit ||\n>   \t\t\t opts->record_origin || should_edit(opts) ||\n> @@ -5423,7 +5427,8 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)\n>   \t\t\tunlink(rebase_path_dropped());\n>   \t\t}\n>   \n> -\t\tctx->reflog_message = reflog_message(opts, \"continue\", NULL);\n> +\t\tfree(ctx->reflog_message);\n> +\t\tctx->reflog_message = xstrdup(reflog_message(opts, \"continue\", NULL));\n>   \t\tif (commit_staged_changes(r, opts, &todo_list)) {\n>   \t\t\tres = -1;\n>   \t\t\tgoto release_todo_list;\n> @@ -5475,7 +5480,8 @@ static int single_pick(struct repository *r,\n>   \t\t\tTODO_PICK : TODO_REVERT;\n>   \titem.commit = cmit;\n>   \n> -\topts->ctx->reflog_message = sequencer_reflog_action(opts);\n> +\tfree(opts->ctx->reflog_message);\n> +\topts->ctx->reflog_message = xstrdup(sequencer_reflog_action(opts));\n>   \treturn do_pick_commit(r, &item, opts, 0, &check_todo);\n>   }\n>   \n> \n> I'm hoping your or Phillip can decide on the best fix from here.\n> \n> -Peff\n\n"},{"id":"517047","messageId":"20250501153638.GA1793056@coredump.intra.peff.net","threadId":"63365","inReplyTo":"e4ea1e44-af7e-4c24-b0ee-f87091200b90@gmail.com","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-01T15:36:38Z","receivedAt":"2025-05-01T15:36:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 01, 2025 at 03:36:13PM +0100, phillip.wood123@gmail.com wrote:\n\n> > But I think the way reflog_message() returns the \"buf\" member of a\n> > static strbuf is kind of an anti-pattern, exactly because you can get\n> > this kind of subtle re-use. It probably should just return a non-const\n> > pointer, handing over memory ownership to the caller. That would require\n> > adjusting its other callers, too.\n> \n> Getting rid of the static buffer would certainly protect us from the\n> use-after-free. The bug here is that we're not calling reflog_message() and\n> storing the result in ctx->reflog_message() to create the correct message in\n> do_merge(). Looking at your patch, having to remember to copy the string\n> returned from reflog_message() is a bit of a pain. I wonder if we could\n> change ctx->reflog_message to be an strbuf and update reflog_message() like\n> so\n\nYeah, I agree that would solve the use-after-free. I just wasn't sure if\nthere would still be a logic bug, though. If we have a sequence like:\n\n  /* somebody sets the variable */\n  ctx->reflog_message = reflog_message(...);\n  ...\n  /* some caller, possibly far away, uses the function again */\n  foo(reflog_message(...));\n  ...\n  /* now the original caller wants to use what it stored */\n  printf(\"%s\", ctx->reflog_message);\n\nRight now that is a potential use-after-free because of reallocating the\nstatic storage inside reflog_message(). If we teach reflog_message\nto store things in ctx->reflog_message always, that use-after-free goes\naway, but will the printf() above print the wrong thing?\n\nI wasn't clear on who is setting the value or why. It could be that\ndoing so is actually _fixing_ a bug (because it should be printing what\ncomes from the invocation in the foo() call).\n\n> I'll try and put a proper patch together next week that removes the static\n> buffer and starts calling reflog_message() when we're merging.\n\nGreat, thanks.\n\n-Peff\n"},{"id":"517049","messageId":"xmqq4iy4z55h.fsf@gitster.g","threadId":"63365","inReplyTo":"20250501131751.GA1725607@coredump.intra.peff.net","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-01T16:07:06Z","receivedAt":"2025-05-01T16:07:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Probably the smallest solution is for ctx->reflog_message to copy the\n> result and always own the memory (and then remember to free it, both at\n> cleanup and if it is ever overwritten).\n>\n> But I think the way reflog_message() returns the \"buf\" member of a\n> static strbuf is kind of an anti-pattern, exactly because you can get\n> this kind of subtle re-use. It probably should just return a non-const\n> pointer, handing over memory ownership to the caller. That would require\n> adjusting its other callers, too.\n\nYeah, yesterday I was looking at the same report and was thinking\nthat the memory ownership model here is somewhat screwed up.  As\na few more allocations/deallocations should be dwarfed by the real\nprocessing cost of replaying each commit, I think this \"smallest\"\nsolution is also the solution in the right direction.\n\n> So the \"smallest\" version is perhaps something like this, totally\n> untested except for confirming that t3430 no longer complains:\n\n;-)\n\n>  \tva_start(ap, fmt);\n> -\tstrbuf_reset(&buf);\n> +\tstrbuf_release(&buf); /* guarantees realloaction */\n\nI initially thought that this comment may have to be updated in the\nproduction version, but because we have to freshly allocate for each\nnew message for ownership change, this comment still is correct.\nThe only difference between the \"here is how to expose\" and \"this is\npart of the smallest solution\" is why we want to guarantee it.\n\n> I'm hoping your or Phillip can decide on the best fix from here.\n\n;-)\n"},{"id":"517052","messageId":"20250501163822.GE1795346@coredump.intra.peff.net","threadId":"63365","inReplyTo":"xmqq4iy4z55h.fsf@gitster.g","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-01T16:38:22Z","receivedAt":"2025-05-01T16:38:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 01, 2025 at 09:07:06AM -0700, Junio C Hamano wrote:\n\n> >  \tva_start(ap, fmt);\n> > -\tstrbuf_reset(&buf);\n> > +\tstrbuf_release(&buf); /* guarantees realloaction */\n> \n> I initially thought that this comment may have to be updated in the\n> production version, but because we have to freshly allocate for each\n> new message for ownership change, this comment still is correct.\n> The only difference between the \"here is how to expose\" and \"this is\n> part of the smallest solution\" is why we want to guarantee it.\n\nThis code change is just to stimulate the bug more readily. ;)\n\nI think if we started to actually allocate here, we'd want to switch the\n\"return buf.buf\" at the end to strbuf_detach().\n\n-Peff\n"},{"id":"517054","messageId":"xmqqcycsxlnx.fsf@gitster.g","threadId":"63365","inReplyTo":"20250501163822.GE1795346@coredump.intra.peff.net","subject":"Re: [BUG] rebase: can write reflog with uninit. `action` string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-01T17:53:22Z","receivedAt":"2025-05-01T17:53:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, May 01, 2025 at 09:07:06AM -0700, Junio C Hamano wrote:\n>\n>> >  \tva_start(ap, fmt);\n>> > -\tstrbuf_reset(&buf);\n>> > +\tstrbuf_release(&buf); /* guarantees realloaction */\n>> \n>> I initially thought that this comment may have to be updated in the\n>> production version, but because we have to freshly allocate for each\n>> new message for ownership change, this comment still is correct.\n>> The only difference between the \"here is how to expose\" and \"this is\n>> part of the smallest solution\" is why we want to guarantee it.\n>\n> This code change is just to stimulate the bug more readily. ;)\n>\n> I think if we started to actually allocate here, we'd want to switch the\n> \"return buf.buf\" at the end to strbuf_detach().\n\nThat's sensible.\n"}]}