git died

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)

...

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

Julian

 ---

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)

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

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(-)

index e3f7675..156f42e 100644
@@ -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);
 	}
 }

index 310f8d3..c2e74f1 100644
@@ -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;