From: Jeff King <peff@peff.net>
To: Troy Moure <troy.moure@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [BUG] Segfault with rev-list --bisect
Date: Wed, 4 Mar 2015 00:33:33 -0500 [thread overview]
Message-ID: <20150304053333.GA9584@peff.net> (raw)
In-Reply-To: <CAMo-WNYNeShbbhNfG455o7krGfY7_9zVU3dMpJ7b4Smh_AiATg@mail.gmail.com>
On Tue, Mar 03, 2015 at 09:19:14AM -0500, Troy Moure wrote:
> I've found a case where git rev-list --bisect segfaults reproducibly
> (git version is 2.3.1). This is the commit topology (A2 is the first
> parent of M):
>
> I - A1 - A2
> \ \
> - B1 -- M (HEAD)
Thanks for finding a simple history which shows the problem. I recreated
this with:
git init repo && cd repo &&
echo I >I && git add I && git commit -m I &&
echo A1 >A && git add A && git commit -m A1 &&
echo A2 >A && git add A && git commit -m A2 &&
git checkout -b side HEAD~2 &&
echo B1 >B && git add B && git commit -m B1 &&
git checkout master &&
GIT_EDITOR=: git merge side
and was able to reproduce the segfault with:
git rev-list --bisect --first-parent HEAD --not HEAD~1
(it drops --parents from your command, which is not relevant to the
segfault). The segfault itself happens because we try to access the
weight() of B1, even though we never called weight_set() on it.
And that, I think, is related to --first-parent. We do not set a weight
because B1 is not an interesting commit to us (it is accessible only as
a second parent). I am not too familiar with the bisect code, but it
looks like it is not really ready to handle --first-parent. There are
several spots where it enumerates the parent list, which is going to
examine parents other than the first.
Below is a fairly hacky patch to respect --first-parent through the
bisection code. Like I said, I'm not very familiar with this code, so I
basically just blindly limited any traversal of commit->parents. It does
solve this particular segfault, but I have no clue if it is fixing other
bugs introducing them. :) E.g., it's changing count_distance(), so
perhaps our bisection counts were all off with --first-parent, even when
it didn't segfault?
diff --git a/bisect.c b/bisect.c
index 8c6d843..c51f37a 100644
--- a/bisect.c
+++ b/bisect.c
@@ -31,7 +31,7 @@ static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD
* We care just barely enough to avoid recursing for
* non-merge entries.
*/
-static int count_distance(struct commit_list *entry)
+static int count_distance(struct commit_list *entry, int max_parents)
{
int nr = 0;
@@ -47,9 +47,10 @@ static int count_distance(struct commit_list *entry)
p = commit->parents;
entry = p;
if (p) {
+ int n = max_parents - 1;
p = p->next;
- while (p) {
- nr += count_distance(p);
+ while (p && n-- > 0) {
+ nr += count_distance(p, max_parents);
p = p->next;
}
}
@@ -79,12 +80,12 @@ static inline void weight_set(struct commit_list *elem, int weight)
*((int*)(elem->item->util)) = weight;
}
-static int count_interesting_parents(struct commit *commit)
+static int count_interesting_parents(struct commit *commit, int max_parents)
{
struct commit_list *p;
int count;
- for (count = 0, p = commit->parents; p; p = p->next) {
+ for (count = 0, p = commit->parents; p && max_parents-- > 0; p = p->next) {
if (p->item->object.flags & UNINTERESTING)
continue;
count++;
@@ -117,7 +118,7 @@ static inline int halfway(struct commit_list *p, int nr)
#define show_list(a,b,c,d) do { ; } while (0)
#else
static void show_list(const char *debug, int counted, int nr,
- struct commit_list *list)
+ struct commit_list *list, int max_parents)
{
struct commit_list *p;
@@ -132,6 +133,7 @@ static void show_list(const char *debug, int counted, int nr,
char *buf = read_sha1_file(commit->object.sha1, &type, &size);
const char *subject_start;
int subject_len;
+ int n = max_parents;
fprintf(stderr, "%c%c%c ",
(flags & TREESAME) ? ' ' : 'T',
@@ -142,7 +144,7 @@ static void show_list(const char *debug, int counted, int nr,
else
fprintf(stderr, "---");
fprintf(stderr, " %.*s", 8, sha1_to_hex(commit->object.sha1));
- for (pp = commit->parents; pp; pp = pp->next)
+ for (pp = commit->parents; pp && n-- > 0; pp = pp->next)
fprintf(stderr, " %.*s", 8,
sha1_to_hex(pp->item->object.sha1));
@@ -245,7 +247,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
*/
static struct commit_list *do_find_bisection(struct commit_list *list,
int nr, int *weights,
- int find_all)
+ int find_all, int max_parents)
{
int n, counted;
struct commit_list *p;
@@ -257,7 +259,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
unsigned flags = commit->object.flags;
p->item->util = &weights[n++];
- switch (count_interesting_parents(commit)) {
+ switch (count_interesting_parents(commit, max_parents)) {
case 0:
if (!(flags & TREESAME)) {
weight_set(p, 1);
@@ -300,7 +302,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
continue;
if (weight(p) != -2)
continue;
- weight_set(p, count_distance(p));
+ weight_set(p, count_distance(p, max_parents));
clear_distance(list);
/* Does it happen to be at exactly half-way? */
@@ -315,10 +317,11 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
for (p = list; p; p = p->next) {
struct commit_list *q;
unsigned flags = p->item->object.flags;
+ int n = max_parents;
if (0 <= weight(p))
continue;
- for (q = p->item->parents; q; q = q->next) {
+ for (q = p->item->parents; q && n-- > 0; q = q->next) {
if (q->item->object.flags & UNINTERESTING)
continue;
if (0 <= weight(q))
@@ -357,11 +360,12 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
struct commit_list *find_bisection(struct commit_list *list,
int *reaches, int *all,
- int find_all)
+ int find_all, int first_parent_only)
{
int nr, on_list;
struct commit_list *p, *best, *next, *last;
int *weights;
+ int max_parents = first_parent_only ? 1 : INT_MAX;
show_list("bisection 2 entry", 0, 0, list);
@@ -390,7 +394,7 @@ struct commit_list *find_bisection(struct commit_list *list,
weights = xcalloc(on_list, sizeof(*weights));
/* Do the real work of finding bisection commit. */
- best = do_find_bisection(list, nr, weights, find_all);
+ best = do_find_bisection(list, nr, weights, find_all, max_parents);
if (best) {
if (!find_all)
best->next = NULL;
@@ -916,7 +920,8 @@ int bisect_next_all(const char *prefix, int no_checkout)
bisect_common(&revs);
revs.commits = find_bisection(revs.commits, &reaches, &all,
- !!skipped_revs.nr);
+ !!skipped_revs.nr,
+ revs.first_parent_only);
revs.commits = managed_skipped(revs.commits, &tried);
if (!revs.commits) {
diff --git a/bisect.h b/bisect.h
index 2a6c831..03d04d1 100644
--- a/bisect.h
+++ b/bisect.h
@@ -3,7 +3,7 @@
extern struct commit_list *find_bisection(struct commit_list *list,
int *reaches, int *all,
- int find_all);
+ int find_all, int first_parent_only);
extern struct commit_list *filter_skipped(struct commit_list *list,
struct commit_list **tried,
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ff84a82..3f531d6 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -380,7 +380,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
int reaches = reaches, all = all;
revs.commits = find_bisection(revs.commits, &reaches, &all,
- bisect_find_all);
+ bisect_find_all,
+ revs.first_parent_only);
if (bisect_show_vars)
return show_bisect_vars(&info, reaches, all);
next prev parent reply other threads:[~2015-03-04 5:33 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-03 14:19 [BUG] Segfault with rev-list --bisect Troy Moure
2015-03-04 5:33 ` Jeff King [this message]
2015-03-04 23:44 ` Junio C Hamano
2015-03-05 2:15 ` Troy Moure
2015-03-07 21:31 ` [PATCH] rev-list: refuse --first-parent combined with --bisect Kevin Daudt
2015-03-07 23:13 ` Kevin Daudt
2015-03-08 8:00 ` Junio C Hamano
2015-03-08 14:18 ` [PATCH v2] " Kevin Daudt
2015-03-08 15:02 ` [PATCH v3] " Kevin Daudt
2015-03-08 15:03 ` Kevin Daudt
2015-03-08 21:58 ` Eric Sunshine
2015-03-09 11:57 ` Kevin Daudt
2015-03-09 20:56 ` [PATCH v4] " Kevin Daudt
2015-03-10 22:09 ` Junio C Hamano
2015-03-10 22:55 ` Kevin Daudt
2015-03-10 23:12 ` Junio C Hamano
2015-03-11 18:45 ` Kevin Daudt
2015-03-11 20:13 ` Junio C Hamano
2015-03-16 16:33 ` Kevin Daudt
2015-03-16 18:53 ` Junio C Hamano
2015-03-16 20:25 ` Philip Oakley
2015-03-16 21:05 ` Junio C Hamano
2015-03-17 16:09 ` Christian Couder
2015-03-17 18:33 ` Junio C Hamano
2015-03-17 19:49 ` Christian Couder
2015-03-17 20:46 ` Junio C Hamano
2015-03-18 10:36 ` Christian Couder
2015-03-19 23:51 ` Philip Oakley
2015-03-20 13:02 ` Scott Schmit
2015-03-19 22:14 ` [PATCH v5] " Kevin Daudt
2015-03-19 22:43 ` Junio C Hamano
2015-03-21 22:01 ` Kevin Daudt
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=20150304053333.GA9584@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=troy.moure@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).