[Buildroot] [PATCH 0/1] linux: limit YYLTYPE kernel modification

[Buildroot] [PATCH 0/1] linux: limit YYLTYPE kernel modification

[Markus Mayer via buildroot]

Hi,

It took me a while to find out why our kernel builds were suddenly
failing with the 2022.02-rcX sources. Oddly, building DTC for the host
would error out like so:

dtc-parser.y: In function `print_error':
dtc-parser.y:478:17: error: `yylloc' undeclared (first use in this function);
    did you mean `yyalloc'?
dtc-parser.y:478:17: note: each undeclared identifier is reported only once
    for each function it appears in
dtc-lexer.lex.c: In function `yylex':
dtc-lexer.l:46:18: error: `yylloc' undeclared (first use in this function);
    did you mean `yyalloc'?
dtc-lexer.lex.c:845:2: note: in expansion of macro `YY_USER_ACTION'
dtc-lexer.lex.c:939:1: note: in expansion of macro `YY_RULE_SETUP'
dtc-lexer.l:46:18: note: each undeclared identifier is reported only once
    for each function it appears in
dtc-lexer.lex.c:845:2: note: in expansion of macro `YY_USER_ACTION'
dtc-lexer.lex.c:939:1: note: in expansion of macro `YY_RULE_SETUP'

The kernel build that's failing is Linux 4.1 with GCC 6.3. Yes, I know.
Old kernel, old toolchain. Still, this used to work, and now it doesn't.
(We have newer toolchains and newer kernels, too. They seem fine.)

Eventually, I tracked it down to this change:

https://git.buildroot.net/buildroot/commit/?id=9b41b54be077

I think there are two issues with it. It claims to be for GCC 10+, but
it is applied at all times. I believe it should be conditional upon
BR2_TOOLCHAIN_GCC_AT_LEAST_10 if the intention is to fix up the sources
for GCC 10 and newer.

Secondly, this code works its magic completely silently and quietly.
There is no indication that anything is being done to the kernel
sources. That makes it really, *REALLY* challenging to figure out what's
going on when things go sideways.

I think an informational message to the user is absolutely crucial. At
least that way it'll show up in the build logs if one searches for
"YYLTYPE". That'll give the users a hint where the sources are being
touched. Modifying source code like this should not happen quietly.

Regards,
-Markus

Markus Mayer (1):
  linux: limit YYLTYPE kernel modification

 linux/linux.mk | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/1] linux: limit YYLTYPE kernel modification

[Markus Mayer via buildroot]

Commit 9b41b54be07711c10ad13ce157be272ed1cf402e addresses a build issue
regarding re-defined YACC symbols. Unfortunately, this can break the
build for older toolchains.

To prevent unintentional breakage, make the source code modification
conditional on BR2_TOOLCHAIN_GCC_AT_LEAST_10. Also inform the user about
the modification when it is performed, so it becomes more obvious what
is happening in case of unforseen problems.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 linux/linux.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux/linux.mk b/linux/linux.mk
index 940dc2849f..382a3f679e 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -293,12 +293,15 @@ endef
 
 LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
 
+ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_10),y)
 # Older versions break on gcc 10+ because of redefined symbols
 define LINUX_DROP_YYLLOC
+	@echo "Removing 'YYLTYPE yylloc;' from kernel sources..."
 	$(Q)grep -Z -l -r -E '^YYLTYPE yylloc;$$' $(@D) \
 	|xargs -0 -r $(SED) '/^YYLTYPE yylloc;$$/d'
 endef
 LINUX_POST_PATCH_HOOKS += LINUX_DROP_YYLLOC
+endif
 
 # Older linux kernels use deprecated perl constructs in timeconst.pl
 # that were removed for perl 5.22+ so it breaks on newer distributions
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] linux: limit YYLTYPE kernel modification

[Yann E. MORIN]

Markus, All,

On 2022-03-04 16:22 -0800, Markus Mayer spake thusly:
> Commit 9b41b54be07711c10ad13ce157be272ed1cf402e addresses a build issue
> regarding re-defined YACC symbols. Unfortunately, this can break the
> build for older toolchains.
> 
> To prevent unintentional breakage, make the source code modification
> conditional on BR2_TOOLCHAIN_GCC_AT_LEAST_10. Also inform the user about
> the modification when it is performed, so it becomes more obvious what
> is happening in case of unforseen problems.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  linux/linux.mk | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 940dc2849f..382a3f679e 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -293,12 +293,15 @@ endef
>  
>  LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
>  
> +ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_10),y)

