Re: [BUG] v2.16.0-rc0 seg faults when git bisect skip

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Jan 03 2018, Yasushi SHOJI jotted:

> Hi,
>
> git version 2.16.0.rc0 seg faults on my machine when I
> [...]
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  0x000055a73107f900 in best_bisection_sorted (list=0x0, nr=0) at bisect.c:232
> 232 free_commit_list(p->next);
> (gdb) bt
> #0  0x000055a73107f900 in best_bisection_sorted (list=0x0, nr=0) at bisect.c:232
> #1  0x000055a73107fc0f in do_find_bisection (list=0x0, nr=0,
> weights=0x55a731b6ffd0, find_all=1) at bisect.c:361
> #2  0x000055a73107fcf4 in find_bisection (commit_list=0x7ffe8750d4d0,
> reaches=0x7ffe8750d4c4, all=0x7ffe8750d4c0, find_all=1) at
> bisect.c:400
> #3  0x000055a73108128d in bisect_next_all (prefix=0x0, no_checkout=0)
> at bisect.c:969
> #4  0x000055a730fd5238 in cmd_bisect__helper (argc=0,
> argv=0x7ffe8750e230, prefix=0x0) at builtin/bisect--helper.c:140
> #5  0x000055a730fcbc76 in run_builtin (p=0x55a73145c778
> <commands+120>, argc=2, argv=0x7ffe8750e230) at git.c:346
> #6  0x000055a730fcbf40 in handle_builtin (argc=2, argv=0x7ffe8750e230)
> at git.c:554
> #7  0x000055a730fcc0e8 in run_argv (argcp=0x7ffe8750e0ec,
> argv=0x7ffe8750e0e0) at git.c:606
> #8  0x000055a730fcc29b in cmd_main (argc=2, argv=0x7ffe8750e230) at git.c:683
> #9  0x000055a731068d9e in main (argc=3, argv=0x7ffe8750e228) at common-main.c:43
> (gdb) p p
> $1 = (struct commit_list *) 0x0
>
> As you can see, the code dereferences to the 'next' while 'p' is NULL.
>
> I'm sure I did 'git bisect good' after git _found_ bad commit.  Then I
> typed 'git bisect skip' on the commit 726804874 of guile repository.
> If that matters at all.
>
> I haven't touched guile repo to preserve the current state.

I can't reproduce this myself, but looking at the backtrace it seems
pretty obvious that 7c117184d7 ("bisect: fix off-by-one error in
`best_bisection_sorted()`", 2017-11-05) is the culprit.

That changed more careful code added by Christian in 50e62a8e70
("rev-list: implement --bisect-all", 2007-10-22) to free a pointer which
as you can see can be NULL.

If you can test a patch to see if it works this should fix it:

diff --git a/bisect.c b/bisect.c
index 0fca17c02b..2f3008b078 100644
--- a/bisect.c
+++ b/bisect.c
@@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
                if (i < cnt - 1)
                        p = p->next;
        }
-       free_commit_list(p->next);
-       p->next = NULL;
+       if (p) {
+               free_commit_list(p->next);
+               p->next = NULL;
+       }
        strbuf_release(&buf);
        free(array);
        return list;

But given the commit message by Martin maybe there's some deeper bug here.