[bug] Segfault in git rev-list --first-parent --bisect

[bug] Segfault in git rev-list --first-parent --bisect

[Avery Pennarun]

I've tried the following command sequence in git 1.5.6, 1.6.0,
1.6.0.8.ga578a, and 1.6.0.90.g436ed.

    $ git clone git://repo.or.cz/versaplex.git
    $ cd versaplex.git
    $ git rev-list --first-parent --bisect 5109c91 ^d798a2bfe094
    Segmentation fault

Removing either the --first-parent or the --bisect makes the segfault go away.

Thoughts?

Thanks,

Avery

Re: [bug] Segfault in git rev-list --first-parent --bisect

[Junio C Hamano]

"Avery Pennarun" <apenwarr@gmail.com> writes:

> I've tried the following command sequence in git 1.5.6, 1.6.0,
> 1.6.0.8.ga578a, and 1.6.0.90.g436ed.
>
>     $ git clone git://repo.or.cz/versaplex.git
>     $ cd versaplex.git
>     $ git rev-list --first-parent --bisect 5109c91 ^d798a2bfe094
>     Segmentation fault
>
> Removing either the --first-parent or the --bisect makes the segfault go away.

Don't use them both then.  I do not think --bisect pays any attention to
the usual revision traversal rules.

Re: [bug] Segfault in git rev-list --first-parent --bisect

[Junio C Hamano]

"Avery Pennarun" <apenwarr@gmail.com> writes:

>     $ git rev-list --first-parent --bisect 5109c91 ^d798a2bfe094
>     Segmentation fault

Totally untested, usefulness fairly unknown.

 builtin-rev-list.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git c/builtin-rev-list.c w/builtin-rev-list.c
index 893762c..aab198f 100644
--- c/builtin-rev-list.c
+++ w/builtin-rev-list.c
@@ -530,19 +530,23 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 		return best_bisection_sorted(list, nr);
 }
 
+#define BISECT_FIND_ALL 01
+#define BISECT_FIRST_PARENT_ONLY 02
 static struct commit_list *find_bisection(struct commit_list *list,
 					  int *reaches, int *all,
-					  int find_all)
+					  int flag)
 {
 	int nr, on_list;
 	struct commit_list *p, *best, *next, *last;
 	int *weights;
+	int find_all = flag & BISECT_FIND_ALL;
+	int first_parent_only = flag & BISECT_FIRST_PARENT_ONLY;
 
 	show_list("bisection 2 entry", 0, 0, list);
 
 	/*
 	 * Count the number of total and tree-changing items on the
-	 * list, while reversing the list.
+	 * list, while reversing the list and removing.
 	 */
 	for (nr = on_list = 0, last = NULL, p = list;
 	     p;
@@ -557,6 +561,8 @@ static struct commit_list *find_bisection(struct commit_list *list,
 		if (!(flags & TREESAME))
 			nr++;
 		on_list++;
+		if (first_parent_only && p->item->parents)
+			p->item->parents->next = NULL;
 	}
 	list = last;
 	show_list("bisection 2 sorted", 0, nr, list);
@@ -656,9 +662,13 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 
 	if (bisect_list) {
 		int reaches = reaches, all = all;
+		int flag = 0;
+		if (bisect_find_all)
+			flag |= BISECT_FIND_ALL;
+		if (revs.first_parent_only)
+			flag |= BISECT_FIRST_PARENT_ONLY;
+		revs.commits = find_bisection(revs.commits, &reaches, &all, flag);
 
-		revs.commits = find_bisection(revs.commits, &reaches, &all,
-					      bisect_find_all);
 		if (bisect_show_vars) {
 			int cnt;
 			char hex[41];

Re: [bug] Segfault in git rev-list --first-parent --bisect

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Avery Pennarun" <apenwarr@gmail.com> writes:
>
>>     $ git rev-list --first-parent --bisect 5109c91 ^d798a2bfe094
>>     Segmentation fault
>
> Totally untested, usefulness fairly unknown.

The latter part of this statement needs a bit of explanation.  I do not
mean "--first-parent --bisect" is useless.  What I meant is that the
approach is only catering to --first-parent and not about non-standard way
to limit the list such as --since, --max-count, etc.

The current bisection algorithm only pays attention to the pathspec based
history simplification and bottom..top (aka "UNINTERESTING or not").  A
proper fix to handle these cases should work inside do_find_bisection(),
and count_interesting_parents() instead of hiding the parents away that
first_parent traversal did not touch, like the patch I sent out.