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