[BUG] seemingly-rare segfault in merge-tree

[BUG] seemingly-rare segfault in merge-tree

[Taylor Blau]

I was looking through alerts this morning and found this one that
happened in GitHub's infrastructure a few days ago. Usually these are
memory corruption issues (and get dealt with via a separate process),
but this was a legitimate SEGV in the merge-tree builtin.

The backtrace is as follows:

  (gdb) bt
  #0  oid_to_hex (oid=0x4) at hex.c:164
  #1  0x000056234188aba3 in real_merge (prefix=0x0, branch2=<optimized out>, branch1=<optimized out>, o=0x7ffd3fc4cd60) at builtin/merge-tree.c:528
  #2  cmd_merge_tree (argc=<optimized out>, argv=0x7ffd3fc4d7c0, prefix=0x0) at builtin/merge-tree.c:658
  #3  0x000056234181ffe9 in run_builtin (argv=0x7ffd3fc4d7c0, argc=8, p=0x562341d5c788 <commands+1896>) at git.c:465
  #4  handle_builtin (argc=8, argv=0x7ffd3fc4d7c0) at git.c:724
  #5  0x000056234182117f in run_argv (argv=0x7ffd3fc4d4f0, argcp=0x7ffd3fc4d4fc) at git.c:792
  #6  cmd_main (argc=<optimized out>, argv=<optimized out>) at git.c:923
  #7  0x000056234181fb80 in main (argc=11, argv=0x7ffd3fc4d7a8) at common-main.c:59

and it looks like the result's tree object is left NULL, which we then
produce a SEGV on when trying to pass `&NULL->object.oid` to
`oid_to_hex()`:

  (gdb) up
  #1  0x000056234188aba3 in real_merge (prefix=0x0, branch2=<optimized out>,
      branch1=<optimized out>, o=0x7ffd3fc4cd60) at builtin/merge-tree.c:528
  528	builtin/merge-tree.c: No such file or directory.
  (gdb) p result->tree
  $1 = (struct tree *) 0x0

I haven't looked too hard at these paths (nor do I have a useful
reproduction, since the top-level `argv` array is optimized out). But I
figured I'd share it here in case this was obvious to either one of you.

Thanks in advance for taking a look, and let me know if there's any
information you need from me in order to debug it further.

Thanks,
Taylor

Re: [BUG] seemingly-rare segfault in merge-tree

[Taylor Blau]

On Tue, Sep 20, 2022 at 04:57:36PM -0400, Taylor Blau wrote:
>   #1  0x000056234188aba3 in real_merge (prefix=0x0, branch2=<optimized out>,
>       branch1=<optimized out>, o=0x7ffd3fc4cd60) at builtin/merge-tree.c:528
>   528	builtin/merge-tree.c: No such file or directory.

I should mention, of course, that this line is custom to GitHub's fork,
and so doesn't match with the latest on `master`. Here's a link to the
relevant call that matches up:

  https://github.com/git/git/blob/dda7228a83e2e9ff584bf6adbf55910565b41e14/builtin/merge-tree.c#L449

Thanks,
Taylor

Re: [BUG] seemingly-rare segfault in merge-tree

[Johannes Schindelin]

Hi Taylor,

On Tue, 20 Sep 2022, Taylor Blau wrote:

> On Tue, Sep 20, 2022 at 04:57:36PM -0400, Taylor Blau wrote:
> >   #1  0x000056234188aba3 in real_merge (prefix=0x0, branch2=<optimized out>,
> >       branch1=<optimized out>, o=0x7ffd3fc4cd60) at builtin/merge-tree.c:528
> >   528	builtin/merge-tree.c: No such file or directory.
>
> I should mention, of course, that this line is custom to GitHub's fork,
> and so doesn't match with the latest on `master`. Here's a link to the
> relevant call that matches up:
>
>   https://github.com/git/git/blob/dda7228a83e2e9ff584bf6adbf55910565b41e14/builtin/merge-tree.c#L449

I am afraid that there is way more custom code from GitHub's internal fork
involved in this report than meets the eye. The `result.tree` that is most
likely the cause of the segmentation fault does not actually stem from
`merge_incore_recursive()` as the linked source code might suggest.

Instead, it stems from the `merge_incore_nonrecursive()` call in the
custom code that allows to specify a merge base manually (rather than let
`git merge-tree` perform a full recursive merge). Custom code, I need to
add, that is nowhere to be seen in upstream Git.

I did hint at this custom code during the Git Contributor Summit, and
there seemed to be enough interest that I had appended an item to my
ever-growing todo list to upstream this.

Ciao,
Dscho

Re: [BUG] seemingly-rare segfault in merge-tree

[Taylor Blau]

On Tue, Sep 20, 2022 at 11:24:00PM +0200, Johannes Schindelin wrote:
> I am afraid that there is way more custom code from GitHub's internal fork
> involved in this report than meets the eye. The `result.tree` that is most
> likely the cause of the segmentation fault does not actually stem from
> `merge_incore_recursive()` as the linked source code might suggest.
>
> Instead, it stems from the `merge_incore_nonrecursive()` call in the
> custom code that allows to specify a merge base manually (rather than let
> `git merge-tree` perform a full recursive merge). Custom code, I need to
> add, that is nowhere to be seen in upstream Git.

Ah, thanks. I didn't recall off-hand the extent of the custom code
GitHub is carrying to support merge-tree. If this has nothing to do with
any code in the open-source project, then please feel free to drop the
list from any more emails on this thread.

> I did hint at this custom code during the Git Contributor Summit, and
> there seemed to be enough interest that I had appended an item to my
> ever-growing todo list to upstream this.

Yes, that would be great.

Thanks,
Taylor