Segmentation fault with latest git (070c57df)

Segmentation fault with latest git (070c57df)

[Jongman Heo]

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 <gitster@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.

Re: Segmentation fault with latest git (070c57df)

[Jeff King]

On Thu, Jan 31, 2013 at 01:35:21AM +0000, 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 <gitster@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

Re: Segmentation fault with latest git (070c57df)

[Antoine Pelisse]

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 <peff@peff.net> wrote:
> On Thu, Jan 31, 2013 at 01:35:21AM +0000, 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 <gitster@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: Segmentation fault with latest git (070c57df)

[Jongman Heo]

On Thu, Jan 31, 2013 at 7:49 AM, Jeff King wrote:
> On Thu, Jan 31, 2013 at 01:35:21AM +0000, 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 majordomo@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.
0x00000001 in ?? ()
(gdb) bt
#0  0x00000001 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==    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 0x810BD9E: do_for_each_ref_in_dirs (refs.c:627)
==2195==    by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195==    by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==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== 
==2195== 
==2195== Process terminating with default action of signal 11 (SIGSEGV)
==2195==  Access not within mapped region at address 0x0
==2195==    at 0x1: ???
==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 0x810BD9E: do_for_each_ref_in_dirs (refs.c:627)
==2195==    by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195==    by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==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==  If you believe this happened as a result of a stack
==2195==  overflow in your program's main thread (unlikely but
==2195==  possible), you can try to increase the size of the
==2195==  main thread stack using the --main-stacksize= flag.
==2195==  The main thread stack size used in this run was 10485760.
==2195== 
==2195== HEAP SUMMARY:
==2195==     in use at exit: 325,604 bytes in 3,579 blocks
==2195==   total heap usage: 3,687 allocs, 108 frees, 518,058 bytes allocated
==2195== 
==2195== LEAK SUMMARY:
==2195==    definitely lost: 72 bytes in 2 blocks
==2195==    indirectly lost: 0 bytes in 0 blocks
==2195==      possibly lost: 31,347 bytes in 460 blocks
==2195==    still reachable: 294,185 bytes in 3,117 blocks
==2195==         suppressed: 0 bytes in 0 blocks
==2195== Rerun with --leak-check=full to see details of leaked memory
==2195== 
==2195== For counts of detected and suppressed errors, rerun with: -v
==2195== Use --track-origins=yes to see where uninitialised values come from
==2195== ERROR SUMMARY: 3 errors from 2 contexts (suppressed: 17 from 8)
Segmentation fault

Re: Segmentation fault with latest git (070c57df)

[Jeff King]

On Thu, Jan 31, 2013 at 08:02:06AM +0100, Antoine Pelisse wrote:

> 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 think that is the problem. Extra struct members that are not
mentioned by an initializer will get set to 0 or NULL.

A code path that tried to initialize each member individually would run
into problems, but I could not find any such code path (all of them
either use an initializer, are static, or memset the struct to
all-zeroes).

-Peff

Re: Re: Segmentation fault with latest git (070c57df)

[Jeff King]

On Thu, Jan 31, 2013 at 07:27:04AM +0000, 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;
}

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.

Re: Re: Segmentation fault with latest git (070c57df)

[Jongman Heo]

Junio C Hamano<gitster@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)

[Jeff King]

On Fri, Feb 01, 2013 at 09:14:41AM +0000, 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

Re: Re: Segmentation fault with latest git (070c57df)

[Jongman Heo]

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)

[Junio C Hamano]

Jongman Heo <jongman.heo@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.

Re: Segmentation fault with latest git (070c57df)

[Jeff King]

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

Re: Segmentation fault with latest git (070c57df)

[Junio C Hamano]

Jeff King <peff@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...

Re: Segmentation fault with latest git (070c57df)

[Junio C Hamano]

Junio C Hamano <gitster@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?

Re: Segmentation fault with latest git (070c57df)

[Jeff King]

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

Re: Segmentation fault with latest git (070c57df)

[Jeff King]

On Mon, Feb 04, 2013 at 01:29:41AM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@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

Re: Segmentation fault with latest git (070c57df)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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.

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.

How expensive to generate the dependency for a single foo.c file
into a temporary without compiling it into foo.o, and comparing the
resulting dependency with existing .depend/foo.o.d?  If it is very
cheap, it might be worth doing it before linking the final build
product and error the build out when we see discrepancies.  I dunno.

Re: Segmentation fault with latest git (070c57df)

[Jonathan Nieder]

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.

Re: Segmentation fault with latest git (070c57df)

[sam]

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  0x00007f06ba7c28c6 in __GI___strdup (s=0x0) at strdup.c:41
#2  0x00000000004f4449 in ?? ()
#3  0x000000000040eab0 in ?? ()
#4  0x000000000040f6f5 in ?? ()
#5  0x0000000000405a88 in ?? ()
#6  0x0000000000404ee2 in ?? ()
#7  0x00007f06ba75aea5 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  0x0000000000405329 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.

Re: Segmentation fault with latest git (070c57df)

[sam]

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.

Re: Segmentation fault with latest git (070c57df)

[Ramkumar Ramachandra]

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?

Re: Segmentation fault with latest git (070c57df)

[Ramkumar Ramachandra]

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.