That is wrong, because this is built with the _host_ compiler, not the
target compiler, and BR2_TOOLCHAIN_GCC_AT_LEAST_XX is about the target
compiler.

So, if we wanted to make that conditional, that would have to be
conditional on BR2_HOST_GCC_AT_LEAST_10 (we only have up to gcc-9 for
the host for now, so we'd need to add it for gcc-10 and 11; hint: search
for BR2_HOST_GCC_AT_LEAST_9, HOST_GCC_VERSION, HOSTCC_VERSION, and
HOSTCC_MAX_VERSION).

Furthermore, that is still wrong in your case: if you switch to a
distribution that has gcc 10 (so, as host compler!), then this fixup
will be applied again, and your build will break again, because your
kernel is (probably) missing the following commits (of the 29 that
touch scripts/dtc/ between 4.1 and e33a814e772c):

    $ git slog -G'YYLTYPE yylloc;' v4.1..e33a814e772cdc36436c8c188d8c42d019fda639 -- scripts/dtc/
    e33a814e772cd scripts/dtc: Remove redundant YYLOC global declaration
    e039139be8c25 scripts/dtc: generate lexer and parser during build instead of shipping
    4760597116e34 scripts/dtc: Update to upstream version 9d3649bd3be245c9

However, you still have a point: this is breaking older kernels that do
not have a recent-enough bundled dtc, whether they be built with a host
gcc 10+ or not...

Maybe we need to find a better heuristic to detect the case when we need
to apply the fix, but then I am afraid this is going to be a nightmare
of so many corner cases with vendor-specific franken-kernels.

I am not sure where to go from here, though...

>  # Older versions break on gcc 10+ because of redefined symbols
>  define LINUX_DROP_YYLLOC
> +	@echo "Removing 'YYLTYPE yylloc;' from kernel sources..."

There are so many fixes and tweaks we do without informing the user. If
we were to, the build would be even more verbose than it currently is.
Finding a reference to yylloc in the build log would just mean grepping
for it, so you would achieve about the same by running:

    $ make V=1

as that would print the fixup line.

Regards,
Yann E. MORIN.

>  	$(Q)grep -Z -l -r -E '^YYLTYPE yylloc;$$' $(@D) \
>  	|xargs -0 -r $(SED) '/^YYLTYPE yylloc;$$/d'
>  endef
>  LINUX_POST_PATCH_HOOKS += LINUX_DROP_YYLLOC
> +endif
>  
>  # Older linux kernels use deprecated perl constructs in timeconst.pl
>  # that were removed for perl 5.22+ so it breaks on newer distributions
> -- 
> 2.25.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] linux: limit YYLTYPE kernel modification

[Markus Mayer via buildroot]

On Sat, 5 Mar 2022 at 01:18, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Markus, All,
>
> On 2022-03-04 16:22 -0800, Markus Mayer spake thusly:
> > Commit 9b41b54be07711c10ad13ce157be272ed1cf402e addresses a build issue
> > regarding re-defined YACC symbols. Unfortunately, this can break the
> > build for older toolchains.
> >
> > To prevent unintentional breakage, make the source code modification
> > conditional on BR2_TOOLCHAIN_GCC_AT_LEAST_10. Also inform the user about
> > the modification when it is performed, so it becomes more obvious what
> > is happening in case of unforseen problems.
[...]
> > diff --git a/linux/linux.mk b/linux/linux.mk
> > index 940dc2849f..382a3f679e 100644
> > --- a/linux/linux.mk
> > +++ b/linux/linux.mk
> > @@ -293,12 +293,15 @@ endef
> >
> >  LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
> >
> > +ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_10),y)
>
> That is wrong, because this is built with the _host_ compiler, not the
> target compiler, and BR2_TOOLCHAIN_GCC_AT_LEAST_XX is about the target
> compiler.

Very good point. I convoluted host and target compilers. The issue was
seen on a Ubuntu 20 build host, which means it was using GCC 9.3.0 as
the host compiler.

> So, if we wanted to make that conditional, that would have to be
> conditional on BR2_HOST_GCC_AT_LEAST_10 (we only have up to gcc-9 for
> the host for now, so we'd need to add it for gcc-10 and 11; hint: search
> for BR2_HOST_GCC_AT_LEAST_9, HOST_GCC_VERSION, HOSTCC_VERSION, and
> HOSTCC_MAX_VERSION).
>
> Furthermore, that is still wrong in your case: if you switch to a
> distribution that has gcc 10 (so, as host compler!), then this fixup
> will be applied again, and your build will break again, because your
> kernel is (probably) missing the following commits (of the 29 that
> touch scripts/dtc/ between 4.1 and e33a814e772c):

