Re: git write-tree segfault with core.untrackedCache true and nonexistent index

From: Martin Ågren <martin.agren@gmail.com>
Date: Jul 22, 2022, 19:25 UTC
Message-ID: <20220722192559.718264-1-martin.agren@gmail.com>
In-Reply-To: <YtrdPguYs3a3xekv@kitenet.net>

On Fri, 22 Jul 2022 at 20:20, Joey Hess <id@joeyh.name> wrote:

Show 8 quoted lines

>
>         joey@darkstar:/tmp>git init emptyrepo
>         Initialized empty Git repository in /tmp/emptyrepo/.git/
>         joey@darkstar:/tmp>cd emptyrepo/
>         joey@darkstar:/tmp/emptyrepo>git config core.untrackedCache true
>         joey@darkstar:/tmp/emptyrepo>git write-tree
>         Segmentation fault
>

[...]

Show 7 quoted lines

>
> Bisecting, e6a653554bb49c26d105f3b478cbdbb1c0648f65 is the first bad commit
> commit e6a653554bb49c26d105f3b478cbdbb1c0648f65
> Author: Tao Klerks <tao@klerks.biz>
> Date:   Thu Mar 31 16:02:15 2022 +0000
>
>     untracked-cache: support '--untracked-files=all' if configured

Thanks for a clear description, and for bisecting.

`repo` is NULL in `new_untracked_cache_flags()` and we're not prepared for that. The diff below fixes this in the sense that your reproducer stops failing, but I'm not sure it's the best approach.

I can't help but think that e6a653554b was just unlucky enough to dereference `istate->repo` and that the real issue is that we're missing

	if (!istate->repo)
		istate->repo = the_repository;

in some strategic place a fair bit earlier. It seems to me like the diff below is just papering over the real bug. It's not obvious to me where that check would want to go, though. Tao, do you have an idea?

Martin

--- a/dir.c
+++ b/dir.c
@@ -2752,6 +2752,9 @@ static unsigned new_untracked_cache_flags(struct index_state *istate)
 	struct repository *repo = istate->repo;
 	char *val;
 
+	if (!repo)
+		repo = the_repository;
+
 	/*
 	 * This logic is coordinated with the setting of these flags in
 	 * wt-status.c#wt_status_collect_untracked(), and the evaluation

-- 
2.37.1.455.g008518b4e5