Re: testsuite failures in mainline...

From: David Miller <davem@davemloft.net>
Date: Dec 15, 2007, 00:08 UTC
Message-ID: <20071214.160845.185161708.davem@davemloft.net>
In-Reply-To: <7v7ijgq311.fsf@gitster.siamese.dyndns.org>

From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 14 Dec 2007 15:18:02 -0800

> It makes me wonder what my C library has been returning during the
> tests...

If the 'name' string is high enough in the address space, the 'NULL - name' is still small enough to keep malloc() from failing.

It might be neat to defeat bugs like this by making a pointer_diff(a,b) macro or similar, that abort()'s when one of the arguments is NULL. Otherwise these bugs are so hard to find.

I tested your patch and that part of the testsuite passes now.

It now fails on t9301-fast-export.sh

+ eval '

	MASTER=$(git rev-parse --verify master) &&
	REIN=$(git rev-parse --verify rein) &&
	WER=$(git rev-parse --verify wer) &&
	MUSS=$(git rev-parse --verify muss) &&
	mkdir new &&
	git --git-dir=new/.git init &&
	git fast-export --all |
	(cd new &&
	 git fast-import &&
	 test $MASTER = $(git rev-parse --verify refs/heads/master) &&
	 test $REIN = $(git rev-parse --verify refs/tags/rein) &&
	 test $WER = $(git rev-parse --verify refs/heads/wer) &&
	 test $MUSS = $(git rev-parse --verify refs/tags/muss))

' +++ git rev-parse --verify master ++ MASTER=e529bca54909ee82f6ed442ef855ff541aec034c +++ git rev-parse --verify rein ++ REIN=e529bca54909ee82f6ed442ef855ff541aec034c +++ git rev-parse --verify wer ++ WER=ce754ded7a378a51278b2ff76d6898ec20093068 +++ git rev-parse --verify muss ++ MUSS=d85ef2305117d94969d4990d3c752752d4719be1 ++ mkdir new ++ git --git-dir=new/.git init Initialized empty Git repository in new/.git/ ++ git fast-export --all ++ cd new ++ git fast-import ./test-lib.sh: line 194: 17409 Bus error (core dumped) git fast-import

This usually indicates an unaligned memory access on sparc, which is where I'm running this.

The problem is the pool allocator in fast-import.c, it aligned objects on the size of a pointer. But this is insufficient, it needs to be at least "uintmax_t" aligned.

Also, mem_pool->space needs to be suitably aligned for a uintmax_t as well.

The following patch fixes the bug, and together with your patch all test cases now pass for me on sparc.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/fast-import.c b/fast-import.c
index 98c2bd5..4646c05 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -196,7 +196,7 @@ struct mem_pool
 	struct mem_pool *next_pool;
 	char *next_free;
 	char *end;
-	char space[FLEX_ARRAY]; /* more */
+	uintmax_t space[FLEX_ARRAY]; /* more */
 };
 
 struct atom_str
@@ -534,15 +534,15 @@ static void *pool_alloc(size_t len)
 		total_allocd += sizeof(struct mem_pool) + mem_pool_alloc;
 		p = xmalloc(sizeof(struct mem_pool) + mem_pool_alloc);
 		p->next_pool = mem_pool;
-		p->next_free = p->space;
+		p->next_free = (char *) p->space;
 		p->end = p->next_free + mem_pool_alloc;
 		mem_pool = p;
 	}
 
 	r = p->next_free;
-	/* round out to a pointer alignment */
-	if (len & (sizeof(void*) - 1))
-		len += sizeof(void*) - (len & (sizeof(void*) - 1));
+	/* round out to a 'uintmax_t' alignment */
+	if (len & (sizeof(uintmax_t) - 1))
+		len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));
 	p->next_free += len;
 	return r;
 }