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

[Kris Van Hees]

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.

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

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.

> We can remove this when we can require flex 2.6.1.
> 
> One tiny output change triggered by this: the conceptual line number on
> which EOF-related errors happen is now one higher, because there's an
> extra call to input() triggered by the previous fix which bumps the line
> number by one (in effect, 'off EOF' is considered an extra line after
> the last line in the file).  This is technically a behaviour change but
> I very much doubt anyone will mind.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>  .gitignore                   |  2 ++
>  dtrace.spec                  |  2 +-
>  libdtrace/Build              |  3 ++-
>  libdtrace/dt_lex.yywrap.diff | 11 +++++++++++
>  4 files changed, 16 insertions(+), 2 deletions(-)
>  create mode 100644 libdtrace/dt_lex.yywrap.diff
> 
> diff --git a/.gitignore b/.gitignore
> index 18f2ecb85c6c..892c156f8910 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,6 +1,8 @@
>  *.[oa]
>  build/
>  *~
> +*.rej
> +*.orig
>  .git-version
>  
>  #
> diff --git a/dtrace.spec b/dtrace.spec
> index 777dd28ded84..6d81222088fb 100644
> --- a/dtrace.spec
> +++ b/dtrace.spec
> @@ -51,7 +51,7 @@
>  %define glibc32 %{nil}
>  %endif
>  
> -BuildRequires: rpm
> +BuildRequires: rpm patch
>  Name:         dtrace
>  License:      Universal Permissive License (UPL), Version 1.0
>  Group:        Development/Tools
> diff --git a/libdtrace/Build b/libdtrace/Build
> index 76fd32e44a2f..310a01bd453a 100644
> --- a/libdtrace/Build
> +++ b/libdtrace/Build
> @@ -124,6 +124,7 @@ $(libdtrace-build_DIR)%.h $(libdtrace-build_DIR)%.c: $(libdtrace-build_DIR)%.y
>  $(libdtrace-build_DIR)dt_lex.c: $(libdtrace-build_DIR)dt_grammar.h $(libdtrace-build_DIR)dt_lex.l
>  	$(call describe-target,LEX,$(libdtrace-build_DIR)dt_lex.c)
>  	flex -o$(libdtrace-build_DIR)dt_lex.c $(libdtrace-build_DIR)dt_lex.l
> +	(cd $(libdtrace-build_DIR) && patch -r - -sNp0 < dt_lex.yywrap.diff >/dev/null || true)
>  
>  M4_DLIBS += \
>  	procfs.d \
> @@ -259,7 +260,7 @@ clean::
>  	$(call describe-target,CLEAN,libdtrace)
>  	rm -f $(libdtrace-build_DIR)dt_errtags.c
>  	rm -f $(libdtrace-build_DIR)dt_grammar.h $(libdtrace-build_DIR)dt_grammar.c
> -	rm -f $(libdtrace-build_DIR)dt_lex.c
> +	rm -f $(libdtrace-build_DIR)dt_lex.c*
>  	rm -f $(addprefix $(libdtrace-build_DIR),$(BUILD_DLIBS))
>  
>  install::
> diff --git a/libdtrace/dt_lex.yywrap.diff b/libdtrace/dt_lex.yywrap.diff
> new file mode 100644
> index 000000000000..78d15a118121
> --- /dev/null
> +++ b/libdtrace/dt_lex.yywrap.diff
> @@ -0,0 +1,11 @@
> +--- dt_lex.c.orig	2023-06-13 14:00:35.440457181 +0100
> ++++ dt_lex.c	2023-06-13 14:00:49.910469682 +0100
> +@@ -2796,7 +2796,7 @@
> + 				case EOB_ACT_END_OF_FILE:
> + 					{
> + 					if ( yywrap( ) )
> +-						return EOF;
> ++						return 0;
> + 
> + 					if ( ! (yy_did_buffer_switch_on_eof) )
> + 						YY_NEW_FILE;
> -- 
> 2.42.0.271.g85384428f1
> 
>