[BUG] seemingly-rare segfault in merge-tree

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

On Tue, Sep 20, 2022 at 04:57:36PM -0400, Taylor Blau wrote:

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

Hi Taylor,

On Tue, 20 Sep 2022, Taylor Blau 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.

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

On Tue, Sep 20, 2022 at 11:24:00PM +0200, Johannes Schindelin wrote:

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.

Yes, that would be great.

Thanks, Taylor