From: Stefan Sperling <stsp@stsp.name>
Subject: Re: segfault when recursively unstaging removed file
To: Johannes Thyssen Tishman <johannes@thyssentishman.com>
Cc: gameoftrees@openbsd.org
Date: Mon, 7 Oct 2024 13:53:03 +0200

On Mon, Oct 07, 2024 at 12:56:25PM +0200, Johannes Thyssen Tishman wrote:
> I know this is a very weird scenario and probably something that
> shouldn't be done. Nevertheless I thought I'd report this. Please find
> the backtrace attached.
> 
> Steps to reproduce:
> 
> mkdir -p foo/bar
> touch foo/bar/file
> got init foobar.git
> got import -r foobar.git -m 'test' foo
> got checkout foobar.git
> cd foobar
> echo 'blah' > bar/file
> got stage bar/file
> mv bar/file bar/new_file
> got unstage bar
> 
> $ got -V
> got 0.104-current


Thanks, the patch below fixes this crash and adds test coverage.


M  lib/worktree.c              |   9+  2-
M  regress/cmdline/unstage.sh  |  69+  0-

2 files changed, 78 insertions(+), 2 deletions(-)

commit - e2308af98f7d01e81f6173b9c264b1c21190a24a
commit + cb41003816bd48d421962c958a4dede3560118a4
blob - 6f1ab86e6bb284e7bd1520334604392068bf8022
blob + 65cddbd86f134477f1ea182a4097014b17686200
--- lib/worktree.c
+++ lib/worktree.c
@@ -3688,7 +3688,7 @@ status_old(void *arg, struct got_fileindex_entry *ie, 
 {
 	const struct got_error *err;
 	struct diff_dir_cb_arg *a = arg;
-	struct got_object_id blob_id, commit_id;
+	struct got_object_id blob_id, commit_id, staged_blob_id;
 	unsigned char status;
 
 	if (a->cancel_cb) {
@@ -3702,12 +3702,13 @@ status_old(void *arg, struct got_fileindex_entry *ie, 
 
 	got_fileindex_entry_get_blob_id(&blob_id, ie);
 	got_fileindex_entry_get_commit_id(&commit_id, ie);
+	got_fileindex_entry_get_staged_blob_id(&staged_blob_id, ie);
 	if (got_fileindex_entry_has_file_on_disk(ie))
 		status = GOT_STATUS_MISSING;
 	else
 		status = GOT_STATUS_DELETE;
 	return (*a->status_cb)(a->status_arg, status, get_staged_status(ie),
-	    ie->path, &blob_id, NULL, &commit_id, -1, NULL);
+	    ie->path, &blob_id, &staged_blob_id, &commit_id, -1, NULL);
 }
 
 static void
@@ -9807,6 +9808,12 @@ unstage_path(void *arg, unsigned char status,
 			break;
 		/* fall through */
 	case GOT_STATUS_ADD:
+		if (status == GOT_STATUS_MISSING) {
+			/* Cannot merge changes into missing files. */
+			err = (*a->progress_cb)(a->progress_arg, status,
+			    relpath);
+			goto done;
+		}
 		if (a->patch_cb) {
 			if (staged_status == GOT_STATUS_ADD) {
 				int choice = GOT_PATCH_CHOICE_NONE;
blob - 5f9b7bb3b3a19f838ac5afc1984ab0cc09919ed9
blob + 797487d03ec67c7d78cb9ee815c911e4a58cbfe6
--- regress/cmdline/unstage.sh
+++ regress/cmdline/unstage.sh
@@ -179,7 +179,76 @@ test_unstage_nonexistent() {
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	cat > $testroot/stdout.expected <<EOF
+ M alpha
+ D beta
+ A foo
+EOF
+	(cd $testroot/wt && got status > $testroot/stdout)
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# removing a staged file from disk and then unstaging
+	# all changes in the work tree would trigger a segfault
+	rm $testroot/wt/alpha
+
+	cat > $testroot/stdout.expected <<EOF
+!M alpha
+ D beta
+ A foo
+EOF
+	(cd $testroot/wt && got status > $testroot/stdout)
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && got unstage > $testroot/stdout)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got unstage command failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	cat > $testroot/stdout.expected <<EOF
+!  alpha
+D  beta
+G  foo
+Files which had incoming changes but could not be found in the work tree: 1
+EOF
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cat > $testroot/stdout.expected <<EOF
+!M alpha
+D  beta
+A  foo
+EOF
+	(cd $testroot/wt && got status > $testroot/stdout)
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+
 	test_done "$testroot" "$ret"
 }