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

[Nick Alcock]

On 11 Dec 2023, Kris Van Hees said:

> On Mon, Dec 11, 2023 at 08:34:08PM +0000, Nick Alcock wrote:
>> 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.
>
> But isn't the way you do the patching going to silently ignore patch failures?

Yes, but the buggy code has never changed in the history of flex: so the
only reason patching is going to fail is if the patch is already applied
(or we run out of disk space, in which case other problems will emerge
immediately).

> Perhaps we should consider including pre-generated flex output that is used
> when flex is too old?  Other projects have done that and it avoids fragile
> patching one way or another.

That's doable -- and also means we can build without flex at all. Maybe
we should be consistent and include pregenerated bison output too, for
the same reason. (Unlike the way Autoconf does it, I'll stuff the
pregenerated output in a subdir and explicitly copy it into place iff
needed, to avoid problems where flex or bison *is* new enough but
timestamps stop it being used -- I think we should use them if they're
present, to pick up any other fixes that postdate our pregenerated
skeletons.)

I'll change things accordingly.

-- 
NULL && (void)