Re: Segmentation fault with latest git (070c57df)
sam wrote: gdb /usr/bin/git core What exactly are you doing? Is core aliased to something? [...] Core was generated by `git apply --verbose --check --ignore-whitespace --directory=/home/sam/P'. What is this? Can you give us clear instructions on how to reproduce the segfault? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
Ramkumar Ramachandra wrote: sam wrote: gdb /usr/bin/git core What exactly are you doing? Is core aliased to something? Sorry about that. I just realized you're loading a core dump. Please tell us how to reproduce this, or give us the backtrace with debugging symbols. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
Hi, Has there been any further progress on this. I just encountered a SEGV with a git apply. This is the latest git version running on Ubuntu 13.04 cat /etc/lsb-release DISTRIB_ID=Ubuntu DISTRIB_RELEASE=13.04 DISTRIB_CODENAME=raring DISTRIB_DESCRIPTION=Ubuntu 13.04 uname -a Linux sam-mac 3.8.0-20-generic #31-Ubuntu SMP Mon May 6 17:03:32 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux git --version git version 1.8.2.2 samueldoyle@sam-MacBookPro:~/Projects/VMware/perforce_repos/esx50u3ps$ sudo gdb /usr/bin/git core GNU gdb (GDB) 7.5.91.20130417-cvs-ubuntu Copyright (C) 2013 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-linux-gnu. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/... Reading symbols from /usr/bin/git...(no debugging symbols found)...done. [New LWP 28634] warning: Can't read pathname for load map: Input/output error. [Thread debugging using libthread_db enabled] Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1. warning: no loadable sections found in added symbol-file system-supplied DSO at 0x7fff94d1a000 Core was generated by `git apply --verbose --check --ignore-whitespace --directory=/home/sam/P'. Program terminated with signal 11, Segmentation fault. #0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31 31 ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory. (gdb) bt 10 #0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31 #1 0x7f06ba7c28c6 in __GI___strdup (s=0x0) at strdup.c:41 #2 0x004f4449 in ?? () #3 0x0040eab0 in ?? () #4 0x0040f6f5 in ?? () #5 0x00405a88 in ?? () #6 0x00404ee2 in ?? () #7 0x7f06ba75aea5 in __libc_start_main (main=0x404e30, argc=7, ubp_av=0x7fff94c3bd08, init=optimized out, fini=optimized out, rtld_fini=optimized out, stack_end=0x7fff94c3bcf8) at libc-start.c:260 #8 0x00405329 in ?? () A quick search turned up this post that appears to provide the cause: http://stackoverflow.com/questions/3608931/why-is-this-program-segfaulting -- View this message in context: http://git.661346.n2.nabble.com/Segmentation-fault-with-latest-git-070c57df-tp7576614p7585906.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
Hmmm nabble embed didn't provide much value there :) http://pastebin.com/RC8mUPF3 -- View this message in context: http://git.661346.n2.nabble.com/Segmentation-fault-with-latest-git-070c57df-tp7576614p7585907.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Sun, Feb 03, 2013 at 11:13:00PM -0800, Junio C Hamano wrote: Yeah, that result is understandable, as .depend/*.o.d files will not be rebuilt when the rules to build them changes in the Makefile. Applying the patch to the Makefile in the pristine old tree, run the build (which will generate .depend/*.o.d files with the corrected rules), then checking out the new tree and running the build again without make clean, with or with the patch applied, would validate that the patch fixes the issue for old ccache. Thanks Jonathan for diagnosing, fixing, and thanks Jongman for testing. Do we want to do anything with the other dependency hole I found here: http://article.gmane.org/gmane.comp.version-control.git/215211 It's definitely a potential problem, but I don't think we have any reports of it happening in practice, so it might not be worth worrying about. Doing a clean version of the fix here: http://article.gmane.org/gmane.comp.version-control.git/215212 would probably involve reorganizing our .depend directory structure, unless somebody can cook up some clever use of make's patsubst. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
Jeff King p...@peff.net writes: Do we want to do anything with the other dependency hole I found here: http://article.gmane.org/gmane.comp.version-control.git/215211 It's definitely a potential problem, but I don't think we have any reports of it happening in practice, so it might not be worth worrying about. Doing a clean version of the fix here: http://article.gmane.org/gmane.comp.version-control.git/215212 would probably involve reorganizing our .depend directory structure, unless somebody can cook up some clever use of make's patsubst. As I understand how the current set-up works: * Initially, we do not have foo.o but foo.c. We automatically build foo.o because it depends on foo.c via the %.o : %.c rule, and as a side effect, we also build .depend/foo.o.d file; * Then, if any real dependency used to build the existing foo.o that is recorded in .depend/foo.o.d file changes, foo.o gets rebuilt, which would update .depend/foo.o.d again for the next invocation. The case where you lose .depend/foo.o.d is a special case of getting a wrong information in .depend/foo.o.d, which may happen by using a broken compiler during the initial build, or going over quota and getting .depend/foo.o.d truncated, or by other breakages. The user may have done rm -rf .depend to lose it, or the user may have done something like this to munge it: find -name '.git' -type d -prune -o -print0 | xargs -0 sed -i -e 's/foo/bar/g' forgetting that just like .git, .depend is precious and should not be touched. I think this really boils down to where we draw the this is good enough line. I am not sure if losing the file as in $gmane/215211 is common enough to be special cased to buy us much, while leaving other .depend/foo.o.d was updated to contain a wrong info cases still broken. And of course the case where .depend/foo.o.d is munged by mistake cannot be solved without recompiling everything all the time, so... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
Junio C Hamano gits...@pobox.com writes: As I understand how the current set-up works: * Initially, we do not have foo.o but foo.c. We automatically build foo.o because it depends on foo.c via the %.o : %.c rule, and as a side effect, we also build .depend/foo.o.d file; * Then, if any real dependency used to build the existing foo.o that is recorded in .depend/foo.o.d file changes, foo.o gets rebuilt, which would update .depend/foo.o.d again for the next invocation. This is unrelated to the case you mentioned, but I wonder what happens if you did this: * You are on branch 'next', where foo.c includes (perhaps indirectly) frotz.h. Compile and you get foo.o and also the dependency recorded for it, foo.o: foo.c frotz.h, in the .depend/foo.o.d file. * You check out branch 'master', where foo.c does not include frotz.h. Indeed, the include file does not even exist on the branch. Do we get confused, because Makefile includes the depend file from the previous build, finds that you need foo.c and frotz.h up to date in order to get foo.o, but there is no rule to generate frotz.h? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Mon, Feb 04, 2013 at 01:16:08AM -0800, Junio C Hamano wrote: I think this really boils down to where we draw the this is good enough line. I am not sure if losing the file as in $gmane/215211 is common enough to be special cased to buy us much, while leaving other .depend/foo.o.d was updated to contain a wrong info cases still broken. Hmm. Yeah, I was thinking it might be more common than ordinary munging due to something like an interrupted git clean -x. But given that: 1. As far as I can tell, it is not a situation that can happen through regular use of checkout/make/etc, and... 2. We have zero reports of it happening in practice (I only discovered it while explicitly trying to break the Makefile), and... 3. It is just one of many possible breakages, all of which can be fixed by git clean -dx if you suspect issues... let's just leave it. Thanks for a sanity check. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Mon, Feb 04, 2013 at 01:29:41AM -0800, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: As I understand how the current set-up works: * Initially, we do not have foo.o but foo.c. We automatically build foo.o because it depends on foo.c via the %.o : %.c rule, and as a side effect, we also build .depend/foo.o.d file; * Then, if any real dependency used to build the existing foo.o that is recorded in .depend/foo.o.d file changes, foo.o gets rebuilt, which would update .depend/foo.o.d again for the next invocation. This is unrelated to the case you mentioned, but I wonder what happens if you did this: * You are on branch 'next', where foo.c includes (perhaps indirectly) frotz.h. Compile and you get foo.o and also the dependency recorded for it, foo.o: foo.c frotz.h, in the .depend/foo.o.d file. * You check out branch 'master', where foo.c does not include frotz.h. Indeed, the include file does not even exist on the branch. Do we get confused, because Makefile includes the depend file from the previous build, finds that you need foo.c and frotz.h up to date in order to get foo.o, but there is no rule to generate frotz.h? No, because the .d files look like this: foo.o: frotz.h frotz.h: So make sees that it can build frotz.h, which of course does nothing. But that's OK, because foo.c doesn't actually include it anymore, and when we recompile it (as we must, since it is different between the two branches), we will rewrite the .d file without frotz.h. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
Junio C Hamano wrote: The only case that worries me is when make or cc gets interrupted. As long as make removes the ultimate target *.o in such a case, it is fine to leave a half-written .depend/foo.o.d (or getting it removed) behind. gcc removes the target .o in its signal handler in such a case. In cases where it doesn't get a chance to (e.g., sudden power failure), there is a partially written .o file already in place, the linker produces errors, and the operator is convinced to run make clean, all without .depend's help. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: Re: Re: Segmentation fault with latest git (070c57df)
Jonathan Nieder wrote: Jongman Heo wrote: But it doesn't stimulate any prerequisites in make, which is weird. What's in builtin/.depend/fetch.o.d? [...] please see below~. $ cat builtin/.depend/fetch.o.d fetch.o: builtin/fetch.c cache.h git-compat-util.h compat/bswap.h \ That's the problem. See the following thread: http://thread.gmane.org/gmane.comp.version-control.git/185625/focus=185680 Currently when COMPUTE_HEADER_DEPENDENCIES=auto git tests for dependency generation support by checking the output and exit status from the following command: $(CC) $(ALL_CFLAGS) -c -MF /dev/null -MMD -MP \ -x c /dev/null -o /dev/null 21 Perhaps this can be improved? Even something as simple as a ccache version test could presumably help a lot. Hope that helps, Jonathan Hi, Unfortunately, the patch didn't help to me. Anyway, ccache is the culprit (I'm using ccache 2.4 version). If I disable ccache using CCACHE_DISABLE=1, then the issue doesn't happen. Thanks. Best regards, Jongman Heo.
Re: Segmentation fault with latest git (070c57df)
Jongman Heo wrote: Unfortunately, the patch didn't help to me. Thanks for testing. Did you apply the patch to the older version of git that generates builtin/.depend/fetch.o.d or the newer version that consumes it? Curious, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Segmentation fault with latest git (070c57df)
Jonathan Nieder wrote: Jongman Heo wrote: Unfortunately, the patch didn't help to me. Thanks for testing. Did you apply the patch to the older version of git that generates builtin/.depend/fetch.o.d or the newer version that consumes it? Curious, Jonathan Hi, Jonathan, I applied the patch to newer version. This time, I tried to apply the patch to older version of Makefile, and now the issue is fixed~. Thanks~! Best regards, Jongman Heo
Re: Segmentation fault with latest git (070c57df)
Jongman Heo jongman@samsung.com writes: Jonathan Nieder wrote: Jongman Heo wrote: Unfortunately, the patch didn't help to me. Thanks for testing. Did you apply the patch to the older version of git that generates builtin/.depend/fetch.o.d or the newer version that consumes it? Curious, Jonathan Hi, Jonathan, I applied the patch to newer version. This time, I tried to apply the patch to older version of Makefile, and now the issue is fixed~. Thanks~! Yeah, that result is understandable, as .depend/*.o.d files will not be rebuilt when the rules to build them changes in the Makefile. Applying the patch to the Makefile in the pristine old tree, run the build (which will generate .depend/*.o.d files with the corrected rules), then checking out the new tree and running the build again without make clean, with or with the patch applied, would validate that the patch fixes the issue for old ccache. Thanks Jonathan for diagnosing, fixing, and thanks Jongman for testing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Segmentation fault with latest git (070c57df)
Junio C Hamanogits...@pobox.com wrote : 허종만 writes: But usually when I build upstream Linux kernel, I don't do make clean after git pull.. I didn't expect that I needed make clean for git build. We don't expect anybody need make clean, either. There is something wrong in the dependency. Hi all, I can reproduce the issue in my machine (RedHat Enterprise 5, x86 PAE) as follows. But in my different machine (Fedora 16 x86) I can't reproduce. $ git reset --hard v1.8.1 # back to v1.8.1 $ make clean $ make all install # this git works fine $ git pull # top commit 9a6c84e6, Merge git://ozlabs.org/~paulus/gitk $ make all install $ git fetch # this git segfaults Segmentation fault So if there is any patch to test, just let me know (but will not available during weekend). Regards, Jongman Heo
Re: Re: Segmentation fault with latest git (070c57df)
On Fri, Feb 01, 2013 at 09:14:41AM +, Jongman Heo wrote: I can reproduce the issue in my machine (RedHat Enterprise 5, x86 PAE) as follows. Great, thanks for taking the time to reproduce. But in my different machine (Fedora 16 x86) I can't reproduce. That makes me wonder if it is related to the gcc or make version. I couldn't reproduce the problem on my gcc-4.1 system, though. My make is: $ make --version GNU Make 3.81 [...] $ git reset --hard v1.8.1 # back to v1.8.1 $ make clean $ make all install # this git works fine After this step, what does builtin/.depend/fetch.o.d contain? It should show a dependency of builtin/fetch.o on string-list (among other things). $ git pull # top commit 9a6c84e6, Merge git://ozlabs.org/~paulus/gitk $ make all install Can you try running make -d builtin/fetch.o here instead of make all install? Can you confirm that it reads builtin/.depend/fetch.o, and that fetch.o gets rebuilt (you should even be able to see the list of newer than dependencies in the debug output)? Another thing to double-check: does it work if you instead run make all install COMPUTE_HEADER_DEPENDENCIES=no ? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: Segmentation fault with latest git (070c57df)
On Fri, Feb 01, 2013 at 10:30:24AM +, Jongman Heo wrote: Short answer; * Version of make is 3.81 on both machines * builtin/fetch.o is not rebuilt (see entire log below) * git works fine with make all install COMPUTE_HEADER_DEPENDENCIES=no OK, that gets us closer. It's definitely a problem with the automatic header dependencies, then. [...make debug output...] Reading makefile `builtin/.depend/fetch.o.d' (search path) (no ~ expansion)... So we definitely have the dep file... [...] Finished prerequisites of target file `builtin/fetch.o'. Prerequisite `builtin/fetch.c' is older than target `builtin/fetch.o'. Prerequisite `GIT-CFLAGS' is older than target `builtin/fetch.o'. No need to remake target `builtin/fetch.o'. But it doesn't stimulate any prerequisites in make, which is weird. What's in builtin/.depend/fetch.o.d? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: Re: Segmentation fault with latest git (070c57df)
[...] Finished prerequisites of target file `builtin/fetch.o'. Prerequisite `builtin/fetch.c' is older than target `builtin/fetch.o'. Prerequisite `GIT-CFLAGS' is older than target `builtin/fetch.o'. No need to remake target `builtin/fetch.o'. But it doesn't stimulate any prerequisites in make, which is weird. What's in builtin/.depend/fetch.o.d? -Peff Hi, please see below~. $ cat builtin/.depend/fetch.o.d fetch.o: builtin/fetch.c cache.h git-compat-util.h compat/bswap.h \ strbuf.h hash.h advice.h gettext.h convert.h refs.h commit.h object.h \ tree.h decorate.h builtin.h cache.h commit.h notes.h string-list.h \ string-list.h remote.h transport.h remote.h run-command.h \ parse-options.h sigchain.h submodule.h connected.h argv-array.h cache.h: git-compat-util.h: compat/bswap.h: strbuf.h: hash.h: advice.h: gettext.h: convert.h: refs.h: commit.h: object.h: tree.h: decorate.h: builtin.h: cache.h: commit.h: notes.h: string-list.h: string-list.h: remote.h: transport.h: remote.h: run-command.h: parse-options.h: sigchain.h: submodule.h: connected.h: argv-array.h:
Re: Re: Re: Re: Segmentation fault with latest git (070c57df)
Jongman Heo wrote: But it doesn't stimulate any prerequisites in make, which is weird. What's in builtin/.depend/fetch.o.d? [...] please see below~. $ cat builtin/.depend/fetch.o.d fetch.o: builtin/fetch.c cache.h git-compat-util.h compat/bswap.h \ That's the problem. See the following thread: http://thread.gmane.org/gmane.comp.version-control.git/185625/focus=185680 Currently when COMPUTE_HEADER_DEPENDENCIES=auto git tests for dependency generation support by checking the output and exit status from the following command: $(CC) $(ALL_CFLAGS) -c -MF /dev/null -MMD -MP \ -x c /dev/null -o /dev/null 21 Perhaps this can be improved? Even something as simple as a ccache version test could presumably help a lot. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: Segmentation fault with latest git (070c57df)
It's almost like the compiler is getting the initializer wrong. It's a long shot, but I wonder if the presence of the bitfield could be triggering a compiler bug (or there is a subtle C rule about bitfield initializations that I do not know). Just for the sake of my sanity, what does the following program output for you? Hi, just cmp is 0 is printed. $ gcc --version gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48) Copyright (C) 2006 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Best regards, Jongman Heo.
Re: Segmentation fault with latest git (070c57df)
Jeff King p...@peff.net writes: On Thu, Jan 31, 2013 at 07:27:04AM +, Jongman Heo wrote: FYI, gdb backtrace and valgrind output attached below, Thanks. Thanks, that's helpful. #4 0x0812bda0 in string_list_insert (list=0xbfffe7c0, string=0x821ec3c refs/remotes/origin/HEAD) at string-list.c:57 #5 0x08071838 in add_existing (refname=0x821ec3c refs/remotes/origin/HEAD, sha1=0x821ec14 \a\fW\337B\352N\255\314C\320Em\021E`\022C, incomplete sequence \303, flag=1, cbdata=0xbfffe7c0) at builtin/fetch.c:570 So we are inserting the string from add_existing, which gets the list from a callback parameter. Which comes from... #13 0x0807390a in do_fetch (remote=value optimized out, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:699 ...here, which does this: struct string_list existing_refs = STRING_LIST_INIT_NODUP; [...] for_each_ref(add_existing, existing_refs); And yet we get: ==2195== Conditional jump or move depends on uninitialised value(s) ==2195==at 0x812B41F: get_entry_index (string-list.c:10) ==2195==by 0x812BD5F: string_list_insert_at_index (string-list.c:33) ==2195==by 0x812BD9F: string_list_insert (string-list.c:57) ==2195==by 0x8071837: add_existing (fetch.c:570) ==2195==by 0x810AF96: do_one_ref (refs.c:525) ==2195==by 0x810BB20: do_for_each_ref_in_dir (refs.c:551) ==2195==by 0x810BD34: do_for_each_ref_in_dirs (refs.c:623) ==2195==by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597) ==2195==by 0x810C303: do_for_each_ref (refs.c:1295) ==2195==by 0x810C63A: for_each_ref (refs.c:1343) ==2195==by 0x8073909: fetch_one (fetch.c:699) ==2195==by 0x8074250: cmd_fetch (fetch.c:992) which seems odd. cmp should be initialized to NULL, and then we never touch it (and even if we did, it wouldn't be unitialized, but rather have the value we put in it). It's almost like the compiler is getting the initializer wrong. It's a long shot, but I wonder if the presence of the bitfield could be triggering a compiler bug (or there is a subtle C rule about bitfield initializations that I do not know). Just for the sake of my sanity, what does the following program output for you? -- 8 -- #include stdio.h #include stdlib.h typedef int (*compare_fn)(const char *, const char *); struct foo { char **items; unsigned int nr, alloc; unsigned int bitfield:1; compare_fn cmp; }; int main(void) { struct foo f = { NULL, 0, 0, 0 }; printf(cmp is %lu\n, (unsigned long)f.cmp); return 0; } I doubt that would help because that stack region would be 0 anyway due to kernel initialization of new pages. You'd have to somehow trample over it first, like below. Or perhaps something in the build process went wrong, and fetch.c didn't get the memo about the new field in the struct. Depending on stack layout, the next variable might be the 'int i' right before the 'string_list list' in the code, which could explain the value of 1. 8 #include stdio.h #include stdlib.h #include string.h typedef int (*compare_fn)(const char *, const char *); struct foo { char **items; unsigned int nr, alloc; unsigned int bitfield:1; compare_fn cmp; }; void scramble() { char foo[256]; memset(foo, 0x42, 256); } void init() { struct foo f = { NULL, 0, 0, 0 }; printf(cmp is %lu\n, (unsigned long)f.cmp); } int main(void) { scramble(); init(); return 0; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Thu, Jan 31, 2013 at 09:42:07AM +0100, Thomas Rast wrote: int main(void) { struct foo f = { NULL, 0, 0, 0 }; printf(cmp is %lu\n, (unsigned long)f.cmp); return 0; } I doubt that would help because that stack region would be 0 anyway due to kernel initialization of new pages. You'd have to somehow trample over it first, like below. Good point. Unfortunately, I can't get either yours or mine to fail, neither with a recent version of gcc nor with gcc-4.1. But I can't convince git to fail, either. The only gcc-4.1 I have is Debian's 4.1.3 release, which is not quite what the OP has. Or perhaps something in the build process went wrong, and fetch.c didn't get the memo about the new field in the struct. Depending on stack layout, the next variable might be the 'int i' right before the 'string_list list' in the code, which could explain the value of 1. Yeah, that would make sense to me with respect to the behavior we are seeing, but that part of the Makefile should be pretty simple and bug-free, I'd think (and from the original report, it seems like he was able to reproduce it well enough to bisect). Still, trying a make clean make might be worth it just to rule that out. Puzzled... -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Segmentation fault with latest git (070c57df)
[snip] Good point. Unfortunately, I can't get either yours or mine to fail, neither with a recent version of gcc nor with gcc-4.1. But I can't convince git to fail, either. The only gcc-4.1 I have is Debian's 4.1.3 release, which is not quite what the OP has. Or perhaps something in the build process went wrong, and fetch.c didn't get the memo about the new field in the struct. Depending on stack layout, the next variable might be the 'int i' right before the 'string_list list' in the code, which could explain the value of 1. Yeah, that would make sense to me with respect to the behavior we are seeing, but that part of the Makefile should be pretty simple and bug-free, I'd think (and from the original report, it seems like he was able to reproduce it well enough to bisect). Still, trying a make clean make might be worth it just to rule that out. Puzzled... -Peff Hi, all, Thomas's test code also returns cmp is 0. But make clean make fixes my issue. Sorry for the noise I made. But usually when I build upstream Linux kernel, I don't do make clean after git pull.. I didn't expect that I needed make clean for git build. Thanks you guys. Best regards, Jongman Heo.N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴젇碼�썳變}찠꼿쟺�j:+v돣�쳭喩zZ+�+zf"톒쉱�~넮녬i鎬z�췿ⅱ�?솳鈺��)刪f
Re: Segmentation fault with latest git (070c57df)
허종만 jongman@samsung.com writes: But usually when I build upstream Linux kernel, I don't do make clean after git pull.. I didn't expect that I needed make clean for git build. We don't expect anybody need make clean, either. There is something wrong in the dependency. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Thu, Jan 31, 2013 at 05:40:02PM -0800, Junio C Hamano wrote: 허종만 jongman@samsung.com writes: But usually when I build upstream Linux kernel, I don't do make clean after git pull.. I didn't expect that I needed make clean for git build. We don't expect anybody need make clean, either. There is something wrong in the dependency. Agreed, but I cannot see what. If auto-header-dependencies is on, gcc should find it (it is not even a recursive dependency for builtin/fetch.c). And if it is not on, we should rebuild based on LIB_H, which includes string-list.h (and always has, as far as I can tell). Hmm. I do notice one oddity with the computed header dependencies, though. We build the computed dependency files as a side effect of doing the actual compilation. So before we have run the compilation once, we need some way to say you _must_ build this, because we do even know the correct dependencies. And to do that, we have each object file depend on any missing .depend dirs, which bootstraps the whole process: we build everything the first time because .depend is missing, and from then on, we use the correct dependencies. But that is not quite right. The .depend directory might exist, but be missing the actual dependency file for a particular object. So if I do: $ make ;# builds all objects and dependency files $ rm builtin/.depend/fetch.o.d $ touch string-list.h $ make we will fail to rebuild builtin/fetch.o properly. It does not see the dependency on string-list (because we have no .d file), nor does it realize that it needs to build the .d file (because it only checks that builtin/.depend exists). It seems like building each object file should depend on its dependency file (but only when COMPUTE_HEADER_DEPENDENCIES is on, of course), since otherwise we cannot know if we have the right dependencies or not. Something like this almost works, I think: diff --git a/Makefile b/Makefile index 6b42f66..a329736 100644 --- a/Makefile +++ b/Makefile @@ -1843,8 +1843,8 @@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) @mkdir -p $@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) -dep_file = $(dir $@).depend/$(notdir $@).d -dep_args = -MF $(dep_file) -MMD -MP +dep_file = $(dir $1).depend/$(notdir $1).d +dep_args = -MF $(call dep_file, $@) -MMD -MP ifdef CHECK_HEADER_DEPENDENCIES $(error cannot compute header dependencies outside a normal build. \ Please unset CHECK_HEADER_DEPENDENCIES and try again) @@ -1909,9 +1909,9 @@ $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) endif ifndef CHECK_HEADER_DEPENDENCIES -$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) +$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(call dep_file, %.o) $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $ -$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) +$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(call dep_file,%.o) $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $ endif But not quite. The problem is that we put the dep file for foo/bar.o into foo/.depend/bar.o. But when we call the dep_file function for the dependency, it sees only %.o, not foo/bar.o, so it can't properly split it apart. I don't think there is a way to force expansion before calling the function. And of course I have no idea if this was the problem that we saw, anyway. I have no idea how one would get into this situation short of manually removing the dependency file. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Fri, Feb 01, 2013 at 01:36:38AM -0500, Jeff King wrote: It seems like building each object file should depend on its dependency file (but only when COMPUTE_HEADER_DEPENDENCIES is on, of course), since otherwise we cannot know if we have the right dependencies or not. Something like this almost works, I think: [...] +$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(call dep_file, %.o) Actually that would not work, as we do not have a rule to create .depend/foo.o.d. We can add one, but it gets pretty hairy (and replicates much of the normal build rule). A much simpler way is to just find the missing dep files and force compilation of their matching objects. Like: diff --git a/Makefile b/Makefile index 6b42f66..f94e8b9 100644 --- a/Makefile +++ b/Makefile @@ -1843,8 +1843,14 @@ dep_args = -MF $(dep_file) -MMD -MP @mkdir -p $@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) +missing_dep_files := $(filter-out $(wildcard $(dep_files)),$(dep_files)) +# we want to rewrite foo/.depend/bar.o.d into foo/bar.o, but +# make's patsubst is not powerful enough to remove something from the middle of +# a string. Hack around it by shelling out. +obj_files_with_missing_deps := $(shell echo $(missing_dep_files:.d=) | tr ' ' '\n' | sed 's,.depend/,,') dep_file = $(dir $@).depend/$(notdir $@).d dep_args = -MF $(dep_file) -MMD -MP +$(obj_files_with_missing_deps): FORCE ifdef CHECK_HEADER_DEPENDENCIES $(error cannot compute header dependencies outside a normal build. \ Please unset CHECK_HEADER_DEPENDENCIES and try again) which does solve the problem, but that shell hack is nasty. It would be much simpler if we stored the dependency for foo/bar.o as .depend/foo/bar.o.d, rather than foo/.depend/bar.o.d, as then we would patsubst it away. Or maybe there is some clever way to convince make to do what I want here. Suggestions welcome. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
On Fri, Feb 01, 2013 at 01:36:38AM -0500, Jeff King wrote: On Thu, Jan 31, 2013 at 05:40:02PM -0800, Junio C Hamano wrote: 허종만 jongman@samsung.com writes: But usually when I build upstream Linux kernel, I don't do make clean after git pull.. I didn't expect that I needed make clean for git build. We don't expect anybody need make clean, either. There is something wrong in the dependency. Agreed, but I cannot see what. If auto-header-dependencies is on, gcc should find it (it is not even a recursive dependency for builtin/fetch.c). And if it is not on, we should rebuild based on LIB_H, which includes string-list.h (and always has, as far as I can tell). By the way, while researching this issue, I noticed this: -- 8 -- Subject: [PATCH] Makefile: add version.h to LIB_H This was forgotten when the file was added by 816fb46, and not noticed because most developers are on modern systems that support COMPUTE_HEADER_DEPENDENCIES. However, people still relying on LIB_H for dependencies might have failed to recompile when this file changed. Found with make CHECK_HEADER_DEPENDENCIES=yes. Signed-off-by: Jeff King p...@peff.net --- I don't see how this could have caused the issue at hand, but it is good to fix nonetheless. I almost wonder if LIB_H should just be set to $(wildcard *.h) or similar, since that is what ends up going into it. And then we would not have to deal with manually keeping it up to date. Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 731b6a8..6b42f66 100644 --- a/Makefile +++ b/Makefile @@ -704,6 +704,7 @@ LIB_H += varint.h LIB_H += userdiff.h LIB_H += utf8.h LIB_H += varint.h +LIB_H += version.h LIB_H += walker.h LIB_H += wildmatch.h LIB_H += wt-status.h -- 1.8.1.2.11.g1a2f572 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Segmentation fault with latest git (070c57df)
Hi all, Looks like following commit causes a segmentation fault in my machine (when running git pull or git fetch); commit 8dd5afc926acb9829ebf56e9b78826a5242cd638 Author: Junio C Hamano gits...@pobox.com Date: Mon Jan 7 12:24:55 2013 -0800 string-list: allow case-insensitive string list In my case, list-cmp (at get_entry_index() function) has an invalid address, obviously not an address of string comparision function, instead it points to 1. Regards, Jongman Heo.N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴젇碼�썳變}찠꼿쟺�j:+v돣�쳭喩zZ+�+zf"톒쉱�~넮녬i鎬z�췿ⅱ�?솳鈺��)刪f
Re: Segmentation fault with latest git (070c57df)
On Thu, Jan 31, 2013 at 01:35:21AM +, Jongman Heo wrote: Looks like following commit causes a segmentation fault in my machine (when running git pull or git fetch); commit 8dd5afc926acb9829ebf56e9b78826a5242cd638 Author: Junio C Hamano gits...@pobox.com Date: Mon Jan 7 12:24:55 2013 -0800 string-list: allow case-insensitive string list In my case, list-cmp (at get_entry_index() function) has an invalid address, obviously not an address of string comparision function, instead it points to 1. Can you show us a stack trace? The string-list functions are generic and get called in a lot of places. It would be useful to know which list is causing the problem. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
In clean.c we have a string_list created on the stack with STRING_LIST_INIT_NODUP (there are probably others, I stopped at the first occurrence). But, STRING_LIST_INIT_NODUP doesn't init the list-cmp pointer which can thus be random. I don't have much time to provide a patch right now (have to go to work), but will tonight if still needed. Antoine, On Thu, Jan 31, 2013 at 7:49 AM, Jeff King p...@peff.net wrote: On Thu, Jan 31, 2013 at 01:35:21AM +, Jongman Heo wrote: Looks like following commit causes a segmentation fault in my machine (when running git pull or git fetch); commit 8dd5afc926acb9829ebf56e9b78826a5242cd638 Author: Junio C Hamano gits...@pobox.com Date: Mon Jan 7 12:24:55 2013 -0800 string-list: allow case-insensitive string list In my case, list-cmp (at get_entry_index() function) has an invalid address, obviously not an address of string comparision function, instead it points to 1. Can you show us a stack trace? The string-list functions are generic and get called in a lot of places. It would be useful to know which list is causing the problem. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Segmentation fault with latest git (070c57df)
On Thu, Jan 31, 2013 at 7:49 AM, Jeff King wrote: On Thu, Jan 31, 2013 at 01:35:21AM +, Jongman Heo wrote: Looks like following commit causes a segmentation fault in my machine (when running git pull or git fetch); commit 8dd5afc926acb9829ebf56e9b78826a5242cd638 Author: Junio C Hamano Date: Mon Jan 7 12:24:55 2013 -0800 string-list: allow case-insensitive string list In my case, list-cmp (at get_entry_index() function) has an invalid address, obviously not an address of string comparision function, instead it points to 1. Can you show us a stack trace? The string-list functions are generic and get called in a lot of places. It would be useful to know which list is causing the problem. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Hi, FYI, gdb backtrace and valgrind output attached below, Thanks. (gdb) run fetch Starting program: /home/hjongman/repos/git/git fetch warning: .dynamic section for /lib/libc.so.6 is not at the expected address warning: difference appears to be caused by prelink, adjusting expectations [Thread debugging using libthread_db enabled] Program received signal SIGSEGV, Segmentation fault. 0x0001 in ?? () (gdb) bt #0 0x0001 in ?? () #1 0x0812b457 in get_entry_index (list=0xbfffe7c0, string=0x821ec3c refs/remotes/origin/HEAD, exact_match=0xbfffe568) at string-list.c:14 #2 0x0812bd60 in add_entry (list=0xbfffe7c0, insert_at=-1, string=0x821ec3c refs/remotes/origin/HEAD) at string-list.c:33 #3 string_list_insert_at_index (list=0xbfffe7c0, insert_at=-1, string=0x821ec3c refs/remotes/origin/HEAD) at string-list.c:63 #4 0x0812bda0 in string_list_insert (list=0xbfffe7c0, string=0x821ec3c refs/remotes/origin/HEAD) at string-list.c:57 #5 0x08071838 in add_existing (refname=0x821ec3c refs/remotes/origin/HEAD, sha1=0x821ec14 \a\fW\337B\352N\255\314C\320Em\021E`\022C, incomplete sequence \303, flag=1, cbdata=0xbfffe7c0) at builtin/fetch.c:570 #6 0x0810af97 in do_one_ref (base=value optimized out, fn=0x8071820 add_existing, trim=0, flags=value optimized out, cb_data=0xbfffe7c0, entry=0x821ec10) at refs.c:525 #7 0x0810bd9f in do_for_each_ref_in_dirs (dir1=0x8215d54, dir2=0x821ea44, base=0x814f9ff , fn=0x8071820 add_existing, trim=0, flags=0, cb_data=0xbfffe7c0) at refs.c:627 #8 0x0810bc8e in do_for_each_ref_in_dirs (dir1=0x8215cac, dir2=0x8226954, base=0x814f9ff , fn=0x8071820 add_existing, trim=0, flags=0, cb_data=0xbfffe7c0) at refs.c:597 #9 0x0810bc8e in do_for_each_ref_in_dirs (dir1=0x8215c0c, dir2=0x8215a54, base=0x814f9ff , fn=0x8071820 add_existing, trim=0, flags=0, cb_data=0xbfffe7c0) at refs.c:597 #10 0x0810bc8e in do_for_each_ref_in_dirs (dir1=0x8215a1c, dir2=0x821862c, base=0x814f9ff , fn=0x8071820 add_existing, trim=0, flags=0, cb_data=0xbfffe7c0) at refs.c:597 #11 0x0810c304 in do_for_each_ref (submodule=value optimized out, base=0x814f9ff , fn=0x8071820 add_existing, trim=0, flags=0, cb_data=0xbfffe7c0) at refs.c:1295 #12 0x0810c63b in for_each_ref (fn=0x8071820 add_existing, cb_data=0xbfffe7c0) at refs.c:1343 #13 0x0807390a in do_fetch (remote=value optimized out, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:699 #14 fetch_one (remote=value optimized out, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:949 #15 0x08074251 in cmd_fetch (argc=1, argv=0xbfffe9f8, prefix=0x0) at builtin/fetch.c:992 #16 0x0804b60b in run_builtin (argc=1, argv=0xbfffe9f8) at git.c:281 #17 handle_internal_command (argc=1, argv=0xbfffe9f8) at git.c:443 #18 0x0804ba51 in run_argv (argc=1, argv=0xbfffe9f8) at git.c:489 #19 main (argc=1, argv=0xbfffe9f8) at git.c:564 $ valgrind git fetch ==2195== Memcheck, a memory error detector ==2195== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al. ==2195== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info ==2195== Command: git fetch ==2195== ==2195== Conditional jump or move depends on uninitialised value(s) ==2195==at 0x812B41F: get_entry_index (string-list.c:10) ==2195==by 0x812BD5F: string_list_insert_at_index (string-list.c:33) ==2195==by 0x812BD9F: string_list_insert (string-list.c:57) ==2195==by 0x8071837: add_existing (fetch.c:570) ==2195==by 0x810AF96: do_one_ref (refs.c:525) ==2195==by 0x810BB20: do_for_each_ref_in_dir (refs.c:551) ==2195==by 0x810BD34: do_for_each_ref_in_dirs (refs.c:623) ==2195==by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597) ==2195==by 0x810C303: do_for_each_ref (refs.c:1295) ==2195==by 0x810C63A: for_each_ref (refs.c:1343) ==2195==by 0x8073909: fetch_one (fetch.c:699) ==2195==by 0x8074250: cmd_fetch (fetch.c:992) ==2195== ==2195== Use of uninitialised value of size 4 ==2195==at 0x812B454: get_entry_index (string-list.c:14) ==2195==by 0x812BD5F: string_list_insert_at_index (string-list.c:33) ==2195==
Re: Re: Segmentation fault with latest git (070c57df)
On Thu, Jan 31, 2013 at 07:27:04AM +, Jongman Heo wrote: FYI, gdb backtrace and valgrind output attached below, Thanks. Thanks, that's helpful. #4 0x0812bda0 in string_list_insert (list=0xbfffe7c0, string=0x821ec3c refs/remotes/origin/HEAD) at string-list.c:57 #5 0x08071838 in add_existing (refname=0x821ec3c refs/remotes/origin/HEAD, sha1=0x821ec14 \a\fW\337B\352N\255\314C\320Em\021E`\022C, incomplete sequence \303, flag=1, cbdata=0xbfffe7c0) at builtin/fetch.c:570 So we are inserting the string from add_existing, which gets the list from a callback parameter. Which comes from... #13 0x0807390a in do_fetch (remote=value optimized out, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:699 ...here, which does this: struct string_list existing_refs = STRING_LIST_INIT_NODUP; [...] for_each_ref(add_existing, existing_refs); And yet we get: ==2195== Conditional jump or move depends on uninitialised value(s) ==2195==at 0x812B41F: get_entry_index (string-list.c:10) ==2195==by 0x812BD5F: string_list_insert_at_index (string-list.c:33) ==2195==by 0x812BD9F: string_list_insert (string-list.c:57) ==2195==by 0x8071837: add_existing (fetch.c:570) ==2195==by 0x810AF96: do_one_ref (refs.c:525) ==2195==by 0x810BB20: do_for_each_ref_in_dir (refs.c:551) ==2195==by 0x810BD34: do_for_each_ref_in_dirs (refs.c:623) ==2195==by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597) ==2195==by 0x810C303: do_for_each_ref (refs.c:1295) ==2195==by 0x810C63A: for_each_ref (refs.c:1343) ==2195==by 0x8073909: fetch_one (fetch.c:699) ==2195==by 0x8074250: cmd_fetch (fetch.c:992) which seems odd. cmp should be initialized to NULL, and then we never touch it (and even if we did, it wouldn't be unitialized, but rather have the value we put in it). It's almost like the compiler is getting the initializer wrong. It's a long shot, but I wonder if the presence of the bitfield could be triggering a compiler bug (or there is a subtle C rule about bitfield initializations that I do not know). Just for the sake of my sanity, what does the following program output for you? -- 8 -- #include stdio.h #include stdlib.h typedef int (*compare_fn)(const char *, const char *); struct foo { char **items; unsigned int nr, alloc; unsigned int bitfield:1; compare_fn cmp; }; int main(void) { struct foo f = { NULL, 0, 0, 0 }; printf(cmp is %lu\n, (unsigned long)f.cmp); return 0; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html