testsuite failures in mainline...

testsuite failures in mainline...

[David Miller]

I've been seeing this for I think a week or two, and I can't figure
out if it's some local problem of mine.  I even reran the testsuite
with "PATH=$(pwd):$PATH" just in case it was picking up my existing
1.5.3.7 installation for some reason, but it still fails even in that
case.

Is the following a known issue?

*** t3200-branch.sh ***
*   ok 1: prepare a trivial repository
*   ok 2: git branch --help should not have created a bogus branch
*   ok 3: git branch abc should create a branch
*   ok 4: git branch a/b/c should create a branch
*   ok 5: git branch -l d/e/f should create a branch and a log
*   ok 6: git branch -d d/e/f should delete a branch and a log
*   ok 7: git branch j/k should work after branch j has been deleted
*   ok 8: git branch l should work after branch l/m has been deleted
*   ok 9: git branch -m m m/m should work
*   ok 10: git branch -m n/n n should work
*   ok 11: git branch -m o/o o should fail when o/p exists
*   ok 12: git branch -m q r/q should fail when r exists
*   ok 13: git branch -m q q2 without config should succeed
*   ok 14: git branch -m s/s s should work when s/t is deleted
*   ok 15: config information was renamed, too
*   ok 16: git branch -m u v should fail when the reflog for u is a symlink
*   ok 17: test tracking setup via --track
*   ok 18: test tracking setup (non-wildcard, matching)
*   ok 19: test tracking setup (non-wildcard, not matching)
* FAIL 20: test tracking setup via config
        git config branch.autosetupmerge true &&
             git config remote.local.url . &&
             git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
             (git show-ref -q refs/remotes/local/master || git-fetch local) &&
             git branch my3 local/master &&
             test $(git config branch.my3.remote) = local &&
             test $(git config branch.my3.merge) = refs/heads/master
* FAIL 21: avoid ambiguous track
        
                git config branch.autosetupmerge true &&
                git config remote.ambi1.url = lalala &&
                git config remote.ambi1.fetch = refs/heads/lalala:refs/heads/master &&
                git config remote.ambi2.url = lilili &&
                git config remote.ambi2.fetch = refs/heads/lilili:refs/heads/master &&
                git branch all1 master &&
                test -z "$(git config branch.all1.merge)"
        
*   ok 22: test overriding tracking setup via --no-track
* FAIL 23: no tracking without .fetch entries
        git branch --track my6 s &&
             test -z "$(git config branch.my6.remote)" &&
             test -z "$(git config branch.my6.merge)"
* FAIL 24: test tracking setup via --track but deeper
        git config remote.local.url . &&
             git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
             (git show-ref -q refs/remotes/local/o/o || git-fetch local) &&
             git branch --track my7 local/o/o &&
             test "$(git config branch.my7.remote)" = local &&
             test "$(git config branch.my7.merge)" = refs/heads/o/o
* FAIL 25: test deleting branch deletes branch config
        git branch -d my7 &&
             test -z "$(git config branch.my7.remote)" &&
             test -z "$(git config branch.my7.merge)"
*   ok 26: test deleting branch without config
*   ok 27: git checkout -b g/h/i -l should create a branch and a log
* failed 5 among 27 test(s)

Re: testsuite failures in mainline...

[Junio C Hamano]

David Miller <davem@davemloft.net> writes:

> I've been seeing this for I think a week or two, and I can't figure
> out if it's some local problem of mine.  I even reran the testsuite
> with "PATH=$(pwd):$PATH" just in case it was picking up my existing
> 1.5.3.7 installation for some reason, but it still fails even in that
> case.
>
> Is the following a known issue?

No, you are the first to report it here.  I do not push out the tip of
'master' and 'next' unless all tests pass in my primary repository on my
private machine and in a repository under my k.org account, so you can
at least assume all tests has passed for _some_ people.  It of course
does not guarantee there isn't timing related bugs I am not catching. 

> *** t3200-branch.sh ***
> ...
> * FAIL 20: test tracking setup via config
>         git config branch.autosetupmerge true &&
>              git config remote.local.url . &&
>              git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
>              (git show-ref -q refs/remotes/local/master || git-fetch local) &&
>              git branch my3 local/master &&
>              test $(git config branch.my3.remote) = local &&
>              test $(git config branch.my3.merge) = refs/heads/master

