| Message ID | 72b5332f14bd13a9725753bb219b8bd3fd874ca6.1683275971.git.christophe.leroy@csgroup.eu |
|---|---|
| State | Accepted |
| Commit | 6ab7c3d6bab1584e43f0c0a7e92134811b78bc61 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Fix sparse checks processing | expand |
On Fri, May 05, 2023 at 10:39:39AM +0200, Christophe Leroy wrote: > A lot of errors are encountered when building with sparse checking > activated (make C=1 or make C=2). > > Many of them are fixed in Linux. > > Resynchronise Makefile and include/linux/build_bug.h with Linux > kernel sources by porting the following Linux commits into u-boot: > - 6c49f359ca14 ("kbuild: disable sparse warnings about unknown attributes") > - 80591e61a0f7 ("kbuild: tell sparse about the $ARCH") > - 8788994376d8 ("linux/build_bug.h: change type to int") > - 527edbc18a70 ("build_bug.h: remove most of dummy BUILD_BUG_ON stubs for Sparse") > - c60d3b79423a ("build_bug.h: remove negative-array fallback for BUILD_BUG_ON()") > - 14e83077d55f ("include: drop pointless __compiler_offsetof indirection") > > Also revert commit aa9e891c63 ("include/linux/stddef.h: avoid > 'warning: preprocessor token offsetof redefined'") because the > error it creates is worse than the warning it is trying to fix. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> First, I've applied to u-boot/next now. But second, I had mentioned CI testing too, but seeing the level of checker-error output on qemu-arm I am reluctant to add a test that should build-to-completion but error so much as I worry about it being seen as a low quality test.
Le 15/05/2023 à 23:12, Tom Rini a écrit : > On Fri, May 05, 2023 at 10:39:39AM +0200, Christophe Leroy wrote: > >> A lot of errors are encountered when building with sparse checking >> activated (make C=1 or make C=2). >> >> Many of them are fixed in Linux. >> >> Resynchronise Makefile and include/linux/build_bug.h with Linux >> kernel sources by porting the following Linux commits into u-boot: >> - 6c49f359ca14 ("kbuild: disable sparse warnings about unknown attributes") >> - 80591e61a0f7 ("kbuild: tell sparse about the $ARCH") >> - 8788994376d8 ("linux/build_bug.h: change type to int") >> - 527edbc18a70 ("build_bug.h: remove most of dummy BUILD_BUG_ON stubs for Sparse") >> - c60d3b79423a ("build_bug.h: remove negative-array fallback for BUILD_BUG_ON()") >> - 14e83077d55f ("include: drop pointless __compiler_offsetof indirection") >> >> Also revert commit aa9e891c63 ("include/linux/stddef.h: avoid >> 'warning: preprocessor token offsetof redefined'") because the >> error it creates is worse than the warning it is trying to fix. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > First, I've applied to u-boot/next now. But second, I had mentioned CI > testing too, but seeing the level of checker-error output on qemu-arm I > am reluctant to add a test that should build-to-completion but error so > much as I worry about it being seen as a low quality test. > Well, at least we can now start detecting and fixing them. Also, I don't know how feasible it is, but in Linux kernel the robots report new warnings/error only so that you know you are not adding new ones with new commits. Could CI do that too ? Christophe
On Tue, May 16, 2023 at 06:18:56AM +0000, Christophe Leroy wrote: > > > Le 15/05/2023 à 23:12, Tom Rini a écrit : > > On Fri, May 05, 2023 at 10:39:39AM +0200, Christophe Leroy wrote: > > > >> A lot of errors are encountered when building with sparse checking > >> activated (make C=1 or make C=2). > >> > >> Many of them are fixed in Linux. > >> > >> Resynchronise Makefile and include/linux/build_bug.h with Linux > >> kernel sources by porting the following Linux commits into u-boot: > >> - 6c49f359ca14 ("kbuild: disable sparse warnings about unknown attributes") > >> - 80591e61a0f7 ("kbuild: tell sparse about the $ARCH") > >> - 8788994376d8 ("linux/build_bug.h: change type to int") > >> - 527edbc18a70 ("build_bug.h: remove most of dummy BUILD_BUG_ON stubs for Sparse") > >> - c60d3b79423a ("build_bug.h: remove negative-array fallback for BUILD_BUG_ON()") > >> - 14e83077d55f ("include: drop pointless __compiler_offsetof indirection") > >> > >> Also revert commit aa9e891c63 ("include/linux/stddef.h: avoid > >> 'warning: preprocessor token offsetof redefined'") because the > >> error it creates is worse than the warning it is trying to fix. > >> > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > > > First, I've applied to u-boot/next now. But second, I had mentioned CI > > testing too, but seeing the level of checker-error output on qemu-arm I > > am reluctant to add a test that should build-to-completion but error so > > much as I worry about it being seen as a low quality test. > > > > Well, at least we can now start detecting and fixing them. > > Also, I don't know how feasible it is, but in Linux kernel the robots > report new warnings/error only so that you know you are not adding new > ones with new commits. Could CI do that too ? In theory something like those could be adapted, and if you can either test something for GitLab / Azure, or even something I can integrate in to my release scripts like Coverity scan for after the fact new introductions, I'd appreciate it.
diff --git a/Makefile b/Makefile index 4f4f014d2c..6bf62ebad6 100644 --- a/Makefile +++ b/Makefile @@ -423,7 +423,8 @@ DTC_MIN_VERSION := 010406 CHECK = sparse CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ - -Wbitwise -Wno-return-void -D__CHECK_ENDIAN__ $(CF) + -Wbitwise -Wno-return-void -Wno-unknown-attribute \ + -D__CHECK_ENDIAN__ $(CF) KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__ @@ -1032,6 +1033,9 @@ ifeq ($(CONFIG_ARC)$(CONFIG_NIOS2)$(CONFIG_X86)$(CONFIG_XTENSA),) LDFLAGS_u-boot += -Ttext $(CONFIG_TEXT_BASE) endif +# make the checker run with the right architecture +CHECKFLAGS += --arch=$(ARCH) + # insure the checker run with the right endianness CHECKFLAGS += $(if $(CONFIG_CPU_BIG_ENDIAN),-mbig-endian,-mlittle-endian) diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h index 9c7088bafa..20c2dc7f4b 100644 --- a/include/linux/build_bug.h +++ b/include/linux/build_bug.h @@ -4,15 +4,16 @@ #include <linux/compiler.h> #ifdef __CHECKER__ -#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) (0) -#define BUILD_BUG_ON_NOT_POWER_OF_2(n) (0) #define BUILD_BUG_ON_ZERO(e) (0) -#define BUILD_BUG_ON_NULL(e) ((void *)0) -#define BUILD_BUG_ON_INVALID(e) (0) -#define BUILD_BUG_ON_MSG(cond, msg) (0) -#define BUILD_BUG_ON(condition) (0) -#define BUILD_BUG() (0) #else /* __CHECKER__ */ +/* + * Force a compilation error if condition is true, but also produce a + * result (of value 0 and type int), so the expression can be used + * e.g. in a structure initializer (or where-ever else comma expressions + * aren't permitted). + */ +#define BUILD_BUG_ON_ZERO(e) ((int)sizeof(struct { int:(-!!(e)); })) +#endif /* __CHECKER__ */ /* Force a compilation error if a constant expression is not a power of 2 */ #define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \ @@ -20,15 +21,6 @@ #define BUILD_BUG_ON_NOT_POWER_OF_2(n) \ BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0)) -/* - * Force a compilation error if condition is true, but also produce a - * result (of value 0 and type size_t), so the expression can be used - * e.g. in a structure initializer (or where-ever else comma expressions - * aren't permitted). - */ -#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); })) -#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:(-!!(e)); })) - /* * BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the * expression but avoids the generation of any code, even if that expression @@ -52,23 +44,9 @@ * If you have some code which relies on certain constants being equal, or * some other compile-time-evaluated condition, you should use BUILD_BUG_ON to * detect if someone changes it. - * - * The implementation uses gcc's reluctance to create a negative array, but gcc - * (as of 4.4) only emits that error for obvious cases (e.g. not arguments to - * inline functions). Luckily, in 4.3 they added the "error" function - * attribute just for this type of case. Thus, we use a negative sized array - * (should always create an error on gcc versions older than 4.4) and then call - * an undefined function with the error attribute (should always create an - * error on gcc 4.3 and later). If for some reason, neither creates a - * compile-time error, we'll still have a link-time error, which is harder to - * track down. */ -#ifndef __OPTIMIZE__ -#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)])) -#else #define BUILD_BUG_ON(condition) \ BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) -#endif /** * BUILD_BUG - break compile if used. @@ -98,6 +76,4 @@ #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) -#endif /* __CHECKER__ */ - #endif /* _LINUX_BUILD_BUG_H */ diff --git a/include/linux/stddef.h b/include/linux/stddef.h index a7f546fdfe..c732eef65a 100644 --- a/include/linux/stddef.h +++ b/include/linux/stddef.h @@ -14,13 +14,7 @@ #include <linux/types.h> #endif -#ifndef __CHECKER__ #undef offsetof -#ifdef __compiler_offsetof -#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER) -#else -#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER) -#endif -#endif +#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER) #endif
A lot of errors are encountered when building with sparse checking activated (make C=1 or make C=2). Many of them are fixed in Linux. Resynchronise Makefile and include/linux/build_bug.h with Linux kernel sources by porting the following Linux commits into u-boot: - 6c49f359ca14 ("kbuild: disable sparse warnings about unknown attributes") - 80591e61a0f7 ("kbuild: tell sparse about the $ARCH") - 8788994376d8 ("linux/build_bug.h: change type to int") - 527edbc18a70 ("build_bug.h: remove most of dummy BUILD_BUG_ON stubs for Sparse") - c60d3b79423a ("build_bug.h: remove negative-array fallback for BUILD_BUG_ON()") - 14e83077d55f ("include: drop pointless __compiler_offsetof indirection") Also revert commit aa9e891c63 ("include/linux/stddef.h: avoid 'warning: preprocessor token offsetof redefined'") because the error it creates is worse than the warning it is trying to fix. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- Makefile | 6 +++++- include/linux/build_bug.h | 40 ++++++++------------------------------- include/linux/stddef.h | 8 +------- 3 files changed, 14 insertions(+), 40 deletions(-)