Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Aug 05, 2024 at 01:53:22AM +1000, Mark Jamsek wrote:
> > Updated diff checks the buffer for the terminating newline--not the
> > line returned from getline(3). This fixes an OOB read in the previous
> > diff.
> 
> ok by me too (style: could avoid the reundant == 1 in boolean context)
> 
> > I still want to write a test for this case though.
> 
> Great!

This is the basic test reproduced from naddy's minimal recipe that I've
been using to fix the bug naddy reported. It only covers the trivial
diff3 of a histedit operation involving a malformed text file that lacks
the terminating newline. I'm still trying to find a minimal repro to
cover the path exercised in naddy's llvm case.

But running this test with a binary built from current HEAD versus one
built with the patch sent earlier helps to see how the bug produces
an empty file.


-----------------------------------------------
commit 58b57e0005409b4537def7f52a5dae8fe13dadce (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Mon Aug  5 14:08:38 2024 UTC
 
 basic histedit/diff3 regress for malformed text files
 
 Test case for diff3 handling of text files without a terminating newline.
 This doesn't exercise all paths of the code under test. A minimal recipe
 of ed's c command involving malformed text files is still needed.
 
 Reproduced from naddy's minimal recipe.
 
 M  regress/cmdline/histedit.sh  |  53+  0-

1 file changed, 53 insertions(+), 0 deletions(-)

diff 581ac2a102c104327012657946baa4ad14ac649b 58b57e0005409b4537def7f52a5dae8fe13dadce
commit - 581ac2a102c104327012657946baa4ad14ac649b
commit + 58b57e0005409b4537def7f52a5dae8fe13dadce
blob - c7454905559296ba3c95edb66a8cf17f0700d11a
blob + 9bdde07957ae228f5b729de9b3f01fbce4bc3ac7
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2758,6 +2758,58 @@ EOF
 	test_done "$testroot" "$ret"
 }
 
+test_histedit_no_eof_newline() {
+	local testroot=$(test_init histedit_no_eof_newline)
+	local id_root=$(git_show_head "$testroot/repo")
+
+	got checkout "$testroot/repo" "$testroot/wt" > /dev/null
+
+	echo -n "fubar" >> $testroot/wt/alpha
+	(cd "$testroot/wt" && got commit -m 'no eof newline' > /dev/null)
+	local id_head=$(git_show_head "$testroot/repo")
+
+	cp $testroot/wt/alpha $testroot/content.expected
+
+	(cd $testroot/wt && got update -c :base:- > /dev/null)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "update to $id_root failed" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	cat > $testroot/editor.sh <<EOF
+#!/bin/sh
+ed -s "\$1" <<-EOF
+	,s/ eof / terminating /
+	w
+	EOF
+EOF
+	chmod +x $testroot/editor.sh
+
+	(cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
+	    VISUAL="$testroot/editor.sh" got histedit -m > /dev/null)
+
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "histedit -m failed" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	cp $testroot/wt/alpha $testroot/content
+
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" 0
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -2786,3 +2838,4 @@ run_test test_histedit_umask
 run_test test_histedit_mesg_filemode_change
 run_test test_histedit_drop_only
 run_test test_histedit_conflict_revert
+run_test test_histedit_no_eof_newline


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68