Fix the handling of REG_STARTEND in sed #47

dashea commented 2025年11月08日 20:24:57 +01:00

sed contains regexec calls that use REG_STARTEND with starts and ends
that are potentially not the start or end of the string being processed.
Adjust the string passed to regexec and the returned match offsets to
account for the difference in behavior when not using REG_STARTEND.

The do-while loop in substitute is the only use of REG_STARTEND with a non-zero start. Without the adjustments, doing multiple substitutions (e.g., echo 'a:b:c' | sed 's/:/ /g') would loop forever.

It's not perfect; it won't handle NUL bytes, and it won't handle word boundaries (also a non-standard extension) correctly. For example, echo one two three | sed 's/\<./1/g' with this change and the glibc regex functions will return "111 111 11111" instead of "1ne 1wo 1hree". I don't think it's possible to handle those cases without including the regex functions from FreeBSD's C library.

The difference in REG_STARTEND's newline behavior should be fine, at least. The do-while loop calls regexec with REG_NOTBOL, and the expression is never compiled with REG_NEWLINE, so regexec will correctly not match '^' before the substring.

sed contains regexec calls that use REG_STARTEND with starts and ends that are potentially not the start or end of the string being processed. Adjust the string passed to regexec and the returned match offsets to account for the difference in behavior when not using REG_STARTEND. The do-while loop in substitute is the only use of REG_STARTEND with a non-zero start. Without the adjustments, doing multiple substitutions (e.g., `echo 'a:b:c' | sed 's/:/ /g'`) would loop forever. It's not perfect; it won't handle NUL bytes, and it won't handle word boundaries (also a non-standard extension) correctly. For example, `echo one two three | sed 's/\<./1/g'` with this change and the glibc regex functions will return "111 111 11111" instead of "1ne 1wo 1hree". I don't think it's possible to handle those cases without including the regex functions from FreeBSD's C library. The difference in REG_STARTEND's newline behavior should be fine, at least. The do-while loop calls regexec with REG_NOTBOL, and the expression is never compiled with REG_NEWLINE, so regexec will correctly not match '^' before the substring.

dcantrell commented 2025年11月08日 21:23:31 +01:00

What about what NetBSD has in their code?

https://github.com/NetBSD/src/tree/trunk/usr.bin/sed

When faced with a portability issue and the FreeBSD code isn't as portable, I go and look at the BSD for toaster ovens since they tend to be more portable. I know that Free and Net share stuff back and forth so it's not deviating too far (likely) from what FreeBSD has.

I haven't looked at the NetBSD sed, so that may be a non-starter.

What about what NetBSD has in their code? https://github.com/NetBSD/src/tree/trunk/usr.bin/sed When faced with a portability issue and the FreeBSD code isn't as portable, I go and look at the BSD for toaster ovens since they tend to be more portable. I know that Free and Net share stuff back and forth so it's not deviating too far (likely) from what FreeBSD has. I haven't looked at the NetBSD sed, so that may be a non-starter.

dashea commented 2025年11月08日 22:14:48 +01:00

hm, looks like I reinvented half of what they do. Their substitute loop is set up so that regexec_e is always called with a start of 0, and then there's some #ifndef REG_STARTEND sections in regexec_e that create a copy of the substring.

hm, looks like I reinvented half of what they do. Their substitute loop is set up so that regexec_e is always called with a start of 0, and then there's some `#ifndef REG_STARTEND` sections in regexec_e that create a copy of the substring.

dashea commented 2025年11月11日 20:25:43 +01:00

I did a little more digging, and the reason FreeBSD uses non-zero starts in substitute() is to fix a bug described in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209352. I changed my change to just undo this patch.

fwiw NetBSD still has this bug.

It sure would be nice the BSDs or GNU had a test suite for sed that made any damn sense to run, but they don't so I didn't. It works with hanoi.sed and math.sed and the case that led me down this rabbit hole to start with, so maybe I didn't break things too horribly.

Just swapping to NetBSD's sed outright might also be an option.

I did a little more digging, and the reason FreeBSD uses non-zero starts in substitute() is to fix a bug described in <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209352>. I changed my change to just undo this patch. fwiw NetBSD still has this bug. It sure would be nice the BSDs or GNU had a test suite for sed that made any damn sense to run, but they don't so I didn't. It works with hanoi.sed and math.sed and the case that led me down this rabbit hole to start with, so maybe I didn't break things too horribly. Just swapping to NetBSD's sed outright might also be an option.

dcantrell commented 2025年11月12日 19:45:59 +01:00

For the tests, I just meant the tests/ subdirectory that is found in usr.bin/sed/ in the FreeBSD source. I think that's just a matter of running the shell scripts in there but making sure it invokes the built sed rather than the one on the system. I just haven't looked in to how to hook that up.

I was going to ask about importing sed from NetBSD instead. I'll merge this for now but maybe this might be a combined project of some things imported from FreeBSD, but some imported from NetBSD when the FreeBSD is just not workable. It would probably be easiest to just import from NetBSD directly anyway, but those commands are far more basic than the FreeBSD ones. I see the FreeBSD developers actually extend these and add in options for GNU compatibility or GNU options that are useful. I'm not sure NetBSD really cares about that.

For the tests, I just meant the tests/ subdirectory that is found in usr.bin/sed/ in the FreeBSD source. I think that's just a matter of running the shell scripts in there but making sure it invokes the built sed rather than the one on the system. I just haven't looked in to how to hook that up. I was going to ask about importing sed from NetBSD instead. I'll merge this for now but maybe this might be a combined project of some things imported from FreeBSD, but some imported from NetBSD when the FreeBSD is just not workable. It would probably be easiest to just import from NetBSD directly anyway, but those commands are far more basic than the FreeBSD ones. I see the FreeBSD developers actually extend these and add in options for GNU compatibility or GNU options that are useful. I'm not sure NetBSD really cares about that.