Re: [BUG] git is segfaulting, was [PATCH v4 04/12] dir: also check directories for matching pathspecs

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:

quoted

On Wed, Sep 25, 2019 at 01:39:19PM -0700, Denton Liu wrote:

quoted

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.

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!