{"thread":{"id":"52900","subject":"git-core: try_to_follow_renames(): git killed by SIGSEGV","startedAt":"2020-02-27T12:56:33Z","lastAt":"2020-03-10T17:35:18Z","messageCount":6,"participants":["Ondrej Pohorelsky","Alexandr Miloslavskiy","SZEDER Gábor","Konstantin Tokarev"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"392607","messageId":"CA+B51BFFvn9puia8+kheeWkDfOQ7RYHTcGa74M5aeiTd8-QJXA@mail.gmail.com","threadId":"52900","inReplyTo":null,"subject":"git-core: try_to_follow_renames(): git killed by SIGSEGV","fromName":"Ondrej Pohorelsky","fromEmail":"opohorel@redhat.com","sentAt":"2020-02-27T12:56:13Z","receivedAt":"2020-02-27T12:56:33Z","isPatch":false,"sender":{"key":"opohorel@redhat.com","avatar":"https://avatars.githubusercontent.com/u/35430604?v=4"},"body":"Hi,\n\nthere is a SIGSEGV appearing in Fedora[0] with Git 2.24.1\n\nThis bug started to appear after update to Git 2.24.1.\nBug reporter said that Git crashed on him while running VS Code with\nGit Lens extension[1]\nI have tried to reproduce this bug with my own compiled Git with debug\nflags, but sadly SIGSEGV never appeared.\n\nTo me it seems like there is a problem in commit a2bb801f6a[2] which\nchanges move_diff_queue() function. This function calls\ndiff_tree_oid() that calls try_to_follow_renames(). In the last two\nfunctions there are no arguments checks.\n\nBest regards,\nOndřej Pohořelský\n\n[0] https://retrace.fedoraproject.org/faf/problems/bthash/?bth=25aa7d7267ab5de548ffca337115cb68f7b65105\n[1] https://bugzilla.redhat.com/show_bug.cgi?id=1791810\n[2] https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2bb801f6a430f6049e5c9729a8f3bf9097d9b34\n\n"},{"id":"392903","messageId":"3c722d21-ee57-7d20-81fb-0399f02f1bc7@syntevo.com","threadId":"52900","inReplyTo":"CA+B51BFFvn9puia8+kheeWkDfOQ7RYHTcGa74M5aeiTd8-QJXA@mail.gmail.com","subject":"Re: git-core: try_to_follow_renames(): git killed by SIGSEGV","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-03-06T14:44:34Z","receivedAt":"2020-03-06T14:58:07Z","isPatch":false,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Since I like studying crashes and noone else replied, I decided to have \na look.\n\nThe problem is easy to reproduce with this (replace 1.c with any file):\n   git log --follow -L 1,1:1.c -- 1.c\n\nIt occurs because `opt->pathspec.items` gets cleaned here:\n     clear_pathspec\n     queue_diffs\n         /* must look at the full tree diff to detect renames */\n         clear_pathspec(&opt->pathspec);\n         DIFF_QUEUE_CLEAR(&diff_queued_diff);\n     process_ranges_ordinary_commit\n     process_ranges_arbitrary_commit\n     line_log_filter\n     prepare_revision_walk\n     cmd_log_walk\n     cmd_log\n\nAnd on next iteration it crashes in 'try_to_follow_renames' on this line:\n     diff_opts.single_follow = opt->pathspec.items[0].match;\n\nI think that bug comes from commit:\n     a2bb801f by SZEDER Gábor, 2019-08-21 13:04:24\n     line-log: avoid unnecessary full tree diffs\n\n@szeder could you please look into that?\n\nOn 27.02.2020 13:56, Ondrej Pohorelsky wrote:\n> Hi,\n> \n> there is a SIGSEGV appearing in Fedora[0] with Git 2.24.1\n> \n> This bug started to appear after update to Git 2.24.1.\n> Bug reporter said that Git crashed on him while running VS Code with\n> Git Lens extension[1]\n> I have tried to reproduce this bug with my own compiled Git with debug\n> flags, but sadly SIGSEGV never appeared.\n> \n> To me it seems like there is a problem in commit a2bb801f6a[2] which\n> changes move_diff_queue() function. This function calls\n> diff_tree_oid() that calls try_to_follow_renames(). In the last two\n> functions there are no arguments checks.\n> \n> Best regards,\n> Ondřej Pohořelský\n> \n> [0] https://retrace.fedoraproject.org/faf/problems/bthash/?bth=25aa7d7267ab5de548ffca337115cb68f7b65105\n> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1791810\n> [2] https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2bb801f6a430f6049e5c9729a8f3bf9097d9b34\n> \n\n"},{"id":"392987","messageId":"CA+B51BErxp0Nzcc=1Ress4tbDmKYmcCqUyC-fmD+8O8KdaZLZQ@mail.gmail.com","threadId":"52900","inReplyTo":"3c722d21-ee57-7d20-81fb-0399f02f1bc7@syntevo.com","subject":"Re: git-core: try_to_follow_renames(): git killed by SIGSEGV","fromName":"Ondrej Pohorelsky","fromEmail":"opohorel@redhat.com","sentAt":"2020-03-10T12:07:55Z","receivedAt":"2020-03-10T12:08:16Z","isPatch":false,"sender":{"key":"opohorel@redhat.com","avatar":"https://avatars.githubusercontent.com/u/35430604?v=4"},"body":"Thank you for your further analyzation and explanation.\n\nI would love to make a patch for this bug, but sadly\nI'm not fully aware of what is going on in this functions\nand how they are affecting other git functionality.\n\nI hope @szeder can look into this bug and provide more explanation\nas he knows this code best.\n\nBest regards,\nOndřej Pohořelský\n\nOn Fri, Mar 6, 2020 at 3:58 PM Alexandr Miloslavskiy\n<alexandr.miloslavskiy@syntevo.com> wrote:\n>\n> Since I like studying crashes and noone else replied, I decided to have\n> a look.\n>\n> The problem is easy to reproduce with this (replace 1.c with any file):\n>    git log --follow -L 1,1:1.c -- 1.c\n>\n> It occurs because `opt->pathspec.items` gets cleaned here:\n>      clear_pathspec\n>      queue_diffs\n>          /* must look at the full tree diff to detect renames */\n>          clear_pathspec(&opt->pathspec);\n>          DIFF_QUEUE_CLEAR(&diff_queued_diff);\n>      process_ranges_ordinary_commit\n>      process_ranges_arbitrary_commit\n>      line_log_filter\n>      prepare_revision_walk\n>      cmd_log_walk\n>      cmd_log\n>\n> And on next iteration it crashes in 'try_to_follow_renames' on this line:\n>      diff_opts.single_follow = opt->pathspec.items[0].match;\n>\n> I think that bug comes from commit:\n>      a2bb801f by SZEDER Gábor, 2019-08-21 13:04:24\n>      line-log: avoid unnecessary full tree diffs\n>\n> @szeder could you please look into that?\n>\n> On 27.02.2020 13:56, Ondrej Pohorelsky wrote:\n> > Hi,\n> >\n> > there is a SIGSEGV appearing in Fedora[0] with Git 2.24.1\n> >\n> > This bug started to appear after update to Git 2.24.1.\n> > Bug reporter said that Git crashed on him while running VS Code with\n> > Git Lens extension[1]\n> > I have tried to reproduce this bug with my own compiled Git with debug\n> > flags, but sadly SIGSEGV never appeared.\n> >\n> > To me it seems like there is a problem in commit a2bb801f6a[2] which\n> > changes move_diff_queue() function. This function calls\n> > diff_tree_oid() that calls try_to_follow_renames(). In the last two\n> > functions there are no arguments checks.\n> >\n> > Best regards,\n> > Ondřej Pohořelský\n> >\n> > [0] https://retrace.fedoraproject.org/faf/problems/bthash/?bth=25aa7d7267ab5de548ffca337115cb68f7b65105\n> > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1791810\n> > [2] https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2bb801f6a430f6049e5c9729a8f3bf9097d9b34\n> >\n>\n\n"},{"id":"393005","messageId":"20200310165723.GB3122@szeder.dev","threadId":"52900","inReplyTo":"3c722d21-ee57-7d20-81fb-0399f02f1bc7@syntevo.com","subject":"Re: git-core: try_to_follow_renames(): git killed by SIGSEGV","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-03-10T16:57:23Z","receivedAt":"2020-03-10T16:57:31Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Mar 06, 2020 at 03:44:34PM +0100, Alexandr Miloslavskiy wrote:\n> Since I like studying crashes and noone else replied, I decided to have a\n> look.\n> \n> The problem is easy to reproduce with this (replace 1.c with any file):\n>   git log --follow -L 1,1:1.c -- 1.c\n\nDon't do this.  In particular:\n\n  - Don't use line-level log with a pathspec, because the\n    documentation of 'git log -L' explicitly told you not to do so\n    (\"You may not give any pathspec limiters.\").  This should have\n    errored out since the beginning, but, unfortunately, has never\n    been enforced.\n\n  - Don't use '-L' with '--follow'.  On one hand, line-level log on\n    its own already follows file renames, even multiple files at once,\n    there is no need for an additional '--follow' (which can only\n    follow one file).  OTOH, you shouldn't be able to use '-L' and\n    '--follow' together, because the former forbids a pathspec, while\n    the latter requires one.\n\nIn any case, '--follow' has always been an ugly hack on top of the\nrevision walking machinery, while line-level log is a rather poorly\nintegrated bolt-on.  They simply weren't designed to work together, as\nevidenced by their contradicting requirements about the pathspec.\n\n> It occurs because `opt->pathspec.items` gets cleaned here:\n>     clear_pathspec\n>     queue_diffs\n>         /* must look at the full tree diff to detect renames */\n>         clear_pathspec(&opt->pathspec);\n>         DIFF_QUEUE_CLEAR(&diff_queued_diff);\n>     process_ranges_ordinary_commit\n>     process_ranges_arbitrary_commit\n>     line_log_filter\n>     prepare_revision_walk\n>     cmd_log_walk\n>     cmd_log\n> \n> And on next iteration it crashes in 'try_to_follow_renames' on this line:\n>     diff_opts.single_follow = opt->pathspec.items[0].match;\n> \n> I think that bug comes from commit:\n>     a2bb801f by SZEDER Gábor, 2019-08-21 13:04:24\n>     line-log: avoid unnecessary full tree diffs\n> \n> @szeder could you please look into that?\n> \n> On 27.02.2020 13:56, Ondrej Pohorelsky wrote:\n> >Hi,\n> >\n> >there is a SIGSEGV appearing in Fedora[0] with Git 2.24.1\n> >\n> >This bug started to appear after update to Git 2.24.1.\n> >Bug reporter said that Git crashed on him while running VS Code with\n> >Git Lens extension[1]\n> >I have tried to reproduce this bug with my own compiled Git with debug\n> >flags, but sadly SIGSEGV never appeared.\n> >\n> >To me it seems like there is a problem in commit a2bb801f6a[2] which\n> >changes move_diff_queue() function. This function calls\n> >diff_tree_oid() that calls try_to_follow_renames(). In the last two\n> >functions there are no arguments checks.\n> >\n> >Best regards,\n> >Ondřej Pohořelský\n> >\n> >[0] https://retrace.fedoraproject.org/faf/problems/bthash/?bth=25aa7d7267ab5de548ffca337115cb68f7b65105\n> >[1] https://bugzilla.redhat.com/show_bug.cgi?id=1791810\n> >[2] https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2bb801f6a430f6049e5c9729a8f3bf9097d9b34\n> >\n> \n"},{"id":"393008","messageId":"347951583861239@vla5-dcf36e533bf7.qloud-c.yandex.net","threadId":"52900","inReplyTo":"20200310165723.GB3122@szeder.dev","subject":"Re: git-core: try_to_follow_renames(): git killed by SIGSEGV","fromName":"Konstantin Tokarev","fromEmail":"annulen@yandex.ru","sentAt":"2020-03-10T17:30:05Z","receivedAt":"2020-03-10T17:30:12Z","isPatch":false,"sender":{"key":"annulen@yandex.ru","avatar":null},"body":"\n\n10.03.2020, 19:57, \"SZEDER Gábor\" <szeder.dev@gmail.com>:\n> On Fri, Mar 06, 2020 at 03:44:34PM +0100, Alexandr Miloslavskiy wrote:\n>>  Since I like studying crashes and noone else replied, I decided to have a\n>>  look.\n>>\n>>  The problem is easy to reproduce with this (replace 1.c with any file):\n>>    git log --follow -L 1,1:1.c -- 1.c\n>\n> Don't do this. In particular:\n>\n>   - Don't use line-level log with a pathspec, because the\n>     documentation of 'git log -L' explicitly told you not to do so\n>     (\"You may not give any pathspec limiters.\"). This should have\n>     errored out since the beginning, but, unfortunately, has never\n>     been enforced.\n>\n>   - Don't use '-L' with '--follow'. On one hand, line-level log on\n>     its own already follows file renames, even multiple files at once,\n>     there is no need for an additional '--follow' (which can only\n>     follow one file). OTOH, you shouldn't be able to use '-L' and\n>     '--follow' together, because the former forbids a pathspec, while\n>     the latter requires one.\n>\n> In any case, '--follow' has always been an ugly hack on top of the\n> revision walking machinery, while line-level log is a rather poorly\n> integrated bolt-on. They simply weren't designed to work together, as\n> evidenced by their contradicting requirements about the pathspec.\n\nThis kind of explains issue with --follow and --full-diff which I've reported\nrecently and got completely ignored.\n\nAre there any plans to integrate --follow better with other tools?\n\n\n>\n>>  It occurs because `opt->pathspec.items` gets cleaned here:\n>>      clear_pathspec\n>>      queue_diffs\n>>          /* must look at the full tree diff to detect renames */\n>>          clear_pathspec(&opt->pathspec);\n>>          DIFF_QUEUE_CLEAR(&diff_queued_diff);\n>>      process_ranges_ordinary_commit\n>>      process_ranges_arbitrary_commit\n>>      line_log_filter\n>>      prepare_revision_walk\n>>      cmd_log_walk\n>>      cmd_log\n>>\n>>  And on next iteration it crashes in 'try_to_follow_renames' on this line:\n>>      diff_opts.single_follow = opt->pathspec.items[0].match;\n>>\n>>  I think that bug comes from commit:\n>>      a2bb801f by SZEDER Gábor, 2019-08-21 13:04:24\n>>      line-log: avoid unnecessary full tree diffs\n>>\n>>  @szeder could you please look into that?\n>>\n>>  On 27.02.2020 13:56, Ondrej Pohorelsky wrote:\n>>  >Hi,\n>>  >\n>>  >there is a SIGSEGV appearing in Fedora[0] with Git 2.24.1\n>>  >\n>>  >This bug started to appear after update to Git 2.24.1.\n>>  >Bug reporter said that Git crashed on him while running VS Code with\n>>  >Git Lens extension[1]\n>>  >I have tried to reproduce this bug with my own compiled Git with debug\n>>  >flags, but sadly SIGSEGV never appeared.\n>>  >\n>>  >To me it seems like there is a problem in commit a2bb801f6a[2] which\n>>  >changes move_diff_queue() function. This function calls\n>>  >diff_tree_oid() that calls try_to_follow_renames(). In the last two\n>>  >functions there are no arguments checks.\n>>  >\n>>  >Best regards,\n>>  >Ondřej Pohořelský\n>>  >\n>>  >[0] https://retrace.fedoraproject.org/faf/problems/bthash/?bth=25aa7d7267ab5de548ffca337115cb68f7b65105\n>>  >[1] https://bugzilla.redhat.com/show_bug.cgi?id=1791810\n>>  >[2] https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2bb801f6a430f6049e5c9729a8f3bf9097d9b34\n>>  >\n\n-- \nRegards,\nKonstantin\n\n"},{"id":"393009","messageId":"28747ec8-0049-b459-0100-420252ca0225@syntevo.com","threadId":"52900","inReplyTo":"20200310165723.GB3122@szeder.dev","subject":"Re: git-core: try_to_follow_renames(): git killed by SIGSEGV","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-03-10T17:35:14Z","receivedAt":"2020-03-10T17:35:18Z","isPatch":false,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 10.03.2020 17:57, SZEDER Gábor wrote:\n> Don't do this.  In particular:\n\nTo clarify, I don't do this, I merely investigated a crash report from \nsomeone else. The reporter (Ondrej) seems to be a crash db maintainer, \nso he also doesn't do this. Finally, the user behind the report also \ndoesn't do this, as he merely uses VS Code. Therefore, \"don't do\" part \nis in fact addressed at VS Code.\n\n > This should have errored out since the beginning\n\nTo me it sounds like a perfect plan to avoid the crash.\n"}]}