threads / discuss / 7853

git died

Subject: git died

## tl;dr

5 messages between Apr 26, 2007 and Apr 26, 2007.

replies: 4people: 4as markdown or json

Christian· Apr 26, 2007, 09:52 UTC · lore
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)
Julian Phillips· Apr 26, 2007, 10:28 UTC · re: Christian · lore

Re: git died

On Thu, 26 Apr 2007, Christian wrote:
Show 6 quoted lines
> 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 :
...
Show 11 quoted lines
> 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

  ---
Christian· Apr 26, 2007, 12:20 UTC · re: Julian Phillips · lore

Re: git died

Julian Phillips wrote:
Show 29 quoted lines
> 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)

Linus Torvalds· Apr 26, 2007, 19:51 UTC · re: Christian · lore

Re: git died

On Thu, 26 Apr 2007, Christian wrote:
Show 10 quoted lines
> 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
Junio C Hamano· Apr 26, 2007, 20:55 UTC · re: Linus Torvalds · lore

Re: git died

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 32 quoted lines
> 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;

← back to recent threads