When I can reproduce a breakage, in our tests, I'd run

	cd t && sh -x t3200-branch.sh -i -v

(replace "t3200-*" with the failing test) and see which one of the steps
chained with && is breaking first.

Re: testsuite failures in mainline...

[David Miller]

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

> When I can reproduce a breakage, in our tests, I'd run
> 
> 	cd t && sh -x t3200-branch.sh -i -v
> 
> (replace "t3200-*" with the failing test) and see which one of the steps
> chained with && is breaking first.

Looks like a malloc() failure:

+ test_expect_success 'test tracking setup via config' 'git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
+ test 2 = 2
+ test_skip 'test tracking setup via config' 'git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
++ expr ./t3200-branch.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t3200
++ expr 19 + 1
+ this_test=t3200.20
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success: git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
+ say_color info 'expecting success: git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
+ test -z info
+ shift
+ echo '* expecting success: git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
* expecting success: git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master
+ test_run_ 'git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
+ eval 'git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
++ git config branch.autosetupmerge true
++ git config remote.local.url .
++ git config remote.local.fetch 'refs/heads/*:refs/remotes/local/*'
++ git show-ref -q refs/remotes/local/master
++ git branch my3 local/master
fatal: Out of memory, malloc failed
+ eval_ret=128
+ return 0
+ '[' 0 = 0 -a 128 = 0 ']'
+ test_failure_ 'test tracking setup via config' 'git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
++ expr 19 + 1
+ test_count=20
++ expr 0 + 1
+ test_failure=1
+ say_color error 'FAIL 20: test tracking setup via config'
+ test -z error
+ shift
+ echo '* FAIL 20: test tracking setup via config'
* FAIL 20: test tracking setup via config
+ shift
+ echo 'git config branch.autosetupmerge true &&
     git config remote.local.url . &&
     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
     git branch my3 local/master &&
     test $(git config branch.my3.remote) = local &&
     test $(git config branch.my3.merge) = refs/heads/master'
+ sed -e 's/^/	/'
	git config branch.autosetupmerge true &&
	     git config remote.local.url . &&
	     git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
	     (git show-ref -q refs/remotes/local/master || git-fetch local) &&
	     git branch my3 local/master &&
	     test $(git config branch.my3.remote) = local &&
	     test $(git config branch.my3.merge) = refs/heads/master
+ test t = ''
+ trap - exit
+ exit 1

Re: testsuite failures in mainline...

[Junio C Hamano]

David Miller <davem@davemloft.net> writes:

> From: Junio C Hamano <gitster@pobox.com>
> Date: Fri, 14 Dec 2007 11:15:01 -0800
>
>> When I can reproduce a breakage, in our tests, I'd run
>> 
>> 	cd t && sh -x t3200-branch.sh -i -v
>> 
>> (replace "t3200-*" with the failing test) and see which one of the steps
>> chained with && is breaking first.
>
> Looks like a malloc() failure:
> ...
> ++ git branch my3 local/master
> fatal: Out of memory, malloc failed

Do you mean this is a malloc() failure that you can reliable reproduce?
Puzzled...

Re: testsuite failures in mainline...

[Alex Riesen]

David Miller, Fri, Dec 14, 2007 20:17:36 +0100:
> ++ git show-ref -q refs/remotes/local/master
> ++ git branch my3 local/master
> fatal: Out of memory, malloc failed

Something unusual about the system? Like a malloc debugger in
LD_PRELOAD configuration?

Maybe you could retry with a little bit instrumentation?
(The program last failed (git-branch) is normally very benign...)

Something like this:


diff --git a/git-compat-util.h b/git-compat-util.h
index 79eb10e..a9cc249 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -230,7 +230,8 @@ static inline char* xstrdup(const char *str)
 	return ret;
 }
 
-static inline void *xmalloc(size_t size)
+#define xmalloc(size) xmalloc_((size),__FILE__,__LINE__)
+static inline void *xmalloc_(size_t size, const char *file, int line)
 {
 	void *ret = malloc(size);
 	if (!ret && !size)
@@ -241,7 +242,8 @@ static inline void *xmalloc(size_t size)
 		if (!ret && !size)
 			ret = malloc(1);
 		if (!ret)
-			die("Out of memory, malloc failed");
+			die("Out of memory, malloc(%u) at %s:%d failed",
+			    size, file, line);
 	}
 #ifdef XMALLOC_POISON
 	memset(ret, 0xA5, size);
