* [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us
2017-12-22 22:14 [PATCH v2 0/2] kbuild: Fix corner caches with .cache.mk Douglas Anderson
@ 2017-12-22 22:14 ` Douglas Anderson
2017-12-31 7:46 ` Masahiro Yamada
0 siblings, 1 reply; 3+ messages in thread
From: Douglas Anderson @ 2017-12-22 22:14 UTC (permalink / raw)
To: Masahiro Yamada
Cc: malat, dave.hansen, yang.s, linux, Douglas Anderson,
Matthias Kaehlcke, Cao jin, Arnd Bergmann, Marcin Nowakowski,
Mark Charlebois, linux-kernel, Ingo Molnar, Nick Desaulniers
Several people reported that the commit 3298b690b21c ("kbuild: Add a
cache for generated variables") caused them problems when they updated
gcc versions. Specifically the reports all looked something similar
to this:
> In file included from ./include/uapi/linux/uuid.h:21:0,
> from ./include/linux/uuid.h:19,
> from ./include/linux/mod_devicetable.h:12,
> from scripts/mod/devicetable-offsets.c:2:
> ./include/linux/string.h:8:20: fatal error: stdarg.h: No such file or
> directory
> #include <stdarg.h>
Masahiro Yamada determined that the problem was with:
NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC)
-print-file-name=include)
Specifically that the stale result of -print-file-name is stored in
the cache file. It was determined that a "make clean" fixed the
problems in all cases.
In this particular case we could certainly try to clean just the cache
when we detect a gcc update, but it seems like overall it's a bad idea
to do an incremental build when gcc changes. We should warn the user
and tell them that they need a 'make clean'.
Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
Reported-by: Yang Shi <yang.s@alibaba-inc.com>
Reported-by: Dave Hansen <dave.hansen@intel.com>
Reported-by: Mathieu Malaterre <malat@debian.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- Don't error if MAKECMDGOALS is blank.
scripts/Kbuild.include | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 065324a8046f..f7efb59d85d1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -222,6 +222,10 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
$(srctree)/scripts/gcc-version.sh -p $(CC))
+# cc-fullversion-uncached
+cc-fullversion-uncached := $(shell $(CONFIG_SHELL) \
+ $(srctree)/scripts/gcc-version.sh -p $(CC))
+
# cc-ifversion
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
@@ -475,3 +479,16 @@ endif
endef
#
###############################################################################
+
+# Require a 'make clean' if the compiler changed; not only does the .cache.mk
+# need to be thrown out but we should also start with fresh object files.
+#
+# NOTE: it's important that we don't error out when the goal is actually to
+# try to make clean, distclean or mrproper.
+ifeq ($(filter %clean,$(MAKECMDGOALS))$(filter mrproper,$(MAKECMDGOALS)),)
+ ifneq ($(MAKECMDGOALS),)
+ ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
+ $(error Detected new CC version ($(cc-fullversion-uncached) vs $(cc-fullversion)). Please 'make clean')
+ endif
+ endif
+endif
--
2.15.1.620.gb9897f4670-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us
@ 2017-12-22 22:44 Guenter Roeck
0 siblings, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2017-12-22 22:44 UTC (permalink / raw)
To: Douglas Anderson
Cc: Masahiro Yamada, malat, dave.hansen, yang.s, Matthias Kaehlcke,
Cao jin, Arnd Bergmann, Marcin Nowakowski, Mark Charlebois,
linux-kernel, Ingo Molnar, Nick Desaulniers
On Fri, Dec 22, 2017 at 02:14:53PM -0800, Douglas Anderson wrote:
> Several people reported that the commit 3298b690b21c ("kbuild: Add a
> cache for generated variables") caused them problems when they updated
> gcc versions. Specifically the reports all looked something similar
> to this:
>
> > In file included from ./include/uapi/linux/uuid.h:21:0,
> > from ./include/linux/uuid.h:19,
> > from ./include/linux/mod_devicetable.h:12,
> > from scripts/mod/devicetable-offsets.c:2:
> > ./include/linux/string.h:8:20: fatal error: stdarg.h: No such file or
> > directory
> > #include <stdarg.h>
>
> Masahiro Yamada determined that the problem was with:
>
> NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC)
> -print-file-name=include)
>
> Specifically that the stale result of -print-file-name is stored in
> the cache file. It was determined that a "make clean" fixed the
> problems in all cases.
>
> In this particular case we could certainly try to clean just the cache
> when we detect a gcc update, but it seems like overall it's a bad idea
> to do an incremental build when gcc changes. We should warn the user
> and tell them that they need a 'make clean'.
>
> Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
> Reported-by: Yang Shi <yang.s@alibaba-inc.com>
> Reported-by: Dave Hansen <dave.hansen@intel.com>
> Reported-by: Mathieu Malaterre <malat@debian.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
No idea if it is correct, but this version works for me.
Tested-by: Guenter Roeck <linux@roeck-us.net>
> ---
>
> Changes in v2:
> - Don't error if MAKECMDGOALS is blank.
>
> scripts/Kbuild.include | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 065324a8046f..f7efb59d85d1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -222,6 +222,10 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
> cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
> $(srctree)/scripts/gcc-version.sh -p $(CC))
>
> +# cc-fullversion-uncached
> +cc-fullversion-uncached := $(shell $(CONFIG_SHELL) \
> + $(srctree)/scripts/gcc-version.sh -p $(CC))
> +
> # cc-ifversion
> # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
> @@ -475,3 +479,16 @@ endif
> endef
> #
> ###############################################################################
> +
> +# Require a 'make clean' if the compiler changed; not only does the .cache.mk
> +# need to be thrown out but we should also start with fresh object files.
> +#
> +# NOTE: it's important that we don't error out when the goal is actually to
> +# try to make clean, distclean or mrproper.
> +ifeq ($(filter %clean,$(MAKECMDGOALS))$(filter mrproper,$(MAKECMDGOALS)),)
> + ifneq ($(MAKECMDGOALS),)
> + ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
> + $(error Detected new CC version ($(cc-fullversion-uncached) vs $(cc-fullversion)). Please 'make clean')
> + endif
> + endif
> +endif
> --
> 2.15.1.620.gb9897f4670-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us
2017-12-22 22:14 ` [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Douglas Anderson
@ 2017-12-31 7:46 ` Masahiro Yamada
0 siblings, 0 replies; 3+ messages in thread
From: Masahiro Yamada @ 2017-12-31 7:46 UTC (permalink / raw)
To: Douglas Anderson
Cc: malat, dave.hansen, Yang Shi, Guenter Roeck, Matthias Kaehlcke,
Cao jin, Arnd Bergmann, Marcin Nowakowski, Mark Charlebois,
Linux Kernel Mailing List, Ingo Molnar, Nick Desaulniers
Hi Douglas,
2017-12-23 7:14 GMT+09:00 Douglas Anderson <dianders@chromium.org>:
> Several people reported that the commit 3298b690b21c ("kbuild: Add a
> cache for generated variables") caused them problems when they updated
> gcc versions. Specifically the reports all looked something similar
> to this:
>
>> In file included from ./include/uapi/linux/uuid.h:21:0,
>> from ./include/linux/uuid.h:19,
>> from ./include/linux/mod_devicetable.h:12,
>> from scripts/mod/devicetable-offsets.c:2:
>> ./include/linux/string.h:8:20: fatal error: stdarg.h: No such file or
>> directory
>> #include <stdarg.h>
>
> Masahiro Yamada determined that the problem was with:
>
> NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC)
> -print-file-name=include)
>
> Specifically that the stale result of -print-file-name is stored in
> the cache file. It was determined that a "make clean" fixed the
> problems in all cases.
>
> In this particular case we could certainly try to clean just the cache
> when we detect a gcc update, but it seems like overall it's a bad idea
> to do an incremental build when gcc changes. We should warn the user
> and tell them that they need a 'make clean'.
>
> Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
> Reported-by: Yang Shi <yang.s@alibaba-inc.com>
> Reported-by: Dave Hansen <dave.hansen@intel.com>
> Reported-by: Mathieu Malaterre <malat@debian.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - Don't error if MAKECMDGOALS is blank.
>
> scripts/Kbuild.include | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 065324a8046f..f7efb59d85d1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -222,6 +222,10 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
> cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
> $(srctree)/scripts/gcc-version.sh -p $(CC))
>
> +# cc-fullversion-uncached
> +cc-fullversion-uncached := $(shell $(CONFIG_SHELL) \
> + $(srctree)/scripts/gcc-version.sh -p $(CC))
> +
No.
You used ':=' flavor assignment, so the gcc-version.sh
script is evaluated here.
The top-level Makefile includes scripts/Kbuild.include at line 278,
then defines CC at line 284.
Since $(CC) is not defined here,
the resulted cc-fullversion-uncached is
"Error: No compiler specified. Usage: ./scripts/gcc-version.sh <gcc-command>"
cc-fullversion-uncached works as expected
only after Kbuild descends into sub-directories.
> # cc-ifversion
> # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
> @@ -475,3 +479,16 @@ endif
> endef
> #
> ###############################################################################
> +
> +# Require a 'make clean' if the compiler changed; not only does the .cache.mk
> +# need to be thrown out but we should also start with fresh object files.
> +#
> +# NOTE: it's important that we don't error out when the goal is actually to
> +# try to make clean, distclean or mrproper.
> +ifeq ($(filter %clean,$(MAKECMDGOALS))$(filter mrproper,$(MAKECMDGOALS)),)
> + ifneq ($(MAKECMDGOALS),)
> + ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
As I noted above, CC is not defined yet when parsing the top-level Makefile.
Evaluating "cc-fullversion" here
adds a strange cache line, like follows:
__cached__./scripts/gcc-version.sh_-p_ := Error: No compiler
specified. Usage: ./scripts/gcc-version.sh <gcc-command>
This check only works only in the second inclusion or later.
At the first inclusion,
both 'cc-fullversion-uncached' and 'cc-fullversion'
contain "Error: No compiler specified. ..."
scripts/Kbuild.include is included every time Kbuild
descends into a sub-directory.
It is pointless to check gcc version multiple times.
> + $(error Detected new CC version ($(cc-fullversion-uncached) vs $(cc-fullversion)). Please 'make clean')
> + endif
> + endif
> +endif
> --
> 2.15.1.620.gb9897f4670-goog
>
After all, I recommend to move this code into the top-level Makefile,
around line 585.
--- a/Makefile
+++ b/Makefile
@@ -585,6 +585,14 @@ virt-y := virt/
endif # KBUILD_EXTMOD
ifeq ($(dot-config),1)
+# Require a 'make clean' if the compiler changed; not only does the .cache.mk
+# need to be thrown out but we should also start with fresh object files.
+cc-fullversion-uncached := $(shell $(CONFIG_SHELL)
$(srctree)/scripts/gcc-version.sh -p $(CC))
+
+ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
+ $(error Detected new CC version ($(cc-fullversion-uncached) vs
$(cc-fullversion)). Please 'make clean')
+endif
+
# Read in config
-include include/config/auto.conf
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-31 7:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-22 22:44 [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Guenter Roeck
-- strict thread matches above, loose matches on Subject: below --
2017-12-22 22:14 [PATCH v2 0/2] kbuild: Fix corner caches with .cache.mk Douglas Anderson
2017-12-22 22:14 ` [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Douglas Anderson
2017-12-31 7:46 ` Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox