[DTrace-devel] [PATCH v2 03/23] lexer: work around bug in flex <= 2.6.0

[Nick Alcock]

On 6 Dec 2023, Kris Van Hees said:
> On Mon, Nov 27, 2023 at 04:47:09PM +0000, Nick Alcock wrote:
>> Flex commit f863c949, released in 2.6.1, is a one-line fix to the
>> generated flex skeleton that fixes an old bug that breaks yywrap();
>> if it returns nonzero, input() is meant to return 0 to its caller
>> on EOF. This bug makes it return EOF instead, which no lexer expects
>> (and certainly the DTrace lexer does not).
>> 
>> Alas we cannot get around this bug just by defining %option noyywrap,
>> since that still emits the same buggy piece of the skeleton but just
>> introduces its own yywrap() which returns 1. And there's no way to
>> fix the skeleton from flex itself, and we can't require flex >= 2.6.1
>> since OL7 has 2.5.37.
>> 
>> So, for now, resort to a one-line patch to the generated lexer, silently
>> applied and skipped if it doesn't apply.  (Any generated lexer it doesn't
>> apply on will have this bug fixed.)
>
> Repeating my review comments from the previous posting of this patch:
>
> # I think it might be better to perhaps do this using a sed command since there
> # is only a single 'return EOF' present in the generated dt_lex.c anyway.  So a
> # simple s/return EOF;/return 0;/ would suffice.  And it is a no-op if no
> # return EOF is found.

That seems terribly fragile to me -- what if a later version of flex
introduces another 'return EOF;'? What if we use that extremely
inoffensive and commonplace line ourselves?

The reason I used patch was that patch has patch context, which makes
this much less likely to misapply. Patching source with sed has been
considered obsolete for decades for a reason.

> Doing so makes this patch smaller and avoids extra build dependencies.

You'll pardon me for not worrying about requiring *patch*. It's tiny and
ubiquitously available. I worry about as much as I would about requiring
cat.

> Alternatively, you could require flex >= 2.6.1 *except* on OL7.  Then the
> patch becomes a simple change to dtrace.spec, right?  That would be even more
> desirable.  I can live with the odd (buggy) error reporting behaviour this
> causes on older flex versions if wew ccan reasonably assume that most people
> will have a recent enough flex anyway.

That seems reasonable enough to me though, and much less ugly -- except
that this bug causes buffer overruns (it's the fundamental reason for
the err.pspec-default.d failure). This *is* a program running as root...
but if we don't care, I'm happy to just require a recent flex!