Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Mar 2025 23:53:19 -0500
From:      Kyle Evans <kevans@FreeBSD.org>
To:        Jamie Landeg-Jones <jamie@catflap.org>, freebsd-current@FreeBSD.org
Subject:   Re: grep(1) bug - duplicate output lines
Message-ID:  <4e719f32-7be0-46dd-8113-d8bee17a846e@FreeBSD.org>
In-Reply-To: <503a0ad0-7a03-4979-9faf-d220f7b3c3a5@FreeBSD.org>
References:  <202309280240.38S2esgN015958@donotpassgo.dyslexicfish.net> <8d0658e9-8984-a241-ad1d-e5aa1328d7fa@FreeBSD.org> <202309291629.38TGTU56092229@donotpassgo.dyslexicfish.net> <202309291825.38TIPNN2096338@donotpassgo.dyslexicfish.net> <85039cfe-3c59-173d-f2f2-f0ca8e0fa1c8@FreeBSD.org> <883774ff-c630-72bf-ebc3-cc53e4d40d03@FreeBSD.org> <202503120021.52C0LG7D088609@donotpassgo.dyslexicfish.net> <503a0ad0-7a03-4979-9faf-d220f7b3c3a5@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 3/11/25 21:58, Kyle Evans wrote:
> On 3/11/25 19:21, Jamie Landeg-Jones wrote:
>> Kyle Evans <kevans@FreeBSD.org> wrote:
>>
>>> On 9/29/23 15:37, Kyle Evans wrote:
>>>> On 9/29/23 13:25, Jamie Landeg-Jones wrote:
>>>>> Jamie Landeg-Jones <jamie@catflap.org> wrote:
>>>>>
>>>>>> Brilliant! Thanks for the quick response and fix. It works fine 
>>>>>> for me -
>>>>>> I've not managed to break it again :-)
>>>>>
>>>>> Famous last words....
>>>>>
>>>>> "grep -v" now produces duplicate lines! e.g. :
>>>>>
>>>>
>>>> Alright, fine, be that way. :-) Try this on top of the existing patch:
>>>>
>>>> https://people.freebsd.org/~kevans/grep-color.diff
>>>>
>>>
>>> This should be spelled:
>>>
>>> https://people.freebsd.org/~kevans/grep-color-addition.diff
>>>
>>> Sorry
>>
>> Hi Kyle. This is an old thread from 2023.
>> ( https://lists.freebsd.org/archives/freebsd-current/2023- 
>> September/004762.html )
>>
> 
> Yikes, I completely forgot about this.
> 
>> I've been running with these two patches since you posted them. I notice
>> that they haven't been commited, and the bug reported in the thread still
>> exists in current, so I'm replying to the original thread, both in the 
>> hope
>> that this specific problem can be fixed, and then your overall fixes be
>> submitted to the tree.
>>
>> Everything else has worked fine all this time, but today I noticed a bug
>> that can be triggered like this:
>>
>>   | % echo boo | /usr/bin/grep ''
>>   | Assertion failed: (pc->matchidx > 0), function procmatch_match, 
>> file /usr/src/usr.bin/grep/util.c, line 223.
>>   | Abort (core dumped)
>>
>> This is caused by the snippet:
>>
>>   |        /* Print the matching line, but only if not quiet/binary */
>>   |         if (mc->printmatch) {
>>   |                 size_t last_out;
>>   |
>>   |                 if (vflag)
>>   |                         assert(pc->matchidx == 0);
>>   |                 else
>>   |                         assert(pc->matchidx > 0);
>>
>> In this case, pc-matchidx is validly 0. However, simply  Removing
>> the assert from the src causes the duplicate line issue again.
>>
>> Why grep for '' ? Long story, but it seems to be allowed.
>> Even if it isn't allowed, it shouldn't be dumping core :-)
>>
>> Anything further I can do to assist, please let me know.
>>
> 
> If it makes you feel better, I clearly didn't even smoke test the patch 
> against our own regression tests. =(
> 
> ===> Expected failures
> grep_test:zgrep_recursive  ->  expected_failure: unimplemented zgrep 
> wrapper script functionality: atf-check failed; see the output of the 
> test for details  [0.029s]
> ===> Failed tests
> grep_test:matchall  ->  failed: atf-check failed; see the output of the 
> test for details  [0.037s]
> grep_test:oflag_zerolen  ->  failed: atf-check failed; see the output of 
> the test for details  [0.074s]
> grep_test:xflag_emptypat  ->  failed: atf-check failed; see the output 
> of the test for details  [0.044s]
> grep_test:xflag_emptypat_plus  ->  failed: atf-check failed; see the 
> output of the test for details  [0.047s]
> grep_test:zgrep_empty_eflag  ->  failed: atf-check failed; see the 
> output of the test for details  [0.047s]
> ===> Summary
> Results read from /root/.kyua/store/ 
> results.usr_obj_usr_src_arm64.aarch64_usr.bin_grep_tests_checkdir_usr_tests_usr.bin_grep.20250312-025551-841462.db
> Test cases: 56 total, 0 skipped, 1 expected failures, 0 broken, 5 failed
> Start time: 2025-03-12T02:55:51.906277Z
> End time:   2025-03-12T02:55:55.435279Z
> Total time: 2.930s
> -- 
> 
> Which would have revealed:
> 
> Standard output:
> Executing command [ zgrep -e  test ]
> 
> Standard error:
> Fail: incorrect exit status: 134, expected: 0
> stdout:
> 
> stderr:
> Assertion failed: (pc->matchidx > 0), function procmatch_match, file / 
> usr/src/usr.bin/grep/util.c, line 223.
> Abort trap (core dumped)
> -- 
> 
> I'll take a little bit to understand the patch I wrote back then, add an 
> extra test to cover the originally-reported bug, fix the patch, then get 
> it into Phabricator ASAP.  Sorry for dropping this-
> 
> Thanks,
> 
> Kyle Evans
> 

Here we go: https://reviews.freebsd.org/D49324

Thanks,

Kyle Evans



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4e719f32-7be0-46dd-8113-d8bee17a846e>