On Fri, Sep 27, 2019 at 04:17:46AM +0200, SZEDER Gábor wrote:
> On Fri, Sep 27, 2019 at 03:09:30AM +0200, SZEDER Gábor wrote:
> > On Wed, Sep 25, 2019 at 01:39:19PM -0700, Denton Liu wrote:
> > > Hi Elijah,
> > > 
> > > I ran into a segfault on MacOS. I managed to bisect it down to
> > > 404ebceda0 (dir: also check directories for matching pathspecs,
> > > 2019-09-17), which should be the patch in the parent thread. The test
> > > case below works fine without this patch applied but segfaults once it
> > > is applied.
> > > 
> > >   #!/bin/sh
> > > 
> > >   git worktree add testdir
> > >   git -C testdir checkout master
> > >   git -C testdir fetch https://github.com/git/git.git todo
> > >   bin-wrappers/git -C testdir checkout FETCH_HEAD # segfault here
> > > 
> > > Note that the worktree part isn't necessary to reproduce the problem but
> > > I didn't want my files to be constantly refreshed, triggering a rebuild
> > > each time.
> > > 
> > > I also managed to get this backtrace from running lldb at the segfault
> > > but it is based on the latest "jch" commit, 1cc52d20df (Merge branch
> > > 'jt/merge-recursive-symlink-is-not-a-dir-in-way' into jch, 2019-09-20).
> > > 
> > >   * thread #1, queue = 'com.apple.main-thread', stop reason = 
> > > EXC_BAD_ACCESS (code=1, address=0x8)
> > >     * frame #0: 0x00000001000f63a0 
> > > git`do_match_pathspec(istate=0x0000000100299940, ps=0x000000010200aa80, 
> > > name="Gitweb/static/js/lib/", namelen=21, prefix=0, 
> > > seen=0x0000000000000000, flags=0) at dir.c:420:2 [opt]
> > >           frame #1: 0x00000001000f632c 
> > > git`match_pathspec(istate=0x0000000100299940, ps=0x0000000000000000, 
> > > name="Gitweb/static/js/lib/", namelen=21, prefix=0, 
> > > seen=0x0000000000000000, is_dir=0) at dir.c:490:13 [opt]
> > >           frame #2: 0x00000001000f8315 
> > > git`read_directory_recursive(dir=0x00007ffeefbfe278, 
> > > istate=0x0000000100299940, base=<unavailable>, baselen=17, 
> > > untracked=<unavailable>, check_only=0, stop_at_first_file=0, 
> > > pathspec=0x0000000000000000) at dir.c:1990:9 [opt]
> > >           frame #3: 0x00000001000f82e9 
> > > git`read_directory_recursive(dir=0x00007ffeefbfe278, 
> > > istate=0x0000000100299940, base=<unavailable>, baselen=14, 
> > > untracked=<unavailable>, check_only=0, stop_at_first_file=0, 
> > > pathspec=0x0000000000000000) at dir.c:1984:5 [opt]
> > >           frame #4: 0x00000001000f82e9 
> > > git`read_directory_recursive(dir=0x00007ffeefbfe278, 
> > > istate=0x0000000100299940, base=<unavailable>, baselen=7, 
> > > untracked=<unavailable>, check_only=0, stop_at_first_file=0, 
> > > pathspec=0x0000000000000000) at dir.c:1984:5 [opt]
> > >           frame #5: 0x00000001000f60d1 
> > > git`read_directory(dir=0x00007ffeefbfe278, istate=0x0000000100299940, 
> > > path="Gitweb/", len=7, pathspec=0x0000000000000000) at dir.c:2298:3 [opt]
> > >           frame #6: 0x00000001001bded1 
> > > git`verify_clean_subdirectory(ce=<unavailable>, o=0x00007ffeefbfe8c0) at 
> > > unpack-trees.c:1846:6 [opt]
> > >           frame #7: 0x00000001001bdc1d 
> > > git`check_ok_to_remove(name="Gitweb", len=6, dtype=4, 
> > > ce=0x0000000103e70de0, st=0x00007ffeefbfe438, 
> > > error_type=ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o=0x00007ffeefbfe8c0) 
> > > at unpack-trees.c:1901:7 [opt]
> > 
> > That 'name="Gitweb" parameter caught my eye.  origin/todo contains a
> > 'Gitweb' file, with upper case 'G', while master contains a 'gitweb'
> > directory, with lower case 'g'.  
> > 
> > Could it be that case (in)sensitivity plays a crucial rule in
> > triggering the segfault?  FWIW I could reproduce it following Denton's
> > description on Travis CI's macOS VM with the debug shell access, and
> > it uses case insensitive file system.
> 
> Indeed, with 404ebceda0 the test below segfaults on case insensitive
> fs, but not on a case sensitive one.

Wow, good catch. I didn't even notice that in the backtrace.

> 
> 
> diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
> index 192c94eccd..5b405c97d7 100755
> --- a/t/t0050-filesystem.sh
> +++ b/t/t0050-filesystem.sh
> @@ -131,4 +131,27 @@ $test_unicode 'merge (silent unicode normalization)' '
>       git merge topic
>  '
>  
> +test_expect_success CASE_INSENSITIVE_FS "Denton's segfault" '
> +     git init repo &&
> +     (
> +             cd repo &&
> +
> +             echo foo >Gitweb &&
> +             git add Gitweb &&
> +             git commit -m "add Gitweb" &&
> +
> +             git checkout --orphan todo &&
> +             git reset --hard &&
> +             # the subdir is crucial, without it there is no segfault
> +             mkdir -p gitweb/subdir &&
> +             echo bar >gitweb/subdir/file &&
> +             # it is not strictly necessary to add and commit the
> +             # gitweb directory, its presence is sufficient
> +             git add gitweb &&
> +             git commit -m "add gitweb/subdir/file" &&
> +
> +             git checkout master
> +     )
> +'
> +
>  test_done

I can confirm that this test case reproduces for me. Thanks for writing
this.

> 
> 
> 
> The end of its trace:
> 
> ++git checkout master
> ./test-lib.sh: line 910: 11220 Segmentation fault: 11  git checkout master
> error: last command exited with $?=139
> 
> Case insensitivity is important because check_ok_to_remove() is
> invoked from verify_absent_1(), which looks like this:
> 
>   if (...)
>      ....
>   else if (...)
>      ....
>   else if (lstat(ce->name, &st))
>       // That lstat() checked whether 'Gitweb' is absent.  On a case
>       // sensitive fs it's absent, so it returns.  On a case
>       // insensitive fs it finds 'master's 'gitweb' directory, so it
>       // goes on to the else below, and eventually segfaults.
>       return;
>   else
>       check_ok_to_remove()
> 
> 
> Good night :)

Thanks for your help!

Reply via email to