On Wed, Sep 25, 2019 at 02:55:30PM -0700, Denton Liu wrote:
> Looks correct to me. I don't see why this wouldn't reproduce. I'll send
> you more information if I figure anything else out.
I looked into it a little more and I think I know why it's being
triggered.
When we checkout 'todo' from 'master', since they're completely
different trees, all of git's source files need to be removed. As a
result, the checkout process at some point invokes check_ok_to_remove().
This kicks off the following call chain:
check_ok_to_remove()
verify_clean_subdirectory()
read_directory()
read_directory_recursive() (this is called recursively, of course)
match_pathspec()
do_match_pathspec()
Where we segfault in do_match_pathspec() because ps is NULL:
GUARD_PATHSPEC(ps,
PATHSPEC_FROMTOP |
PATHSPEC_MAXDEPTH |
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
PATHSPEC_EXCLUDE |
PATHSPEC_ATTR);
So why is ps == NULL? In verify_clean_subdirectory(), we call
read_directory() like this:
i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
where we explictly pass in a NULL and it is handed down the callstack. I
guess this means that we should be expecting that pathspecs can be NULL
in this path. So I've applied the patch at the bottom and it fixes the
problem.
I was wondering if we should stick a
if (!ps)
BUG("ps is NULL");
into do_match_pathspec(), though, so we can avoid these situations in
the future.
Also, I'm still not sure why the issue wasn't reproducible on your
side... I'm not too familiar with this area of the code, though.
-- >8 --
diff --git a/dir.c b/dir.c
index 76a3c3894b..b7a6de58c6 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,7 +1952,7 @@ static enum path_treatment
read_directory_recursive(struct dir_struct *dir,
if (subdir_state > dir_state)
dir_state = subdir_state;
- if (!match_pathspec(istate, pathspec, path.buf,
path.len,
+ if (pathspec && !match_pathspec(istate, pathspec,
path.buf, path.len,
0 /* prefix */, NULL,
0 /* do NOT special case dirs */))
state = path_none;
