{"thread":{"id":"61513","subject":"bug: Segfault with git diff","startedAt":"2024-05-19T03:02:35Z","lastAt":"2025-05-01T13:23:58Z","messageCount":7,"participants":["jake roggenbuck","Jake Roggenbuck","Eric Sunshine","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"495052","messageId":"CAEUC8gmgq_yViedLGHOeSyvR9rQK+O-8Fh9wzds=2+326ngUjw@mail.gmail.com","threadId":"61513","inReplyTo":null,"subject":"bug: Segfault with git diff","fromName":"jake roggenbuck","fromEmail":"roggenbuckjake@gmail.com","sentAt":"2024-05-19T03:02:19Z","receivedAt":"2024-05-19T03:02:35Z","isPatch":false,"sender":{"key":"roggenbuckjake@gmail.com","avatar":null},"body":"> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n\nI have found a segfault when running git diff.\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n\n1. After running `git diff`, git showed me that object files were empty.\n2. I deleted the empty object files, and ran `git diff` again.\n3. I continued deleting the empty files until `git diff` segfaulted.\n\n> What did you expect to happen? (Expected behavior)\nAn error message of some type or a graceful exit.\n\n> What happened instead? (Actual behavior)\nA segmentation fault.\n\n`Segmentation fault (core dumped)`\n\n> What's different between what you expected and what actually happened?\nInstead of closing gracefully, there was a segmentation fault.\n\n> Anything else you want to add:\ngit log displays:\nfatal: bad object HEAD\n\ngit branch displays:\nfatal: missing object 7610511b1b4db888e8e6bb8d0ff158f932961345 for\nrefs/heads/main\n\nNeither log nor branch causes the segfault.\n\n[System Info]\ngit version:\ngit version 2.45.1\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 6.7.3-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 01 Feb 2024\n10:30:35 +0000 x86_64\ncompiler info: gnuc: 14.1\nlibc info: glibc: 2.39\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\n\nBest,\nJake Roggenbuck\n"},{"id":"510142","messageId":"20250108060151.7218-1-jakeroggenbuck2@gmail.com","threadId":"61513","inReplyTo":"CAEUC8gmgq_yViedLGHOeSyvR9rQK+O-8Fh9wzds=2+326ngUjw@mail.gmail.com","subject":"[PATCH 0/1] Exit on invalid diff status of diff_filepair","fromName":"Jake Roggenbuck","fromEmail":"jakeroggenbuck2@gmail.com","sentAt":"2025-01-08T06:01:50Z","receivedAt":"2025-01-08T06:02:24Z","isPatch":true,"sender":{"key":"jakeroggenbuck2@gmail.com","avatar":"https://gravatar.com/avatar/679d8789156770028bd93be4364916140ea973126151827a53ce248a236f5e52?d=mp&s=160"},"body":"Hi all,\n\nGit showed that object files were empty, so I manually removed them.\nAfter removing a few object files, I ran `git diff` and I got a segfault.\n\nI have since narrowed down the source of the error to a usage of\n`diff_filepair`. I also noticed that the `status` field of `diff_filepair`\ndoes not get set and stays a zero value. My fix is to check for that invalid\nstatus character and exit gracefully with an error message.\n\nI modeled the behavior of this patch after what `git log` does in the same\nsituation. `git log` will exit with a message that reads `bad object HEAD`.\n\nI initially found this segfault when using git while working on a school\nassignment. I am really interested in helping fix this segfault.\nI'd be happy to hear any feedback.\n\nThank you!\n\n\nJake Roggenbuck (1):\n  Exit on invalid diff status of diff_filepair\n\n diff.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\n-- \n2.47.0\n\n"},{"id":"510143","messageId":"20250108060151.7218-2-jakeroggenbuck2@gmail.com","threadId":"61513","inReplyTo":"20250108060151.7218-1-jakeroggenbuck2@gmail.com","subject":"[PATCH 1/1] Exit on invalid diff status of diff_filepair","fromName":"Jake Roggenbuck","fromEmail":"jakeroggenbuck2@gmail.com","sentAt":"2025-01-08T06:01:51Z","receivedAt":"2025-01-08T06:02:29Z","isPatch":true,"sender":{"key":"jakeroggenbuck2@gmail.com","avatar":"https://gravatar.com/avatar/679d8789156770028bd93be4364916140ea973126151827a53ce248a236f5e52?d=mp&s=160"},"body":"Add a check for the invalid status of `0` for `diff_filepair` when certain\nobject files are missing. When these object files are missing, 'git log'\nreturns 'fatal: bad object HEAD' but 'git diff' segfaults.\n\nNormally, the `diff_filepair` status should be a character, but when object\nfiles are removed, status becomes a zero character which isn't listed as one\nof the possible status letters in `Documentation/diff-format.txt`.\n\nThis patch checks for that invalid status character and gracefully exits with\nan error message.\n\nSigned-off-by: Jake Roggenbuck <jakeroggenbuck2@gmail.com>\n---\n diff.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 6c96154fed..aeab1ac445 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -7018,6 +7018,10 @@ void diff_queued_diff_prefetch(void *repository)\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n+\n+\t\tif (!p->status)\n+\t\t\tdie(\"invalid diff status\");\n+\n \t\tdiff_add_if_missing(repo, &to_fetch, p->one);\n \t\tdiff_add_if_missing(repo, &to_fetch, p->two);\n \t}\n-- \n2.47.0\n\n"},{"id":"517030","messageId":"20250430185309.11197-1-jakeroggenbuck2@gmail.com","threadId":"61513","inReplyTo":"20250108060151.7218-2-jakeroggenbuck2@gmail.com","subject":"Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair","fromName":"Jake Roggenbuck","fromEmail":"jakeroggenbuck2@gmail.com","sentAt":"2025-04-30T18:50:37Z","receivedAt":"2025-04-30T18:53:29Z","isPatch":true,"sender":{"key":"jakeroggenbuck2@gmail.com","avatar":"https://gravatar.com/avatar/679d8789156770028bd93be4364916140ea973126151827a53ce248a236f5e52?d=mp&s=160"},"body":"Hi all,\n\nHas anyone gotten a chance to take a look at this simple fix?\nWhen git is built of the main branch, it still segfaults when there is an invalid diff_filepair.\n\n    git (main) $ make\n    <make output omitted>\n    git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/\n    hw3-skeleton-broken (main) $ ~/Build/git/git diff\n    Segmentation fault (core dumped)\n    hw3-skeleton-broken (main) $\n\nLet me know if you have any feedback or suggestions.\n\nSigned-off-by: Jake Roggenbuck <jakeroggenbuck2@gmail.com>\n\n"},{"id":"517031","messageId":"20250430185309.11197-2-jakeroggenbuck2@gmail.com","threadId":"61513","inReplyTo":"20250430185309.11197-1-jakeroggenbuck2@gmail.com","subject":"Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair","fromName":"Jake Roggenbuck","fromEmail":"jakeroggenbuck2@gmail.com","sentAt":"2025-04-30T18:50:38Z","receivedAt":"2025-04-30T18:53:30Z","isPatch":true,"sender":{"key":"jakeroggenbuck2@gmail.com","avatar":"https://gravatar.com/avatar/679d8789156770028bd93be4364916140ea973126151827a53ce248a236f5e52?d=mp&s=160"},"body":""},{"id":"517039","messageId":"CAPig+cRehhM-z0md_iV-VYCk_Qwv0A4zS1TfPqPxsrLZ3eYxvA@mail.gmail.com","threadId":"61513","inReplyTo":"20250430185309.11197-1-jakeroggenbuck2@gmail.com","subject":"Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-05-01T06:16:36Z","receivedAt":"2025-05-01T06:16:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 30, 2025 at 2:53 PM Jake Roggenbuck\n<jakeroggenbuck2@gmail.com> wrote:\n> Has anyone gotten a chance to take a look at this simple fix?\n> When git is built of the main branch, it still segfaults when there is an invalid diff_filepair.\n>\n>     git (main) $ make\n>     <make output omitted>\n>     git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/\n>     hw3-skeleton-broken (main) $ ~/Build/git/git diff\n>     Segmentation fault (core dumped)\n>     hw3-skeleton-broken (main) $\n>\n> Let me know if you have any feedback or suggestions.\n\nDo you have a reproduction recipe which demonstrates the problem which\nyour patch fixes? Including the recipe in the patch's commit message\nwould help reviewers better understand the circumstances under which\nthe crash occurs since the descriptions provided by both the original\nproblem report[1] and the submitted patch[2] seem rather nebulous[3].\n\nMore importantly, if you have a reproduction recipe, then it can be\nused as the basis for creating a test which should accompany the patch\n(and which should be added to one of the `t/t40xx-*.sh` files). We can\nhelp you convert the reproduction recipe into a test if desired.\n\nFOOTNOTES\n\n[1]: https://lore.kernel.org/git/CAEUC8gmgq_yViedLGHOeSyvR9rQK+O-8Fh9wzds=2+326ngUjw@mail.gmail.com/\n\n[2]: https://lore.kernel.org/git/20250108060151.7218-2-jakeroggenbuck2@gmail.com/\n\n[3]: It is not clear, for instance, what an \"object file\" is in the\nprovided description. Are you referring to one of the files under\n`.git/objects/`?\n"},{"id":"517044","messageId":"xmqq8qngzcpg.fsf@gitster.g","threadId":"61513","inReplyTo":"CAPig+cRehhM-z0md_iV-VYCk_Qwv0A4zS1TfPqPxsrLZ3eYxvA@mail.gmail.com","subject":"Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-01T13:23:55Z","receivedAt":"2025-05-01T13:23:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>>     git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/\n>>     hw3-skeleton-broken (main) $ ~/Build/git/git diff\n>>     Segmentation fault (core dumped)\n>>     hw3-skeleton-broken (main) $\n>>\n>> Let me know if you have any feedback or suggestions.\n>\n> Do you have a reproduction recipe which demonstrates the problem which\n> your patch fixes? Including the recipe in the patch's commit message\n> would help reviewers better understand the circumstances under which\n> the crash occurs since the descriptions provided by both the original\n> problem report[1] and the submitted patch[2] seem rather nebulous[3].\n>\n> More importantly, if you have a reproduction recipe, then it can be\n> used as the basis for creating a test which should accompany the patch\n> (and which should be added to one of the `t/t40xx-*.sh` files). We can\n> help you convert the reproduction recipe into a test if desired.\n\nNicely put.\n\nIt is a bug _elsewhere_ in the code for the .status member to be\nunassigned when the control reaches that point, so a patch to exit\nafter that happens is not all that interesting.  Instead of exiting\nthere, we would want to see a patch against the place where it\nshould have set the .status member but fails to do so.\n\nMaybe with a corrupted repository, some of the blob objects we read\nfor comparison may fail to load and the function may be taking an\nearly return instead of complaining and dying upon unreadable blob\n(I am not saying that is the only or even a likely case; just giving\nan example situation for illustrate the point), in which case we\nwould want to fix _that_ code to complain and die properly.\n\nThanks.\n"}]}