Re: grep(1) bug - duplicate output lines

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