{"thread":{"id":"20958","subject":"Git crashes on pull","startedAt":"2009-09-15T18:47:41Z","lastAt":"2009-09-18T13:39:48Z","messageCount":6,"participants":["Guido Ostkamp","Junio C Hamano","Michael Wookey","Tay Ray Chuan"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"123274","messageId":"alpine.LSU.2.01.0909152044450.10936@bianca.dialin.t-online.de","threadId":"20958","inReplyTo":null,"subject":"Git crashes on pull","fromName":"Guido Ostkamp","fromEmail":"git@ostkamp.fastmail.fm","sentAt":"2009-09-15T18:47:41Z","receivedAt":"2009-09-15T18:47:41Z","isPatch":false,"sender":{"key":"git@ostkamp.fastmail.fm","avatar":null},"body":"Hi,\n\nI have a clone of http://git.postgresql.org/git/postgresql.git where head \nis at commit 167501570c74390dfb7a5dd71e260ab3d4fd9904.\n\nI'm using Git version 1.6.5.rc1.10.g20f34 (should be at commit \n20f34902d154f390ebaa7eed7f42ad14140b8acb from Mon Sep 14 10:49:01 2009 \n+0200)\n\nNow when I 'git pull' then Git crashes with\n\ngit pull 2>&1 > /tmp/git-error\n*** glibc detected *** git-remote-curl: free(): invalid pointer: \n0xb7d19140 ***\n======= Backtrace: =========\n/lib/libc.so.6[0xb7c4f4b6]\n/lib/libc.so.6(cfree+0x89)[0xb7c51179]\ngit-remote-curl[0x804d290]\ngit-remote-curl[0x804df04]\ngit-remote-curl[0x8065ea5]\ngit-remote-curl[0x804aac6]\n/lib/libc.so.6(__libc_start_main+0xe0)[0xb7bfefe0]\ngit-remote-curl[0x804a991]\n======= Memory map: ========\n08048000-080a1000 r-xp 00000000 08:15 1658246 \n/usr/local/libexec/git-core/git-remote-curl\n080a1000-080a2000 r--p 00058000 08:15 1658246 \n/usr/local/libexec/git-core/git-remote-curl\n080a2000-080a3000 rw-p 00059000 08:15 1658246 \n/usr/local/libexec/git-core/git-remote-curl\n080a3000-08143000 rw-p 080a3000 00:00 0 [heap]\nb4400000-b4421000 rw-p b4400000 00:00 0\nb4421000-b4500000 ---p b4421000 00:00 0\nb45ea000-b45f4000 r-xp 00000000 08:13 1097821 /lib/libgcc_s.so.1\nb45f4000-b45f6000 rw-p 00009000 08:13 1097821 /lib/libgcc_s.so.1\n...\n\nAny idea what's causing this?\n\nPlease keep me on CC, as I'm not subscribed on list.\n\nRegards\n\nGuido\n"},{"id":"123278","messageId":"7vljkg57xs.fsf@alter.siamese.dyndns.org","threadId":"20958","inReplyTo":"alpine.LSU.2.01.0909152044450.10936@bianca.dialin.t-online.de","subject":"Re: Git crashes on pull","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-15T19:22:55Z","receivedAt":"2009-09-15T19:22:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guido Ostkamp writes:\n\n> I have a clone of http://git.postgresql.org/git/postgresql.git where\n> head is at commit 167501570c74390dfb7a5dd71e260ab3d4fd9904.\n>\n> I'm using Git version 1.6.5.rc1.10.g20f34 (should be at commit\n> 20f34902d154f390ebaa7eed7f42ad14140b8acb from Mon Sep 14 10:49:01 2009\n> +0200)\n>\n> Now when I 'git pull' then Git crashes with\n>\n> git pull 2>&1 > /tmp/git-error\n> *** glibc detected *** git-remote-curl: free(): invalid pointer:\n\nPlease try this patch, which I have been preparing for later pushout.\n\nFrom: Junio C Hamano \nDate: Mon, 14 Sep 2009 14:48:15 -0700\nSubject: [PATCH] http.c: avoid freeing an uninitialized pointer\n\nAn earlier 59b8d38 (http.c: remove verification of remote packs) left\nthe variable \"url\" uninitialized; \"goto cleanup\" codepath can free it\nwhich is not very nice.\n\nSigned-off-by: Junio C Hamano \n---\n http.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex d0cc1b3..15926d8 100644\n--- a/http.c\n+++ b/http.c\n@@ -866,7 +866,7 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)\n \tint ret = 0;\n \tchar *hex = xstrdup(sha1_to_hex(sha1));\n \tchar *filename;\n-\tchar *url;\n+\tchar *url = NULL;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \tif (has_pack_index(sha1)) {\n-- \n1.6.5.rc1\n"},{"id":"123286","messageId":"alpine.LSU.2.01.0909160022430.24554@bianca.dialin.t-online.de","threadId":"20958","inReplyTo":"7vljkg57xs.fsf@alter.siamese.dyndns.org","subject":"Re: Git crashes on pull","fromName":"Guido Ostkamp","fromEmail":"git@ostkamp.fastmail.fm","sentAt":"2009-09-15T22:30:16Z","receivedAt":"2009-09-15T22:30:16Z","isPatch":false,"sender":{"key":"git@ostkamp.fastmail.fm","avatar":null},"body":"On Tue, 15 Sep 2009, Junio C Hamano wrote:\n\n> Please try this patch, which I have been preparing for later pushout.\n>\n> From: Junio C Hamano \n> Date: Mon, 14 Sep 2009 14:48:15 -0700\n> Subject: [PATCH] http.c: avoid freeing an uninitialized pointer\n>\n> An earlier 59b8d38 (http.c: remove verification of remote packs) left\n> the variable \"url\" uninitialized; \"goto cleanup\" codepath can free it\n> which is not very nice.\n>\n> Signed-off-by: Junio C Hamano \n\nAppears to be working ok now, thanks.\n\nBTW: Is there any way to easily invoke GDB in case of such a problem to \nget a real symbolic stack backtrace?\n\nI tried it on the 'git' binary, but of course this didn't work because it \ninvokes a git-pull script which again runs another git-remote-curl binary.\n\nRegards\n\nGuido\n"},{"id":"123288","messageId":"7vzl8v4y5g.fsf@alter.siamese.dyndns.org","threadId":"20958","inReplyTo":"alpine.LSU.2.01.0909160022430.24554@bianca.dialin.t-online.de","subject":"Re: Git crashes on pull","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-15T22:54:19Z","receivedAt":"2009-09-15T22:54:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guido Ostkamp writes:\n\n> On Tue, 15 Sep 2009, Junio C Hamano wrote:\n>\n>> Please try this patch, which I have been preparing for later pushout.\n>>\n>> From: Junio C Hamano \n>> Date: Mon, 14 Sep 2009 14:48:15 -0700\n>> Subject: [PATCH] http.c: avoid freeing an uninitialized pointer\n>>\n>> An earlier 59b8d38 (http.c: remove verification of remote packs) left\n>> the variable \"url\" uninitialized; \"goto cleanup\" codepath can free it\n>> which is not very nice.\n>>\n>> Signed-off-by: Junio C Hamano \n>\n> Appears to be working ok now, thanks.\n\nThanks.\n\nThe sad part of the story was that this regression was introduced by a\nchange to work around recent breakage observed when fetching from the http\nserver github runs, and it was the primary purpose of pushing 1.6.4.3 out.\n\nNow we need to cut a 1.6.4.4 with this fix-on-fix soon, like tomorrow.\n\n> BTW: Is there any way to easily invoke GDB in case of such a problem\n> to get a real symbolic stack backtrace?\n>\n> I tried it on the 'git' binary, but of course this didn't work because\n> it invokes a git-pull script which again runs another git-remote-curl\n> binary.\n\nNot very easily. The best you can do is to run with GIT_TRACE to see what\ncommand actually dies and run that binary directly. gdb can choose to\nfollow either parent or child across forks, but I do not know how to tell\nit to follow across execs into a different binary.\n"},{"id":"123289","messageId":"d2e97e800909151630w44d440f5hadb088aa5e1f8e22@mail.gmail.com","threadId":"20958","inReplyTo":"7vzl8v4y5g.fsf@alter.siamese.dyndns.org","subject":"Re: Git crashes on pull","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2009-09-15T23:30:51Z","receivedAt":"2009-09-15T23:30:51Z","isPatch":false,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"2009/9/16 Junio C Hamano :\n> Guido Ostkamp writes:\n>\n>> On Tue, 15 Sep 2009, Junio C Hamano wrote:\n>>\n>>> Please try this patch, which I have been preparing for later pushout.\n>>>\n>>> From: Junio C Hamano \n>>> Date: Mon, 14 Sep 2009 14:48:15 -0700\n>>> Subject: [PATCH] http.c: avoid freeing an uninitialized pointer\n>>>\n>>> An earlier 59b8d38 (http.c: remove verification of remote packs) left\n>>> the variable \"url\" uninitialized; \"goto cleanup\" codepath can free it\n>>> which is not very nice.\n>>>\n>>> Signed-off-by: Junio C Hamano \n>>\n>> Appears to be working ok now, thanks.\n>\n> Thanks.\n>\n> The sad part of the story was that this regression was introduced by a\n> change to work around recent breakage observed when fetching from the http\n> server github runs, and it was the primary purpose of pushing 1.6.4.3 out.\n\nIf only I had given it a run with the clang static analyzer earlier :(\n\nHere is what Xcode would have shown -\n\n http://dl.getdropbox.com/u/1006983/git-clang.png\n\nI can make the Xcode project available if anyone is interested.\n"},{"id":"123491","messageId":"20090918213948.4bb65f4e.rctay89@gmail.com","threadId":"20958","inReplyTo":"7vzl8v4y5g.fsf@alter.siamese.dyndns.org","subject":"Re: Git crashes on pull","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-09-18T13:39:48Z","receivedAt":"2009-09-18T13:39:48Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Sep 16, 2009 at 6:54 AM, Junio C Hamano wrote:\n> Thanks.\n>\n> The sad part of the story was that this regression was introduced by a\n> change to work around recent breakage observed when fetching from the http\n> server github runs, and it was the primary purpose of pushing 1.6.4.3 out.\n>\n> Now we need to cut a 1.6.4.4 with this fix-on-fix soon, like tomorrow.\n\nsorry for all the trouble caused.\n\nJunio, do you think moving out the free() would be a better option? Setting it to NULL just so we can free() is rather contrived, I feel.\n\n-- >8 --\n\nSubject: [PATCH] http.c: move free() out of cleanup block\n\nInstead of initializing a variable (url) just so we can do a free() on\nit, as in b202514 (http.c: avoid freeing an uninitialized pointer), we\nmove the free() out of cleanup block.\n\nSigned-off-by: Tay Ray Chuan \n---\n http.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 23b2a19..a67f62e 100644\n--- a/http.c\n+++ b/http.c\n@@ -866,7 +866,7 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)\n \tint ret = 0;\n \tchar *hex = xstrdup(sha1_to_hex(sha1));\n \tchar *filename;\n-\tchar *url = NULL;\n+\tchar *url;\n \tstruct strbuf buf = STRBUF_INIT;\n\n \tif (has_pack_index(sha1)) {\n@@ -885,9 +885,9 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)\n \tif (http_get_file(url, filename, 0) != HTTP_OK)\n \t\tret = error(\"Unable to get pack index %s\\n\", url);\n\n+\tfree(url);\n cleanup:\n \tfree(hex);\n-\tfree(url);\n \treturn ret;\n }\n\n--\n1.6.4.2\n"}]}