From: Mark Jamsek Subject: Re: got rebase or diff library bug To: Stefan Sperling Cc: Christian Weisgerber , gameoftrees@openbsd.org Date: Tue, 06 Aug 2024 00:14:58 +1000 Stefan Sperling 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 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 < /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 GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68