From: Yasushi SHOJI <yasushi.shoji@gmail.com>
To: "Martin Ågren" <martin.agren@gmail.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Git Mailing List" <git@vger.kernel.org>,
"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [BUG] v2.16.0-rc0 seg faults when git bisect skip
Date: Fri, 5 Jan 2018 11:45:43 +0900 [thread overview]
Message-ID: <CAELBRWK6Y=-7WBwai16dBKd8OLxdXWOiZMALVJXrP9ak8gF-LA@mail.gmail.com> (raw)
In-Reply-To: <CAN0heSrZ4dEFqNX69PgtGCERJKabokz88v-vnNZkUBXfK118mg@mail.gmail.com>
Hi,
On Thu, Jan 4, 2018 at 3:26 AM, Martin Ågren <martin.agren@gmail.com> wrote:
> On 3 January 2018 at 15:21, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>>
>> 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.
>
> I haven't tried to reproduce, or tested the patch, but from the looks of
> it, your analysis and fix are both spot on. The special case that yashi
> has hit is that `list` is NULL. The old code handled that very well, the
> code after my patch ... not so well. The loop-sort-loop pattern reduces
> to a no-op, both before and after my patch. But what I failed to realize
> was that `list` could be NULL.
The patch (actually, I've tested the one in pu, 2e9fdc795cb27) avoids
the seg fault for sure, but the question is:
When does the list allowed to contain NULLs?
Since nobody noticed it since 7c117184d7, it must be a rare case, right?
Would you guys elaborate a bit? I don't have any insight how
best_bisection_sorted() should work and what the list should contain.
So that I can make a test case.
Thanks,
--
yashi
next prev parent reply other threads:[~2018-01-05 2:45 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-03 12:36 [BUG] v2.16.0-rc0 seg faults when git bisect skip Yasushi SHOJI
2018-01-03 14:21 ` Ævar Arnfjörð Bjarmason
2018-01-03 18:26 ` Martin Ågren
2018-01-03 18:48 ` [PATCH] bisect: fix a regression causing a segfault Ævar Arnfjörð Bjarmason
2018-01-05 2:45 ` Yasushi SHOJI [this message]
2018-01-05 5:28 ` [BUG] v2.16.0-rc0 seg faults when git bisect skip Yasushi SHOJI
2018-01-06 8:21 ` Martin Ågren
2018-01-06 14:27 ` Yasushi SHOJI
2018-01-06 15:02 ` Martin Ågren
2018-01-06 15:05 ` Christian Couder
2018-01-08 14:45 ` Yasushi SHOJI
2018-01-08 15:09 ` Christian Couder
2018-01-09 11:09 ` Yasushi SHOJI
2018-01-05 22:20 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAELBRWK6Y=-7WBwai16dBKd8OLxdXWOiZMALVJXrP9ak8gF-LA@mail.gmail.com' \
--to=yasushi.shoji@gmail.com \
--cc=avarab@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=martin.agren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).