Yes. Unfortunately, this is true. Using GCC 10, the build stumbles
over yylloc again.  I installed gcc-10 and gave it another spin:

[...]
/usr/bin/gcc-10 -O2
-I/local/users/mmayer/buildroot/output/arm64/host/include -DNDEBUG
-L/local/users/mmayer/buildroot/output/arm64/host/lib
-Wl,-rpath,/local/users/mmayer/buildroot/output/arm64/host/lib
-Wp,-MD,scripts/dtc/.dtc-lexer.lex.o.d -Wall -Wmissing-prototypes
-Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu89
-Iscripts/dtc -Iscripts/dtc/libfdt -c -o scripts/dtc/dtc-lexer.lex.o
scripts/dtc/dtc-lexer.lex.c
dtc-lexer.lex.c: In function ‘yylex’:
dtc-lexer.l:46:18: error: ‘yylloc’ undeclared (first use in this
function); did you mean ‘yyalloc’?
dtc-lexer.lex.c:845:2: note: in expansion of macro ‘YY_USER_ACTION’
dtc-lexer.lex.c:939:1: note: in expansion of macro ‘YY_RULE_SETUP’
dtc-lexer.l:46:18: note: each undeclared identifier is reported only
once for each function it appears in
dtc-lexer.lex.c:845:2: note: in expansion of macro ‘YY_USER_ACTION’
dtc-lexer.lex.c:939:1: note: in expansion of macro ‘YY_RULE_SETUP’
make[4]: *** [scripts/Makefile.host:108: scripts/dtc/dtc-lexer.lex.o] Error 1
make[3]: *** [scripts/Makefile.build:476: scripts/dtc] Error 2
make[2]: *** [Makefile:555: scripts] Error 2
make[1]: *** [package/pkg-generic.mk:292:
/local/users/mmayer/buildroot/output/arm64/build/linux-stb-4.1/.stamp_built]
Error 2
make: *** [Makefile:27: _all] Error 2

>     $ git slog -G'YYLTYPE yylloc;' v4.1..e33a814e772cdc36436c8c188d8c42d019fda639 -- scripts/dtc/
>     e33a814e772cd scripts/dtc: Remove redundant YYLOC global declaration
>     e039139be8c25 scripts/dtc: generate lexer and parser during build instead of shipping
>     4760597116e34 scripts/dtc: Update to upstream version 9d3649bd3be245c9

Correct. We don't have those. However, it might be possible for us to
pull some of them in. Not sure yet. I'd have to take a closer look.

> However, you still have a point: this is breaking older kernels that do
> not have a recent-enough bundled dtc, whether they be built with a host
> gcc 10+ or not...

Agreed. We might be able to overcome the issue by looking at the
aforementioned kernel patches, but others might still run into it.

> Maybe we need to find a better heuristic to detect the case when we need
> to apply the fix, but then I am afraid this is going to be a nightmare
> of so many corner cases with vendor-specific franken-kernels.
>
> I am not sure where to go from here, though...

Let me mull this one over for a bit, as well. I am guessing you'll
have thought of most approaches already, but one never knows what one
might stumble upon.

> >  # Older versions break on gcc 10+ because of redefined symbols
> >  define LINUX_DROP_YYLLOC
> > +     @echo "Removing 'YYLTYPE yylloc;' from kernel sources..."
>
> There are so many fixes and tweaks we do without informing the user. If
> we were to, the build would be even more verbose than it currently is.
> Finding a reference to yylloc in the build log would just mean grepping
> for it, so you would achieve about the same by running:
>
>     $ make V=1
>
> as that would print the fixup line.

True and I see your point regarding super-verbose output, however, the
problem I was facing here was that I didn't *know* to run "make V=1"
on buildroot as a whole -- and that I would have to do so *before* it
patched the kernel. It won't go through that stage again if you just
reconfigure or rebuild Linux. I was thinking that the Linux build
system itself was somehow messing with the files. So, I was looking in
the completely wrong place for a while.

I am not saying a message not relying on "V=1" in the buildroot build
log would have actually helped me find this issue much more quickly,
but it could have served as a pointer to look at the Buildroot patch
stage more closely and not the kernel build system. And that
definitely wouldn't have hurt.

Regards,
-Markus
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot