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
