git died

git died

[Christian]

hi list,

i've got git-1.5.1 here and did a git push to a http based repository, i 
got a segfault :(

please see the logs :


root@beroTester:/usr/src/mISDN# git push
Fetching remote heads...
  refs/remotes/origin/
  refs/remotes/
  refs/tags/
  refs/heads/
  refs/
'refs/remotes/origin/origin': up-to-date
'refs/remotes/origin/mqueue': up-to-date
updating 'refs/remotes/origin/master'
  from 943055277343bac48eed11ef6f2a9c8a4139e8bf
  to   9fe4b6bc5eadbcec188cb0c91910a1b557a27478
    done
'refs/remotes/origin/mISDN_1_1': up-to-date
'refs/remotes/origin/NEWCTRL_BRANCH': up-to-date
updating 'refs/remotes/origin/HEAD'
  from 943055277343bac48eed11ef6f2a9c8a4139e8bf
  to   9fe4b6bc5eadbcec188cb0c91910a1b557a27478
    done
'refs/tags/VERSION_0_1': up-to-date
'refs/tags/mISDN_1_1_2': up-to-date
'refs/tags/mISDN_1_0_2': up-to-date
'refs/tags/mISDN_1_0_0': up-to-date
'refs/tags/before_mISDN': up-to-date
'refs/tags/mISDN_1_0_4': up-to-date
'refs/tags/mISDN_1_1_1': up-to-date
'refs/tags/mISDN_1_0_1': up-to-date
'refs/tags/BEFORE_SKB': up-to-date
'refs/tags/OLDCTRL': up-to-date
'refs/tags/PRE_MQUEUE': up-to-date
'refs/tags/mISDN_1_0_3': up-to-date
'refs/tags/mISDN_1_1_0': up-to-date
'refs/heads/mISDN_1_1': up-to-date
updating 'refs/heads/master'
  from 9fe4b6bc5eadbcec188cb0c91910a1b557a27478
  to   4c711e270dcddf90e634f312dbbc002f3c025fc5
fatal: http-push died with strange error



root@beroTester:/usr/src/mISDN# gdb git core
GNU gdb 6.0
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain 
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-linux"...

warning: core file may not match specified executable file.
Core was generated by `/usr//bin/git-http-push 
http://git.misdn.org/git/mISDN.git/'.
Program terminated with signal 11, Segmentation fault.
#0  0x0806c7ad in git_pack_config (k=0xbff51138 
"È\023õ¿Í4\005\b@\002\r\b\224\023õ¿Ø\021õ¿", v=0x80d0240 "i")
    at builtin-pack-objects.c:1442
1442            return git_default_config(k, v);
(gdb) p k
$1 = 0xbff51138 "È\023õ¿Í4\005\b@\002\r\b\224\023õ¿Ø\021õ¿"
(gdb) p v
$2 = 0x80d0240 "i"
(gdb) bt
#0  0x0806c7ad in git_pack_config (k=0xbff51138 
"È\023õ¿Í4\005\b@\002\r\b\224\023õ¿Ø\021õ¿", v=0x80d0240 "i")
    at builtin-pack-objects.c:1442
#1  0x0804b9a8 in cmd_add (argc=135070272, argv=0xbff51394, 
prefix=0xbff511d8 "(&\017\b") at builtin-add.c:213
#2  0x080534cd in find_origin (sb=0x0, parent=0xb7f0ecc0, 
origin=0xbff51428) at builtin-blame.c:337
#3  0xb7c22eb0 in ?? ()
(gdb) bt full
#0  0x0806c7ad in git_pack_config (k=0xbff51138 
"È\023õ¿Í4\005\b@\002\r\b\224\023õ¿Ø\021õ¿", v=0x80d0240 "i")
    at builtin-pack-objects.c:1442
No locals.
#1  0x0804b9a8 in cmd_add (argc=135070272, argv=0xbff51394, 
prefix=0xbff511d8 "(&\017\b") at builtin-add.c:213
        st = {st_dev = 0, __pad1 = 0, __st_ino = 0, st_mode = 0, 
st_nlink = 411, st_uid = 0, st_gid = 4096,
  st_rdev = 8, __pad2 = 25220, st_size = 5057650701223280700, st_blksize 
= 827022072,
  st_blocks = 3552032753487733380, st_atim = {tv_sec = 119235924, 
tv_nsec = 0}, st_mtim = {tv_sec = 2,
    tv_nsec = -1074458392}, st_ctim = {tv_sec = 134573638, tv_nsec = 8}, 
st_ino = 13831980762889212616}
        i = Variable "i" is not available.
(gdb)

Re: git died

[Julian Phillips]

On Thu, 26 Apr 2007, Christian wrote:

> hi list,
>
> i've got git-1.5.1 here and did a git push to a http based repository, i got 
> a segfault :(
>
> please see the logs :

...

> root@beroTester:/usr/src/mISDN# gdb git core
> GNU gdb 6.0
> Copyright 2003 Free Software Foundation, Inc.
> GDB is free software, covered by the GNU General Public License, and you are
> welcome to change it and/or distribute copies of it under certain conditions.
> Type "show copying" to see the conditions.
> There is absolutely no warranty for GDB.  Type "show warranty" for details.
> This GDB was configured as "i386-linux"...
>
> warning: core file may not match specified executable file.
> Core was generated by `/usr//bin/git-http-push

git-http-push is a separate program.  You might want to try "gdb 
git-http-push core"?

-- 
Julian

  ---

Re: git died

[Christian]

Julian Phillips wrote:
> On Thu, 26 Apr 2007, Christian wrote:
>
>> hi list,
>>
>> i've got git-1.5.1 here and did a git push to a http based 
>> repository, i got a segfault :(
>>
>> please see the logs :
>
> ...
>
>> root@beroTester:/usr/src/mISDN# gdb git core
>> GNU gdb 6.0
>> Copyright 2003 Free Software Foundation, Inc.
>> GDB is free software, covered by the GNU General Public License, and 
>> you are
>> welcome to change it and/or distribute copies of it under certain 
>> conditions.
>> Type "show copying" to see the conditions.
>> There is absolutely no warranty for GDB.  Type "show warranty" for 
>> details.
>> This GDB was configured as "i386-linux"...
>>
>> warning: core file may not match specified executable file.
>> Core was generated by `/usr//bin/git-http-push
>
> git-http-push is a separate program.  You might want to try "gdb 
> git-http-push core"?
>
Loaded symbols for /lib/tls/libnss_dns.so.2
#0  decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
10              while ((c = *str++) != ' ') {
(gdb) bt
#0  decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
#1  0x0804b9a8 in mark_tree_uninteresting (tree=0x80d0240) at revision.c:65
#2  0x080534cd in main (argc=2, argv=Cannot access memory at address 0x1ca
) at http-push.c:1998
(gdb)

Re: git died

[Linus Torvalds]

On Thu, 26 Apr 2007, Christian wrote:

> Julian Phillips wrote:
> 
> Loaded symbols for /lib/tls/libnss_dns.so.2
> #0  decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
> 10              while ((c = *str++) != ' ') {
> (gdb) bt
> #0  decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
> #1  0x0804b9a8 in mark_tree_uninteresting (tree=0x80d0240) at revision.c:65
> #2  0x080534cd in main (argc=2, argv=Cannot access memory at address 0x1ca
> ) at http-push.c:1998

It looks like some strange corruption in http-push.

You seem to have a NULL "buf" pointer (which certainly explains a 
SIGSEGV!), but mark_tree_uninteresting will have done

	if (parse_tree(tree) < 0)
		die("bad tree %s", sha1_to_hex(obj->sha1));

	init_tree_desc(&desc, tree->buffer, tree->size);
	while (tree_entry(&desc, &entry)) {
		..


and tree_entry() does

	*entry = desc->entry;
	update_tree_entry(desc);

and none of that sets "entry->buf" to NULL unless "size" was also zero.

Can you run it under something like valgrind? You'd need to run 
"git-http-push" directly (as I don't think valgrind follows forks, and 
even if you can make it do so some way that would just be horribly slow 
and hard to debug).

		Linus

Re: git died

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 26 Apr 2007, Christian wrote:
>
>> Julian Phillips wrote:
>> 
>> Loaded symbols for /lib/tls/libnss_dns.so.2
>> #0  decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
>> 10              while ((c = *str++) != ' ') {
>> (gdb) bt
>> #0  decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
>> #1  0x0804b9a8 in mark_tree_uninteresting (tree=0x80d0240) at revision.c:65
>> #2  0x080534cd in main (argc=2, argv=Cannot access memory at address 0x1ca
>> ) at http-push.c:1998
>
> It looks like some strange corruption in http-push.
> 
> You seem to have a NULL "buf" pointer (which certainly explains a 
> SIGSEGV!), but mark_tree_uninteresting will have done
>
> 	if (parse_tree(tree) < 0)
> 		die("bad tree %s", sha1_to_hex(obj->sha1));
>
> 	init_tree_desc(&desc, tree->buffer, tree->size);
> 	while (tree_entry(&desc, &entry)) {
> 		..
>
>
> and tree_entry() does
>
> 	*entry = desc->entry;
> 	update_tree_entry(desc);
>
> and none of that sets "entry->buf" to NULL unless "size" was also zero.

This has been bugging me for quite some time by now.

The call to parse_tree is coming from mark_edge_parents_uninteresting(),
which in turn is called from mark_edges_uninteresting(), both in
http-push.c

However, I think these two functions are cut&paste from old rev-list.c,
which we refactored into list-objects.c some time ago (list-objects.c
has almost identical looking functions).

I admit that I do not care much about http-push, but I looked at
these two functions in list-objects.c; it is not clear why we do
not get the same error while running rev-list --objects-edge
(aka "thin pack transfer").

mark_edges_uninteresting calls mark_tree_uninteresting() on the
tree associated with uninteresting edge commits.  I am
reasonably sure that these commits have always been parsed when
prepare_revision_walk() returns.  However, it also calls
mark_edge_parents_uninteresting(), trying to make the parent
commit and its tree uninteresting.  I do not see how we are
guaranteeing that the parents are parsed (and their trees are
available to us).

While I think your analysis above is correct, which means that
the breakage Julian is seeing (which does not have NULL tree
object) has nothing to do with what I observed above, I'd sleep
better if we had something like this patch:


 http-push.c    |    2 ++
 list-objects.c |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/http-push.c b/http-push.c
index e3f7675..156f42e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1995,6 +1995,8 @@ static void mark_edge_parents_uninteresting(struct commit *commit)
 		struct commit *parent = parents->item;
 		if (!(parent->object.flags & UNINTERESTING))
 			continue;
+		if (!parent->object.parsed)
+			parse_object(parent->object.sha1);
 		mark_tree_uninteresting(parent->tree);
 	}
 }
diff --git a/list-objects.c b/list-objects.c
index 310f8d3..c2e74f1 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -109,6 +109,8 @@ static void mark_edge_parents_uninteresting(struct commit *commit,
 		struct commit *parent = parents->item;
 		if (!(parent->object.flags & UNINTERESTING))
 			continue;
+		if (!parent->object.parsed)
+			parse_object(parent->object.sha1);
 		mark_tree_uninteresting(parent->tree);
 		if (revs->edge_hint && !(parent->object.flags & SHOWN)) {
 			parent->object.flags |= SHOWN;