@@ -263,7 +265,8 @@ static inline char *xstrndup(const char *str, size_t len)
 	return xmemdupz(str, p ? p - str : len);
 }
 
-static inline void *xrealloc(void *ptr, size_t size)
+#define xrealloc(ptr,size) xrealloc_((ptr),(size),__FILE__,__LINE__)
+static inline void *xrealloc_(void *ptr, size_t size, const char *file, int line)
 {
 	void *ret = realloc(ptr, size);
 	if (!ret && !size)
@@ -274,7 +277,8 @@ static inline void *xrealloc(void *ptr, size_t size)
 		if (!ret && !size)
 			ret = realloc(ptr, 1);
 		if (!ret)
-			die("Out of memory, realloc failed");
+			die("Out of memory, realloc(%u) at %s:%d failed",
+			    size, file, line);
 	}
 	return ret;
 }

Re: testsuite failures in mainline...

[David Miller]

From: Alex Riesen <raa.lkml@gmail.com>
Date: Fri, 14 Dec 2007 22:45:33 +0100

> David Miller, Fri, Dec 14, 2007 20:17:36 +0100:
> > ++ git show-ref -q refs/remotes/local/master
> > ++ git branch my3 local/master
> > fatal: Out of memory, malloc failed
> 
> Something unusual about the system? Like a malloc debugger in
> LD_PRELOAD configuration?

No, I'm not doing anything fancy like that.

> Maybe you could retry with a little bit instrumentation?
> (The program last failed (git-branch) is normally very benign...)
> 
> Something like this:

Here is the output from the debugging patch:

++ git branch my3 local/master
fatal: Out of memory, malloc(4293963242) at git-compat-util.h:256 failed

This bogus size value in hex is 0xfff0adea, FWIW.

I added similar diags to xmemdupz() and xstrndup() and that gives us:

fatal: Out of memory, xstrndup(0x103ebf:4293902657:4293902657) at remote.c:112 failed

(the first three values are xstrndup() vars 'str', 'len', and
 the computed second argument to xmemdupz).

That bogus length is being generated via the length argument
passed to make_branch() by handle_config() in remote.c:

	if (!prefixcmp(key, "branch.")) {
		name = key + 7;
		subkey = strrchr(name, '.');
		branch = make_branch(name, subkey - name);

What if 'subkey' is NULL?  I bet that's what happening here.

I added a debugging check for this and indeed, subkey is NULL
in handle_condig() and that's why 'subkey - name' generates that
bogus negative length.

The string 'key' is "branch.autosetupmerge" in this case.

Re: testsuite failures in mainline...

[Junio C Hamano]

David Miller <davem@davemloft.net> writes:

> 	if (!prefixcmp(key, "branch.")) {
> 		name = key + 7;
> 		subkey = strrchr(name, '.');
> 		branch = make_branch(name, subkey - name);
>
> What if 'subkey' is NULL?  I bet that's what happening here.

Wow, good eyes.

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

---

 remote.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote.c b/remote.c
index 3fb0f99..0e00680 100644
--- a/remote.c
+++ b/remote.c
@@ -220,11 +220,11 @@ static int handle_config(const char *key, const char *value)
 	if (!prefixcmp(key, "branch.")) {
 		name = key + 7;
 		subkey = strrchr(name, '.');
-		branch = make_branch(name, subkey - name);
 		if (!subkey)
 			return 0;
 		if (!value)
 			return 0;
+		branch = make_branch(name, subkey - name);
 		if (!strcmp(subkey, ".remote")) {
 			branch->remote_name = xstrdup(value);
 			if (branch == current_branch)

Re: testsuite failures in mainline...

[David Miller]

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;
 }

Re: testsuite failures in mainline...

[Johannes Schindelin]

Hi,

On Fri, 14 Dec 2007, David Miller wrote:

> -	char space[FLEX_ARRAY]; /* more */
> +	uintmax_t space[FLEX_ARRAY]; /* more */

Usually, a much better idea is to use

	union {
		char cp[FLEX_ARRAY];
		uintmax_t up[FLEX_ARRAY];
	}

because that is exactly the reason union was invented for.

Ciao,
Dscho