* [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT @ 2022-09-07 4:59 Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions Nick Desaulniers ` (4 more replies) 0 siblings, 5 replies; 53+ messages in thread From: Nick Desaulniers @ 2022-09-07 4:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Nathan Chancellor, Tom Rix, linux-kbuild, linux-kernel, llvm, x86, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Nick Desaulniers Alexey reported that the fraction of unknown filename instances in kallsyms grew from ~0.3% to ~10% recently; Bill and Greg tracked it down to assembler defined symbols, which regressed as a result of: commit b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") In that commit, I allude to restoring debug info for assembler defined symbols in a follow up patch, but it seems I forgot to do so in commit a66049e2cf0e ("Kbuild: make DWARF version a choice") Do so requires a fixup for as-option, which seems to be failing when used in scripts/Makefile.debug. Also includes a fix for DEBUG_INFO_SPLIT while I'm here. Dmitrii reports that this has been broken since gcc-11+ & clang-12+. I'm guessing no one uses this .config option...since no one else has reported it being broken yet... Changes from v2 -> v3: * Pick up Nathan's Reviewed-by and Tested-by tags. * Reword commit message 1/5 and 2/5 as per Nathan. * Add Masahiro's Suggested-by tag where relevant. * Use -x assembler-with-cpp -Werror in as-option and as-instr as per Masahiro. * Fix AMDGPU compiler version check as per Nathan. * Drop cc-min-version, as per Masahiro. * Replace patch 4 with a variant of Masahiro's suggestion. * Reword commit message 4/5. Changes from v1 -> v2: * 5 patches now, rather than 3. * Split change to arch/x86/boot/compressed/Makefile off of first patch, as per Masahiro. * Introduce compiler specific macros, as per Bill, and eradicate cc-ifversion while I'm at it. * Update commit message of final patch to refer to 866ced950bcd. v2: https://lore.kernel.org/llvm/20220831184408.2778264-1-ndesaulniers@google.com/ v1: https://lore.kernel.org/llvm/20220826181035.859042-1-ndesaulniers@google.com/ *** BLURB HERE *** Nick Desaulniers (5): x86/boot/compressed: prefer cc-option for CFLAGS additions Makefile.compiler: Use KBUILD_AFLAGS for as-option Makefile.compiler: replace cc-ifversion with compiler-specific macros Makefile.debug: re-enable debug info for .S files Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- Makefile | 6 ++--- arch/x86/boot/compressed/Makefile | 2 +- drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- lib/Kconfig.debug | 4 ++- scripts/Makefile.compiler | 18 ++++++++----- scripts/Makefile.debug | 25 ++++++++---------- 7 files changed, 46 insertions(+), 40 deletions(-) -- 2.37.2.789.g6183377224-goog ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions 2022-09-07 4:59 [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT Nick Desaulniers @ 2022-09-07 4:59 ` Nick Desaulniers 2022-09-09 21:05 ` Masahiro Yamada 2022-09-07 4:59 ` [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option Nick Desaulniers ` (3 subsequent siblings) 4 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-07 4:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Nathan Chancellor, Tom Rix, linux-kbuild, linux-kernel, llvm, x86, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Nick Desaulniers, Arvind Sankar as-option tests new options using KBUILD_CFLAGS, which causes problems when using as-option to update KBUILD_AFLAGS because many compiler options are not valid assembler options. This is being fixed in a follow up patch. Before doing so, move the assembler test for -Wa,-mrelax-relocations=no from using as-option to cc-option. Cc: Arvind Sankar <nivedita@alum.mit.edu> Cc: x86@kernel.org Link: https://lore.kernel.org/llvm/CAK7LNATcHt7GcXZ=jMszyH=+M_LC9Qr6yeAGRCBbE6xriLxtUQ@mail.gmail.com/ Suggested-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Tested-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v2 -> v3: * Reword commit message as per Nathan. * Pick up Nathan's RB/TB tags. Changes v1 -> v2: * Split off of v1 [1/3]. * Use cc-option to update CFLAGS, as per Masahiro. * Add Masahiro's Suggested-by, Cc Arvind. arch/x86/boot/compressed/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 35ce1a64068b..85934204d905 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -49,7 +49,7 @@ KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) KBUILD_CFLAGS += -fno-asynchronous-unwind-tables KBUILD_CFLAGS += -D__DISABLE_EXPORTS # Disable relocation relaxation in case the link is not PIE. -KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no) +KBUILD_CFLAGS += $(call cc-option,-Wa$(comma)-mrelax-relocations=no) KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h # sev.c indirectly inludes inat-table.h which is generated during -- 2.37.2.789.g6183377224-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions 2022-09-07 4:59 ` [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions Nick Desaulniers @ 2022-09-09 21:05 ` Masahiro Yamada 0 siblings, 0 replies; 53+ messages in thread From: Masahiro Yamada @ 2022-09-09 21:05 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Nathan Chancellor, Tom Rix, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, X86 ML, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Arvind Sankar On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > as-option tests new options using KBUILD_CFLAGS, which causes problems > when using as-option to update KBUILD_AFLAGS because many compiler > options are not valid assembler options. > > This is being fixed in a follow up patch. Before doing so, move the > assembler test for -Wa,-mrelax-relocations=no from using as-option to > cc-option. > > Cc: Arvind Sankar <nivedita@alum.mit.edu> > Cc: x86@kernel.org > Link: https://lore.kernel.org/llvm/CAK7LNATcHt7GcXZ=jMszyH=+M_LC9Qr6yeAGRCBbE6xriLxtUQ@mail.gmail.com/ > Suggested-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Tested-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> 01 and 02, applied to linux-kbuild/fixes. Thanks. > --- > Changes v2 -> v3: > * Reword commit message as per Nathan. > * Pick up Nathan's RB/TB tags. > > Changes v1 -> v2: > * Split off of v1 [1/3]. > * Use cc-option to update CFLAGS, as per Masahiro. > * Add Masahiro's Suggested-by, Cc Arvind. > > arch/x86/boot/compressed/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > index 35ce1a64068b..85934204d905 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -49,7 +49,7 @@ KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) > KBUILD_CFLAGS += -fno-asynchronous-unwind-tables > KBUILD_CFLAGS += -D__DISABLE_EXPORTS > # Disable relocation relaxation in case the link is not PIE. > -KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no) > +KBUILD_CFLAGS += $(call cc-option,-Wa$(comma)-mrelax-relocations=no) > KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h > > # sev.c indirectly inludes inat-table.h which is generated during > -- > 2.37.2.789.g6183377224-goog > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option 2022-09-07 4:59 [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions Nick Desaulniers @ 2022-09-07 4:59 ` Nick Desaulniers 2022-09-09 21:06 ` Masahiro Yamada 2022-09-07 4:59 ` [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers ` (2 subsequent siblings) 4 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-07 4:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Nathan Chancellor, Tom Rix, linux-kbuild, linux-kernel, llvm, x86, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Nick Desaulniers as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS. This can cause as-option to fail unexpectedly when CONFIG_WERROR is set, because clang will emit -Werror,-Wunused-command-line-argument for various -m and -f flags for assembler sources. Callers of as-option (and as-instr) likely want to be adding flags to KBUILD_AFLAGS/aflags-y, not KBUILD_CFLAGS/cflags-y. Also, change as-option and as-instr to use -x assembler-with-cpp since kernel sources are .S files that use the compiler as the driver. And then add -Werror as well. Link: https://github.com/ClangBuiltLinux/linux/issues/1699 Suggested-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v2 -> v1: * Add -x assembler-with-cpp -Werror to both as-option and (new) as-instr, as per Masahiro. * Add Masahiro's SB tag. Changes v1 -> v2: * Split off changes to arch/x86/boot/compressed/Makefile into parent patch, as per Masahiro. scripts/Makefile.compiler | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 94d0d40cddb3..a66638b5f4a5 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -29,16 +29,16 @@ try-run = $(shell set -e; \ fi) # as-option -# Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,) +# Usage: aflags-y += $(call as-option,-Wa$(comma)-isa=foo,) as-option = $(call try-run,\ - $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2)) + $(CC) -Werror $(KBUILD_AFLAGS) $(1) -c -x assembler-with-cpp /dev/null -o "$$TMP",$(1),$(2)) # as-instr -# Usage: cflags-y += $(call as-instr,instr,option1,option2) +# Usage: aflags-y += $(call as-instr,instr,option1,option2) as-instr = $(call try-run,\ - printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3)) + printf "%b\n" "$(1)" | $(CC) -Werror $(KBUILD_AFLAGS) -c -x assembler-with-cpp -o "$$TMP" -,$(2),$(3)) # __cc-option # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586) -- 2.37.2.789.g6183377224-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option 2022-09-07 4:59 ` [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option Nick Desaulniers @ 2022-09-09 21:06 ` Masahiro Yamada 0 siblings, 0 replies; 53+ messages in thread From: Masahiro Yamada @ 2022-09-09 21:06 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Nathan Chancellor, Tom Rix, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, X86 ML, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS. This can > cause as-option to fail unexpectedly when CONFIG_WERROR is set, because > clang will emit -Werror,-Wunused-command-line-argument for various -m > and -f flags for assembler sources. > > Callers of as-option (and as-instr) likely want to be adding flags to > KBUILD_AFLAGS/aflags-y, not KBUILD_CFLAGS/cflags-y. > > Also, change as-option and as-instr to use -x assembler-with-cpp since > kernel sources are .S files that use the compiler as the driver. And > then add -Werror as well. > > Link: https://github.com/ClangBuiltLinux/linux/issues/1699 > Suggested-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- 01 and 02, applied to linux-kbuild/fixes. Thanks. > Changes v2 -> v1: > * Add -x assembler-with-cpp -Werror to both as-option and (new) > as-instr, as per Masahiro. > * Add Masahiro's SB tag. > > Changes v1 -> v2: > * Split off changes to arch/x86/boot/compressed/Makefile into parent > patch, as per Masahiro. > > scripts/Makefile.compiler | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > index 94d0d40cddb3..a66638b5f4a5 100644 > --- a/scripts/Makefile.compiler > +++ b/scripts/Makefile.compiler > @@ -29,16 +29,16 @@ try-run = $(shell set -e; \ > fi) > > # as-option > -# Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,) > +# Usage: aflags-y += $(call as-option,-Wa$(comma)-isa=foo,) > > as-option = $(call try-run,\ > - $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2)) > + $(CC) -Werror $(KBUILD_AFLAGS) $(1) -c -x assembler-with-cpp /dev/null -o "$$TMP",$(1),$(2)) > > # as-instr > -# Usage: cflags-y += $(call as-instr,instr,option1,option2) > +# Usage: aflags-y += $(call as-instr,instr,option1,option2) > > as-instr = $(call try-run,\ > - printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3)) > + printf "%b\n" "$(1)" | $(CC) -Werror $(KBUILD_AFLAGS) -c -x assembler-with-cpp -o "$$TMP" -,$(2),$(3)) > > # __cc-option > # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586) > -- > 2.37.2.789.g6183377224-goog > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-07 4:59 [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option Nick Desaulniers @ 2022-09-07 4:59 ` Nick Desaulniers 2022-09-09 21:59 ` Masahiro Yamada 2022-09-07 4:59 ` [PATCH v3 4/5] Makefile.debug: re-enable debug info for .S files Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers 4 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-07 4:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Nathan Chancellor, Tom Rix, linux-kbuild, linux-kernel, llvm, x86, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Nick Desaulniers cc-ifversion is GCC specific. Replace it with compiler specific variants. Update the users of cc-ifversion to use these new macros. Link: https://github.com/ClangBuiltLinux/linux/issues/350 Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ Suggested-by: Bill Wendling <morbo@google.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v2 -> v3: * Fix AMDGPU -msse flag, as per Nathan. * Pick up Nathan's RB tag. * Drop cc-min-version, as per Masahiro. Changes v1 -> v2: * New patch. Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- Makefile | 6 ++--- drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- scripts/Makefile.compiler | 10 ++++--- 4 files changed, 27 insertions(+), 20 deletions(-) diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst index 11a296e52d68..ee7e3ea1fbe1 100644 --- a/Documentation/kbuild/makefiles.rst +++ b/Documentation/kbuild/makefiles.rst @@ -682,22 +682,27 @@ more details, with real examples. In the above example, -Wno-unused-but-set-variable will be added to KBUILD_CFLAGS only if gcc really accepts it. - cc-ifversion - cc-ifversion tests the version of $(CC) and equals the fourth parameter - if version expression is true, or the fifth (if given) if the version - expression is false. + gcc-min-version + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than + or equal to the provided value and evaluates to y if so. Example:: - #fs/reiserfs/Makefile - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) + cflags-$(call gcc-min-version, 70100) := -foo - In this example, ccflags-y will be assigned the value -O1 if the - $(CC) version is less than 4.2. - cc-ifversion takes all the shell operators: - -eq, -ne, -lt, -le, -gt, and -ge - The third parameter may be a text as in this example, but it may also - be an expanded variable or a macro. + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and + $(CONFIG_GCC_VERSION) is >= 7.1. + + clang-min-version + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater + than or equal to the provided value and evaluates to y if so. + + Example:: + + cflags-$(call clang-min-version, 110000) := -foo + + In this example, cflags-y will be assigned the value -foo if $(CC) is clang + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. cc-cross-prefix cc-cross-prefix is used to check if there exists a $(CC) in path with diff --git a/Makefile b/Makefile index a4f71076cacb..cd9919c66b96 100644 --- a/Makefile +++ b/Makefile @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) ifdef CONFIG_CC_IS_CLANG KBUILD_CPPFLAGS += -Qunused-arguments @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC KBUILD_CFLAGS += -Wno-maybe-uninitialized endif -ifdef CONFIG_CC_IS_GCC # The allocators already balk at large sizes, so silence the compiler # warnings for bounds checks involving those possible values. While # -Wno-alloc-size-larger-than would normally be used here, earlier versions @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC # ignored, continuing to default to PTRDIFF_MAX. So, left with no other # choice, we must perform a versioned check to disable this warning. # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) -endif +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) # disable invalid "can't wrap" optimizations for signed / pointers KBUILD_CFLAGS += -fno-strict-overflow diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index 86a3b5bfd699..3c64ae0b212c 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec endif ifdef CONFIG_CC_IS_GCC -ifeq ($(call cc-ifversion, -lt, 0701, y), y) +ifneq ($(call gcc-min-version, 70100),y) IS_OLD_GCC = 1 endif endif diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index a66638b5f4a5..4b998dadd79d 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ cc-disable-warning = $(call try-run,\ $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) -# cc-ifversion -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) +# gcc-min-version +# Usage: cflags-$(call gcc-min-version, 70100) += -foo +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) + +# clang-min-version +# Usage: cflags-$(call clang-min-version, 110000) += -foo +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) # ld-option # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) -- 2.37.2.789.g6183377224-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-07 4:59 ` [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers @ 2022-09-09 21:59 ` Masahiro Yamada 0 siblings, 0 replies; 53+ messages in thread From: Masahiro Yamada @ 2022-09-09 21:59 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Nathan Chancellor, Tom Rix, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, X86 ML, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > cc-ifversion is GCC specific. Replace it with compiler specific > variants. Update the users of cc-ifversion to use these new macros. Now, this is independent of the rest of this series. Can you rebase this on top of Linus' tree so that you use clean up b0839b281c427e844143dba3893e25c83cdd6c17 Otherwise, clang-min-version will not get any users. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3 4/5] Makefile.debug: re-enable debug info for .S files 2022-09-07 4:59 [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT Nick Desaulniers ` (2 preceding siblings ...) 2022-09-07 4:59 ` [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers @ 2022-09-07 4:59 ` Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers 4 siblings, 0 replies; 53+ messages in thread From: Nick Desaulniers @ 2022-09-07 4:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Nathan Chancellor, Tom Rix, linux-kbuild, linux-kernel, llvm, x86, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Nick Desaulniers Alexey reported that the fraction of unknown filename instances in kallsyms grew from ~0.3% to ~10% recently; Bill and Greg tracked it down to assembler defined symbols, which regressed as a result of: commit b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") In that commit, I allude to restoring debug info for assembler defined symbols in a follow up patch, but it seems I forgot to do so in commit a66049e2cf0e ("Kbuild: make DWARF version a choice") Link: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=31bf18645d98b4d3d7357353be840e320649a67d Fixes: b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") Reported-by: Alexey Alexandrov <aalexand@google.com> Reported-by: Bill Wendling <morbo@google.com> Reported-by: Greg Thelen <gthelen@google.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Suggested-by: Masahiro Yamada <masahiroy@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v2 -> v3: * Replace diff outright with Masahiro's suggestion in https://lore.kernel.org/llvm/CAK7LNATWDH01=ZKLnsxc0vcib1zGDbEq8jLQwhWP7HkkmSb_Mw@mail.gmail.com/2-dwarf.diff with some modifications, PTAL. * Pick up Nathan's RB tag and Masahiro's SB tag. * Cut down commit message. Changes v1 -> v2: * Use newly added compiler-specific macros, as per Bill. lib/Kconfig.debug | 4 +++- scripts/Makefile.debug | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index bcbe60d6c80c..d3e5f36bb01e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -264,8 +264,10 @@ config DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT config DEBUG_INFO_DWARF4 bool "Generate DWARF Version 4 debuginfo" select DEBUG_INFO + depends on !CC_IS_CLANG || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502))) help - Generate DWARF v4 debug info. This requires gcc 4.5+ and gdb 7.0+. + Generate DWARF v4 debug info. This requires gcc 4.5+, binutils 2.35.2 + if using clang without clang's integrated assembler, and gdb 7.0+. If you have consumers of DWARF debug info that are not ready for newer revisions of DWARF, you may wish to choose this or have your diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug index 9f39b0130551..2845145c1220 100644 --- a/scripts/Makefile.debug +++ b/scripts/Makefile.debug @@ -3,17 +3,16 @@ DEBUG_CFLAGS := ifdef CONFIG_DEBUG_INFO_SPLIT DEBUG_CFLAGS += -gsplit-dwarf else -DEBUG_CFLAGS += -g +debug-cflags-y += -g endif -ifndef CONFIG_AS_IS_LLVM -KBUILD_AFLAGS += -Wa,-gdwarf-2 -endif +debug-flags-$(CONFIG_DEBUG_INFO_DWARF4) += -gdwarf-4 +debug-flags-$(CONFIG_DEBUG_INFO_DWARF5) += -gdwarf-5 -ifndef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT -dwarf-version-$(CONFIG_DEBUG_INFO_DWARF4) := 4 -dwarf-version-$(CONFIG_DEBUG_INFO_DWARF5) := 5 -DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y) +ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_AS_IS_GNU),yy) +# Clang does not pass -g or -gdwarf-* option down to GAS. +# Add -Wa, prefix to explicitly specify the flags. +KBUILD_AFLAGS += $(addprefix -Wa$(comma), $(debug-flags-y)) endif ifdef CONFIG_DEBUG_INFO_REDUCED @@ -29,5 +28,7 @@ KBUILD_AFLAGS += -gz=zlib KBUILD_LDFLAGS += --compress-debug-sections=zlib endif -KBUILD_CFLAGS += $(DEBUG_CFLAGS) +DEBUG_CFLAGS += $(debug-flags-y) +KBUILD_AFLAGS += $(debug-flags-y) +KBUILD_CFLAGS += $(DEBUG_CFLAGS) export DEBUG_CFLAGS -- 2.37.2.789.g6183377224-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT 2022-09-07 4:59 [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT Nick Desaulniers ` (3 preceding siblings ...) 2022-09-07 4:59 ` [PATCH v3 4/5] Makefile.debug: re-enable debug info for .S files Nick Desaulniers @ 2022-09-07 4:59 ` Nick Desaulniers 2022-09-09 20:56 ` Masahiro Yamada 4 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-07 4:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Nathan Chancellor, Tom Rix, linux-kbuild, linux-kernel, llvm, x86, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Nick Desaulniers, Andi Kleen Dmitrii, Fangrui, and Mashahiro note: Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2. Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g specified in order for -gsplit-dwarf to work at all. -gsplit-dwarf has been mutually exclusive with -g since support for CONFIG_DEBUG_INFO_SPLIT was introduced in commit 866ced950bcd ("kbuild: Support split debug info v4") I don't think it ever needed to be. Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/ Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/ Link: https://reviews.llvm.org/D80391 Cc: Andi Kleen <ak@linux.intel.com> Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> Reported-by: Fangrui Song <maskray@google.com> Reported-by: Masahiro Yamada <masahiroy@kernel.org> Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v2 -> v3: * Pick up Nathan's RB tag. Changes v1 -> v2: * Add reference to 866ced950bcd, cc Andi, in commit message. scripts/Makefile.debug | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug index 2845145c1220..c20f8f2e76bf 100644 --- a/scripts/Makefile.debug +++ b/scripts/Makefile.debug @@ -1,10 +1,4 @@ -DEBUG_CFLAGS := - -ifdef CONFIG_DEBUG_INFO_SPLIT -DEBUG_CFLAGS += -gsplit-dwarf -else -debug-cflags-y += -g -endif +DEBUG_CFLAGS := -g debug-flags-$(CONFIG_DEBUG_INFO_DWARF4) += -gdwarf-4 debug-flags-$(CONFIG_DEBUG_INFO_DWARF5) += -gdwarf-5 @@ -15,6 +9,8 @@ ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_AS_IS_GNU),yy) KBUILD_AFLAGS += $(addprefix -Wa$(comma), $(debug-flags-y)) endif +debug-flags-$(CONFIG_DEBUG_INFO_SPLIT) += -gsplit-dwarf + ifdef CONFIG_DEBUG_INFO_REDUCED DEBUG_CFLAGS += -fno-var-tracking ifdef CONFIG_CC_IS_GCC -- 2.37.2.789.g6183377224-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT 2022-09-07 4:59 ` [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers @ 2022-09-09 20:56 ` Masahiro Yamada 2022-09-09 21:02 ` Masahiro Yamada 0 siblings, 1 reply; 53+ messages in thread From: Masahiro Yamada @ 2022-09-09 20:56 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Nathan Chancellor, Tom Rix, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, X86 ML, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Andi Kleen On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > Dmitrii, Fangrui, and Mashahiro note: > > Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2. > > Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g > specified in order for -gsplit-dwarf to work at all. > > -gsplit-dwarf has been mutually exclusive with -g since support for > CONFIG_DEBUG_INFO_SPLIT was introduced in > commit 866ced950bcd ("kbuild: Support split debug info v4") > I don't think it ever needed to be. > > Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/ > Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/ > Link: https://reviews.llvm.org/D80391 > Cc: Andi Kleen <ak@linux.intel.com> > Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> > Reported-by: Fangrui Song <maskray@google.com> > Reported-by: Masahiro Yamada <masahiroy@kernel.org> > Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes v2 -> v3: > * Pick up Nathan's RB tag. > > Changes v1 -> v2: > * Add reference to 866ced950bcd, cc Andi, in commit message. > > scripts/Makefile.debug | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 2845145c1220..c20f8f2e76bf 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,10 +1,4 @@ > -DEBUG_CFLAGS := > - > -ifdef CONFIG_DEBUG_INFO_SPLIT > -DEBUG_CFLAGS += -gsplit-dwarf > -else > -debug-cflags-y += -g > -endif > +DEBUG_CFLAGS := -g > > debug-flags-$(CONFIG_DEBUG_INFO_DWARF4) += -gdwarf-4 > debug-flags-$(CONFIG_DEBUG_INFO_DWARF5) += -gdwarf-5 > @@ -15,6 +9,8 @@ ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_AS_IS_GNU),yy) > KBUILD_AFLAGS += $(addprefix -Wa$(comma), $(debug-flags-y)) > endif > > +debug-flags-$(CONFIG_DEBUG_INFO_SPLIT) += -gsplit-dwarf > + This patch changes the behavior that is not mentioned in the commit log. Previously, -gsplit-dwarf was passed only when compiling *.c files. (DEBUG_CFLAGS). Now it is passed also when compiling *.S files. (debug-flags-y is appended to KBUILD_AFLAGS) Please confirm if this makes sense, and if so, please mention it in the commit log. As far as I tested, I did not see this change was useful. When *.S is compiled to *.o, *.dwo is also created, but it does not contain any debug info. > ifdef CONFIG_DEBUG_INFO_REDUCED > DEBUG_CFLAGS += -fno-var-tracking > ifdef CONFIG_CC_IS_GCC > -- > 2.37.2.789.g6183377224-goog > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT 2022-09-09 20:56 ` Masahiro Yamada @ 2022-09-09 21:02 ` Masahiro Yamada 2022-09-19 17:08 ` [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers ` (2 more replies) 0 siblings, 3 replies; 53+ messages in thread From: Masahiro Yamada @ 2022-09-09 21:02 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Nathan Chancellor, Tom Rix, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, X86 ML, Dmitrii Bundin, Fangrui Song, Alexey Alexandrov, Bill Wendling, Greg Thelen, Andi Kleen On Sat, Sep 10, 2022 at 5:56 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > Dmitrii, Fangrui, and Mashahiro note: > > > > Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2. > > > > Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g > > specified in order for -gsplit-dwarf to work at all. > > > > -gsplit-dwarf has been mutually exclusive with -g since support for > > CONFIG_DEBUG_INFO_SPLIT was introduced in > > commit 866ced950bcd ("kbuild: Support split debug info v4") > > I don't think it ever needed to be. > > > > Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/ > > Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/ > > Link: https://reviews.llvm.org/D80391 > > Cc: Andi Kleen <ak@linux.intel.com> > > Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> > > Reported-by: Fangrui Song <maskray@google.com> > > Reported-by: Masahiro Yamada <masahiroy@kernel.org> > > Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > Changes v2 -> v3: > > * Pick up Nathan's RB tag. > > > > Changes v1 -> v2: > > * Add reference to 866ced950bcd, cc Andi, in commit message. > > > > scripts/Makefile.debug | 10 +++------- > > 1 file changed, 3 insertions(+), 7 deletions(-) > > > > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > > index 2845145c1220..c20f8f2e76bf 100644 > > --- a/scripts/Makefile.debug > > +++ b/scripts/Makefile.debug > > @@ -1,10 +1,4 @@ > > -DEBUG_CFLAGS := > > - > > -ifdef CONFIG_DEBUG_INFO_SPLIT > > -DEBUG_CFLAGS += -gsplit-dwarf > > -else > > -debug-cflags-y += -g > > -endif > > +DEBUG_CFLAGS := -g > > > > debug-flags-$(CONFIG_DEBUG_INFO_DWARF4) += -gdwarf-4 > > debug-flags-$(CONFIG_DEBUG_INFO_DWARF5) += -gdwarf-5 > > @@ -15,6 +9,8 @@ ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_AS_IS_GNU),yy) > > KBUILD_AFLAGS += $(addprefix -Wa$(comma), $(debug-flags-y)) > > endif > > > > +debug-flags-$(CONFIG_DEBUG_INFO_SPLIT) += -gsplit-dwarf > > + > > > This patch changes the behavior that is not mentioned in the commit log. > > > > > Previously, -gsplit-dwarf was passed only when compiling *.c files. > (DEBUG_CFLAGS). > > Now it is passed also when compiling *.S files. > (debug-flags-y is appended to KBUILD_AFLAGS) > > > Please confirm if this makes sense, and if so, > please mention it in the commit log. > > > > As far as I tested, I did not see this change was useful. > > When *.S is compiled to *.o, *.dwo is also created, > but it does not contain any debug info. > > > > > ifdef CONFIG_DEBUG_INFO_REDUCED > > DEBUG_CFLAGS += -fno-var-tracking > > ifdef CONFIG_CC_IS_GCC > > -- > > 2.37.2.789.g6183377224-goog > > > > > -- > Best Regards > Masahiro Yamada BTW, you do not need to resend the entire series even if you end up with v4. - 01-02 - 03 - 04-05 are mutually independent of the others now. They can be reviewed separately. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-09 21:02 ` Masahiro Yamada @ 2022-09-19 17:08 ` Nick Desaulniers 2022-09-23 19:44 ` Masahiro Yamada 2023-04-27 11:53 ` Shreeya Patel 2022-09-19 17:30 ` [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers 2022-09-19 17:45 ` [PATCH v4] Makefile.debug: re-enable debug info for .S files Nick Desaulniers 2 siblings, 2 replies; 53+ messages in thread From: Nick Desaulniers @ 2022-09-19 17:08 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers, Bill Wendling, Nathan Chancellor cc-ifversion is GCC specific. Replace it with compiler specific variants. Update the users of cc-ifversion to use these new macros. Link: https://github.com/ClangBuiltLinux/linux/issues/350 Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ Suggested-by: Bill Wendling <morbo@google.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v3 -> v4: * Split into its own patch again from series, as per Masahiro. * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- Makefile | 6 ++--- drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- scripts/Makefile.compiler | 10 ++++--- scripts/Makefile.extrawarn | 4 +-- 5 files changed, 29 insertions(+), 22 deletions(-) diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst index 11a296e52d68..ee7e3ea1fbe1 100644 --- a/Documentation/kbuild/makefiles.rst +++ b/Documentation/kbuild/makefiles.rst @@ -682,22 +682,27 @@ more details, with real examples. In the above example, -Wno-unused-but-set-variable will be added to KBUILD_CFLAGS only if gcc really accepts it. - cc-ifversion - cc-ifversion tests the version of $(CC) and equals the fourth parameter - if version expression is true, or the fifth (if given) if the version - expression is false. + gcc-min-version + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than + or equal to the provided value and evaluates to y if so. Example:: - #fs/reiserfs/Makefile - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) + cflags-$(call gcc-min-version, 70100) := -foo - In this example, ccflags-y will be assigned the value -O1 if the - $(CC) version is less than 4.2. - cc-ifversion takes all the shell operators: - -eq, -ne, -lt, -le, -gt, and -ge - The third parameter may be a text as in this example, but it may also - be an expanded variable or a macro. + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and + $(CONFIG_GCC_VERSION) is >= 7.1. + + clang-min-version + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater + than or equal to the provided value and evaluates to y if so. + + Example:: + + cflags-$(call clang-min-version, 110000) := -foo + + In this example, cflags-y will be assigned the value -foo if $(CC) is clang + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. cc-cross-prefix cc-cross-prefix is used to check if there exists a $(CC) in path with diff --git a/Makefile b/Makefile index 298f69060f10..411c8480b37e 100644 --- a/Makefile +++ b/Makefile @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) ifdef CONFIG_CC_IS_CLANG KBUILD_CPPFLAGS += -Qunused-arguments @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC KBUILD_CFLAGS += -Wno-maybe-uninitialized endif -ifdef CONFIG_CC_IS_GCC # The allocators already balk at large sizes, so silence the compiler # warnings for bounds checks involving those possible values. While # -Wno-alloc-size-larger-than would normally be used here, earlier versions @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC # ignored, continuing to default to PTRDIFF_MAX. So, left with no other # choice, we must perform a versioned check to disable this warning. # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) -endif +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) # disable invalid "can't wrap" optimizations for signed / pointers KBUILD_CFLAGS += -fno-strict-overflow diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index cb81ed2fbd53..d70838edba80 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec endif ifdef CONFIG_CC_IS_GCC -ifeq ($(call cc-ifversion, -lt, 0701, y), y) +ifneq ($(call gcc-min-version, 70100),y) IS_OLD_GCC = 1 endif endif diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 94d0d40cddb3..9d18fb91890e 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ cc-disable-warning = $(call try-run,\ $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) -# cc-ifversion -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) +# gcc-min-version +# Usage: cflags-$(call gcc-min-version, 70100) += -foo +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) + +# clang-min-version +# Usage: cflags-$(call clang-min-version, 110000) += -foo +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) # ld-option # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 6ae482158bc4..5769c1939d40 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -48,7 +48,7 @@ else ifdef CONFIG_CC_IS_CLANG KBUILD_CFLAGS += -Wno-initializer-overrides # Clang before clang-16 would warn on default argument promotions. -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) +ifneq ($(call clang-min-version, 160000),y) # Disable -Wformat KBUILD_CFLAGS += -Wno-format # Then re-enable flags that were part of the -Wformat group that aren't @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull # Requires clang-12+. -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) +ifeq ($(call clang-min-version, 120000),y) KBUILD_CFLAGS += -Wformat-insufficient-args endif endif -- 2.37.3.968.ga6b4b080e4-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-19 17:08 ` [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers @ 2022-09-23 19:44 ` Masahiro Yamada 2022-09-24 14:28 ` Masahiro Yamada 2023-04-27 11:53 ` Shreeya Patel 1 sibling, 1 reply; 53+ messages in thread From: Masahiro Yamada @ 2022-09-23 19:44 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor On Tue, Sep 20, 2022 at 2:08 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > cc-ifversion is GCC specific. Replace it with compiler specific > variants. Update the users of cc-ifversion to use these new macros. > > Link: https://github.com/ClangBuiltLinux/linux/issues/350 > Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > Suggested-by: Bill Wendling <morbo@google.com> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes v3 -> v4: > * Split into its own patch again from series, as per Masahiro. > * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. Applied to linux-kbuild. Thanks. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-23 19:44 ` Masahiro Yamada @ 2022-09-24 14:28 ` Masahiro Yamada 2022-09-25 1:22 ` Masahiro Yamada 0 siblings, 1 reply; 53+ messages in thread From: Masahiro Yamada @ 2022-09-24 14:28 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor On Sat, Sep 24, 2022 at 4:44 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Sep 20, 2022 at 2:08 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > cc-ifversion is GCC specific. Replace it with compiler specific > > variants. Update the users of cc-ifversion to use these new macros. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/350 > > Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > > Suggested-by: Bill Wendling <morbo@google.com> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > Changes v3 -> v4: > > * Split into its own patch again from series, as per Masahiro. > > * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > > clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. > > > Applied to linux-kbuild. > Thanks. > > > > -- > Best Regards > Masahiro Yamada I noticed a small flaw now. $ make mrproper; make /bin/sh: 1: [: -ge: unexpected operator *** *** Configuration file ".config" not found! *** *** Please run some configurator (e.g. "make oldconfig" or *** "make menuconfig" or "make xconfig"). *** Makefile:711: include/config/auto.conf.cmd: No such file or directory make: *** [Makefile:720: .config] Error 1 This fails anyway, but it shows annoying /bin/sh: 1: [: -ge: unexpected operator It is emit by this line: KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than When $(CONFIG_GCC_VERSION) is empty, it becomes invalid shell code: [ -ge $(1) ] && echo y Now I just recalled why I wrote the original code like this: cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] ... -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-24 14:28 ` Masahiro Yamada @ 2022-09-25 1:22 ` Masahiro Yamada 2022-09-26 21:41 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Masahiro Yamada @ 2022-09-25 1:22 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor On Sat, Sep 24, 2022 at 11:28 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sat, Sep 24, 2022 at 4:44 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Tue, Sep 20, 2022 at 2:08 AM Nick Desaulniers > > <ndesaulniers@google.com> wrote: > > > > > > cc-ifversion is GCC specific. Replace it with compiler specific > > > variants. Update the users of cc-ifversion to use these new macros. > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/350 > > > Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > > > Suggested-by: Bill Wendling <morbo@google.com> > > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > > Changes v3 -> v4: > > > * Split into its own patch again from series, as per Masahiro. > > > * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > > > clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. > > > > > > Applied to linux-kbuild. > > Thanks. > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > > > > > > I noticed a small flaw now. > > > > $ make mrproper; make > /bin/sh: 1: [: -ge: unexpected operator > *** > *** Configuration file ".config" not found! > *** > *** Please run some configurator (e.g. "make oldconfig" or > *** "make menuconfig" or "make xconfig"). > *** > Makefile:711: include/config/auto.conf.cmd: No such file or directory > make: *** [Makefile:720: .config] Error 1 > > > > > > > > > This fails anyway, but it shows annoying > > /bin/sh: 1: [: -ge: unexpected operator > > > > It is emit by this line: > > KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than > > > > > When $(CONFIG_GCC_VERSION) is empty, it becomes invalid shell code: > > [ -ge $(1) ] && echo y > > > > > > Now I just recalled why I wrote the original code like this: > > > cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] ... > > > > > -- > Best Regards > Masahiro Yamada I squashed the following code diff. Please let me know if there is a problem. diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 9d18fb91890e..20d353dcabfb 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -63,11 +63,11 @@ cc-disable-warning = $(call try-run,\ # gcc-min-version # Usage: cflags-$(call gcc-min-version, 70100) += -foo -gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION)0 -ge $(1)0 ] && echo y) # clang-min-version # Usage: cflags-$(call clang-min-version, 110000) += -foo -clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION)0 -ge $(1)0 ] && echo y) # ld-option # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) -- Best Regards Masahiro Yamada ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-25 1:22 ` Masahiro Yamada @ 2022-09-26 21:41 ` Nick Desaulniers 0 siblings, 0 replies; 53+ messages in thread From: Nick Desaulniers @ 2022-09-26 21:41 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor On Sat, Sep 24, 2022 at 6:23 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > I noticed a small flaw now. > > > > > > > > $ make mrproper; make > > /bin/sh: 1: [: -ge: unexpected operator > > *** > > *** Configuration file ".config" not found! > > *** > > *** Please run some configurator (e.g. "make oldconfig" or > > *** "make menuconfig" or "make xconfig"). > > *** > > Makefile:711: include/config/auto.conf.cmd: No such file or directory > > make: *** [Makefile:720: .config] Error 1 > > > > > > > > > > > > > > > > > > This fails anyway, but it shows annoying > > > > /bin/sh: 1: [: -ge: unexpected operator > > > > > > > > It is emit by this line: > > > > KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than > > > > > > > > > > When $(CONFIG_GCC_VERSION) is empty, it becomes invalid shell code: > > > > [ -ge $(1) ] && echo y > > > > > > > > > > > > Now I just recalled why I wrote the original code like this: > > > > > > cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] ... > > > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > > > > > > > I squashed the following code diff. > Please let me know if there is a problem. LGTM; thanks (again). > > > > > > > > > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > index 9d18fb91890e..20d353dcabfb 100644 > --- a/scripts/Makefile.compiler > +++ b/scripts/Makefile.compiler > @@ -63,11 +63,11 @@ cc-disable-warning = $(call try-run,\ > > # gcc-min-version > # Usage: cflags-$(call gcc-min-version, 70100) += -foo > -gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) > +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION)0 -ge $(1)0 ] && echo y) > > # clang-min-version > # Usage: cflags-$(call clang-min-version, 110000) += -foo > -clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) > +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION)0 -ge $(1)0 ] && echo y) > > # ld-option > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) > > > > > > > -- > Best Regards > Masahiro Yamada > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2022-09-19 17:08 ` [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers 2022-09-23 19:44 ` Masahiro Yamada @ 2023-04-27 11:53 ` Shreeya Patel 2023-04-28 7:41 ` Thorsten Leemhuis 2023-04-28 17:27 ` Nick Desaulniers 1 sibling, 2 replies; 53+ messages in thread From: Shreeya Patel @ 2023-04-27 11:53 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions Hi Nick, On 19/09/22 22:38, Nick Desaulniers wrote: > cc-ifversion is GCC specific. Replace it with compiler specific > variants. Update the users of cc-ifversion to use these new macros. > > Link: https://github.com/ClangBuiltLinux/linux/issues/350 > Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > Suggested-by: Bill Wendling <morbo@google.com> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> KernelCI found this patch causes a regression in the baseline.logintest on qemu_arm-virt-gicv3-uefi [1], see the bisection report for more details [2]. Let me know if you have any questions. [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ [2] https://groups.io/g/kernelci-results/message/40804 Thanks, Shreeya Patel #regzbot introduced: 88b61e3bff93 > --- > Changes v3 -> v4: > * Split into its own patch again from series, as per Masahiro. > * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. > > Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- > Makefile | 6 ++--- > drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- > scripts/Makefile.compiler | 10 ++++--- > scripts/Makefile.extrawarn | 4 +-- > 5 files changed, 29 insertions(+), 22 deletions(-) > > diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst > index 11a296e52d68..ee7e3ea1fbe1 100644 > --- a/Documentation/kbuild/makefiles.rst > +++ b/Documentation/kbuild/makefiles.rst > @@ -682,22 +682,27 @@ more details, with real examples. > In the above example, -Wno-unused-but-set-variable will be added to > KBUILD_CFLAGS only if gcc really accepts it. > > - cc-ifversion > - cc-ifversion tests the version of $(CC) and equals the fourth parameter > - if version expression is true, or the fifth (if given) if the version > - expression is false. > + gcc-min-version > + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than > + or equal to the provided value and evaluates to y if so. > > Example:: > > - #fs/reiserfs/Makefile > - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) > + cflags-$(call gcc-min-version, 70100) := -foo > > - In this example, ccflags-y will be assigned the value -O1 if the > - $(CC) version is less than 4.2. > - cc-ifversion takes all the shell operators: > - -eq, -ne, -lt, -le, -gt, and -ge > - The third parameter may be a text as in this example, but it may also > - be an expanded variable or a macro. > + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and > + $(CONFIG_GCC_VERSION) is >= 7.1. > + > + clang-min-version > + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater > + than or equal to the provided value and evaluates to y if so. > + > + Example:: > + > + cflags-$(call clang-min-version, 110000) := -foo > + > + In this example, cflags-y will be assigned the value -foo if $(CC) is clang > + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. > > cc-cross-prefix > cc-cross-prefix is used to check if there exists a $(CC) in path with > diff --git a/Makefile b/Makefile > index 298f69060f10..411c8480b37e 100644 > --- a/Makefile > +++ b/Makefile > @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) > > KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror > KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds > -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > > ifdef CONFIG_CC_IS_CLANG > KBUILD_CPPFLAGS += -Qunused-arguments > @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC > KBUILD_CFLAGS += -Wno-maybe-uninitialized > endif > > -ifdef CONFIG_CC_IS_GCC > # The allocators already balk at large sizes, so silence the compiler > # warnings for bounds checks involving those possible values. While > # -Wno-alloc-size-larger-than would normally be used here, earlier versions > @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC > # ignored, continuing to default to PTRDIFF_MAX. So, left with no other > # choice, we must perform a versioned check to disable this warning. > # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au > -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) > -endif > +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than > +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > > # disable invalid "can't wrap" optimizations for signed / pointers > KBUILD_CFLAGS += -fno-strict-overflow > diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile > index cb81ed2fbd53..d70838edba80 100644 > --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile > @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec > endif > > ifdef CONFIG_CC_IS_GCC > -ifeq ($(call cc-ifversion, -lt, 0701, y), y) > +ifneq ($(call gcc-min-version, 70100),y) > IS_OLD_GCC = 1 > endif > endif > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > index 94d0d40cddb3..9d18fb91890e 100644 > --- a/scripts/Makefile.compiler > +++ b/scripts/Makefile.compiler > @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ > cc-disable-warning = $(call try-run,\ > $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) > > -# cc-ifversion > -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) > -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) > +# gcc-min-version > +# Usage: cflags-$(call gcc-min-version, 70100) += -foo > +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) > + > +# clang-min-version > +# Usage: cflags-$(call clang-min-version, 110000) += -foo > +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) > > # ld-option > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > index 6ae482158bc4..5769c1939d40 100644 > --- a/scripts/Makefile.extrawarn > +++ b/scripts/Makefile.extrawarn > @@ -48,7 +48,7 @@ else > ifdef CONFIG_CC_IS_CLANG > KBUILD_CFLAGS += -Wno-initializer-overrides > # Clang before clang-16 would warn on default argument promotions. > -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) > +ifneq ($(call clang-min-version, 160000),y) > # Disable -Wformat > KBUILD_CFLAGS += -Wno-format > # Then re-enable flags that were part of the -Wformat group that aren't > @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format > KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier > KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull > # Requires clang-12+. > -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) > +ifeq ($(call clang-min-version, 120000),y) > KBUILD_CFLAGS += -Wformat-insufficient-args > endif > endif ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-04-27 11:53 ` Shreeya Patel @ 2023-04-28 7:41 ` Thorsten Leemhuis 2023-05-02 9:48 ` Shreeya Patel 2023-04-28 17:27 ` Nick Desaulniers 1 sibling, 1 reply; 53+ messages in thread From: Thorsten Leemhuis @ 2023-04-28 7:41 UTC (permalink / raw) To: Shreeya Patel, Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions Hi Shreeya! On 27.04.23 13:53, Shreeya Patel wrote: > On 19/09/22 22:38, Nick Desaulniers wrote: >> cc-ifversion is GCC specific. Replace it with compiler specific >> variants. Update the users of cc-ifversion to use these new macros. >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/350 >> Link: >> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ >> Suggested-by: Bill Wendling <morbo@google.com> >> Reviewed-by: Nathan Chancellor <nathan@kernel.org> >> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > KernelCI found this patch causes a regression in the > baseline.logintest on qemu_arm-virt-gicv3-uefi [1], > see the bisection report for more details [2]. > > Let me know if you have any questions. > > [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ > [2] https://groups.io/g/kernelci-results/message/40804> [...] > #regzbot introduced: 88b61e3bff93 How much of this text is auto generated? I ask for two reasons: * You made regzbot track this, which is great, but didn't specify a title. That is fine in general, if the subject round about says what the regression is about. But in this case it doesn't; hence it would be great if you in the future could specify one through "#regzbot title:" or adjust the mail's subject (I guess the former is what developers will prefer). * I'm not a developer that has to look into bugs like this, but from my experience I expect a lot more developers are likely willing to look into reports like this if you specified what the actual problem is -- ideally with the relevant error messages. Side note: I for one still am unsure what this is actually about after looking at the page you provided as [1] and clicking on a few links there (which took me a few minutes, which I guess not everyone is willing to invest). I noticed two kernel warning in the log (one from "arch/arm/kernel/insn.c:48 __arm_gen_branch+0x88/0xa4", the other "kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4") and a complaint UBSAN (btw: those newlines in the logs make things harder to read, at least for me). To check if those were old or new problems, I tired to go back in the history and on page 9 found an entry for the last succeeding test. But clicking on the logs got me a 404 :-/ Then I looked at the logs on [1] again and in the html view "Boot result: FAIL". Is that the actual problem? Ciao, Thorsten P.S.: let me update the regzbot title while at it: #regzbot title: kernelci: qemu_arm-virt-gicv3-uefi stopped booting >> --- >> Changes v3 -> v4: >> * Split into its own patch again from series, as per Masahiro. >> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update >> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. >> >> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- >> Makefile | 6 ++--- >> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- >> scripts/Makefile.compiler | 10 ++++--- >> scripts/Makefile.extrawarn | 4 +-- >> 5 files changed, 29 insertions(+), 22 deletions(-) >> >> diff --git a/Documentation/kbuild/makefiles.rst >> b/Documentation/kbuild/makefiles.rst >> index 11a296e52d68..ee7e3ea1fbe1 100644 >> --- a/Documentation/kbuild/makefiles.rst >> +++ b/Documentation/kbuild/makefiles.rst >> @@ -682,22 +682,27 @@ more details, with real examples. >> In the above example, -Wno-unused-but-set-variable will be added to >> KBUILD_CFLAGS only if gcc really accepts it. >> - cc-ifversion >> - cc-ifversion tests the version of $(CC) and equals the fourth >> parameter >> - if version expression is true, or the fifth (if given) if the >> version >> - expression is false. >> + gcc-min-version >> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is >> greater than >> + or equal to the provided value and evaluates to y if so. >> Example:: >> - #fs/reiserfs/Makefile >> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) >> + cflags-$(call gcc-min-version, 70100) := -foo >> - In this example, ccflags-y will be assigned the value -O1 if the >> - $(CC) version is less than 4.2. >> - cc-ifversion takes all the shell operators: >> - -eq, -ne, -lt, -le, -gt, and -ge >> - The third parameter may be a text as in this example, but it may >> also >> - be an expanded variable or a macro. >> + In this example, cflags-y will be assigned the value -foo if >> $(CC) is gcc and >> + $(CONFIG_GCC_VERSION) is >= 7.1. >> + >> + clang-min-version >> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) >> is greater >> + than or equal to the provided value and evaluates to y if so. >> + >> + Example:: >> + >> + cflags-$(call clang-min-version, 110000) := -foo >> + >> + In this example, cflags-y will be assigned the value -foo if >> $(CC) is clang >> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. >> cc-cross-prefix >> cc-cross-prefix is used to check if there exists a $(CC) in path >> with >> diff --git a/Makefile b/Makefile >> index 298f69060f10..411c8480b37e 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) >> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror >> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds >> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >> ifdef CONFIG_CC_IS_CLANG >> KBUILD_CPPFLAGS += -Qunused-arguments >> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC >> KBUILD_CFLAGS += -Wno-maybe-uninitialized >> endif >> -ifdef CONFIG_CC_IS_GCC >> # The allocators already balk at large sizes, so silence the compiler >> # warnings for bounds checks involving those possible values. While >> # -Wno-alloc-size-larger-than would normally be used here, earlier >> versions >> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC >> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other >> # choice, we must perform a versioned check to disable this warning. >> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au >> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, >> -Wno-alloc-size-larger-than) >> -endif >> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += >> -Wno-alloc-size-larger-than >> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >> # disable invalid "can't wrap" optimizations for signed / pointers >> KBUILD_CFLAGS += -fno-strict-overflow >> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile >> b/drivers/gpu/drm/amd/display/dc/dml/Makefile >> index cb81ed2fbd53..d70838edba80 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile >> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile >> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec >> endif >> ifdef CONFIG_CC_IS_GCC >> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) >> +ifneq ($(call gcc-min-version, 70100),y) >> IS_OLD_GCC = 1 >> endif >> endif >> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler >> index 94d0d40cddb3..9d18fb91890e 100644 >> --- a/scripts/Makefile.compiler >> +++ b/scripts/Makefile.compiler >> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ >> cc-disable-warning = $(call try-run,\ >> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip >> $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) >> -# cc-ifversion >> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) >> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && >> echo $(3) || echo $(4)) >> +# gcc-min-version >> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo >> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) >> + >> +# clang-min-version >> +# Usage: cflags-$(call clang-min-version, 110000) += -foo >> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && >> echo y) >> # ld-option >> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) >> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn >> index 6ae482158bc4..5769c1939d40 100644 >> --- a/scripts/Makefile.extrawarn >> +++ b/scripts/Makefile.extrawarn >> @@ -48,7 +48,7 @@ else >> ifdef CONFIG_CC_IS_CLANG >> KBUILD_CFLAGS += -Wno-initializer-overrides >> # Clang before clang-16 would warn on default argument promotions. >> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) >> +ifneq ($(call clang-min-version, 160000),y) >> # Disable -Wformat >> KBUILD_CFLAGS += -Wno-format >> # Then re-enable flags that were part of the -Wformat group that aren't >> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format >> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier >> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull >> # Requires clang-12+. >> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) >> +ifeq ($(call clang-min-version, 120000),y) >> KBUILD_CFLAGS += -Wformat-insufficient-args >> endif >> endif > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-04-28 7:41 ` Thorsten Leemhuis @ 2023-05-02 9:48 ` Shreeya Patel 2023-05-02 11:58 ` Linux regression tracking (Thorsten Leemhuis) 0 siblings, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-05-02 9:48 UTC (permalink / raw) To: Thorsten Leemhuis, Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, kernelci, gustavo.padovan, Guillaume Charles Tucker, ricardo.canuelo, denys.f Hi Thorsten, On 28/04/23 13:11, Thorsten Leemhuis wrote: > Hi Shreeya! > > On 27.04.23 13:53, Shreeya Patel wrote: >> On 19/09/22 22:38, Nick Desaulniers wrote: >>> cc-ifversion is GCC specific. Replace it with compiler specific >>> variants. Update the users of cc-ifversion to use these new macros. >>> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 >>> Link: >>> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ >>> Suggested-by: Bill Wendling <morbo@google.com> >>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >> KernelCI found this patch causes a regression in the >> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], >> see the bisection report for more details [2]. >> >> Let me know if you have any questions. >> >> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ >> [2] https://groups.io/g/kernelci-results/message/40804> [...] >> #regzbot introduced: 88b61e3bff93 > How much of this text is auto generated? I ask for two reasons: None of this text is auto generated yet but we plan to do it soon once we think the format of the reporting email is good enough for people to understand and look into it. Which is why your comments are really helpful here. > > * You made regzbot track this, which is great, but didn't specify a > title. That is fine in general, if the subject round about says what the > regression is about. But in this case it doesn't; hence it would be > great if you in the future could specify one through "#regzbot title:" > or adjust the mail's subject (I guess the former is what developers will > prefer). Noted. If I think the title is not very explanatory then I'll change it to reflect the problem in future. > * I'm not a developer that has to look into bugs like this, but from my > experience I expect a lot more developers are likely willing to look > into reports like this if you specified what the actual problem is -- > ideally with the relevant error messages. > > Side note: I for one still am unsure what this is actually about after > looking at the page you provided as [1] and clicking on a few links > there (which took me a few minutes, which I guess not everyone is > willing to invest). I noticed two kernel warning in the log (one from > "arch/arm/kernel/insn.c:48 __arm_gen_branch+0x88/0xa4", the other > "kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4") and a complaint > UBSAN (btw: those newlines in the logs make things harder to read, at > least for me). For this particular regression, it is possible that the following kernel warnings/errors could be the reason of failure. 572 20:30:46.811318 <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in ../arch/arm/mach-sunxi/mc_smp.c:811:29 574 20:30:46.812203 <3>[ 0.417963][ T1] index 2 is out of range for type 'sunxi_mc_smp_data [2]' and 426 20:30:44.726421 <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 428 20:30:44.727284 <4>[ 0.000000][ T0] Modules linked in: 430 20:30:44.728487 <4>[ 0.000000][ T0] CPU: 0 PID: 0 Comm: swapper Tainted: G W 6.3.0 #1 ......... I understand that it might be more helpful to atleast put little more information about what is causing the regression here. I'll provide some more details in future for it to be easy for developers to look into it. > To check if those were old or new problems, I tired to go back in the > history and on page 9 found an entry for the last succeeding test. But > clicking on the logs got me a 404 :-/ > > Then I looked at the logs on [1] again and in the html view "Boot > result: FAIL". Is that the actual problem? Unfortunately, we do have some broken links in the current KernelCI dashboard but the KernelCI team is working on a new API and database interface which will fix these issues. It might not be worth to spend time to fix issues on the current dashboard since the process for getting the archived logs is not very straightforward. What I can do from my side is to attach logs of the working kernel if I can get them through LAVA. But one thing to note is that even LAVA stores limited logs and we won't be able to provide them always like in this case since the regression has been happening from a long time. Thanks for your input though, we will work on it and get a better format ready for reporting the regressions. Thanks, Shreeya Patel > > Ciao, Thorsten > > P.S.: let me update the regzbot title while at it: > > #regzbot title: kernelci: qemu_arm-virt-gicv3-uefi stopped booting > >>> --- >>> Changes v3 -> v4: >>> * Split into its own patch again from series, as per Masahiro. >>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update >>> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. >>> >>> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- >>> Makefile | 6 ++--- >>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- >>> scripts/Makefile.compiler | 10 ++++--- >>> scripts/Makefile.extrawarn | 4 +-- >>> 5 files changed, 29 insertions(+), 22 deletions(-) >>> >>> diff --git a/Documentation/kbuild/makefiles.rst >>> b/Documentation/kbuild/makefiles.rst >>> index 11a296e52d68..ee7e3ea1fbe1 100644 >>> --- a/Documentation/kbuild/makefiles.rst >>> +++ b/Documentation/kbuild/makefiles.rst >>> @@ -682,22 +682,27 @@ more details, with real examples. >>> In the above example, -Wno-unused-but-set-variable will be added to >>> KBUILD_CFLAGS only if gcc really accepts it. >>> - cc-ifversion >>> - cc-ifversion tests the version of $(CC) and equals the fourth >>> parameter >>> - if version expression is true, or the fifth (if given) if the >>> version >>> - expression is false. >>> + gcc-min-version >>> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is >>> greater than >>> + or equal to the provided value and evaluates to y if so. >>> Example:: >>> - #fs/reiserfs/Makefile >>> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) >>> + cflags-$(call gcc-min-version, 70100) := -foo >>> - In this example, ccflags-y will be assigned the value -O1 if the >>> - $(CC) version is less than 4.2. >>> - cc-ifversion takes all the shell operators: >>> - -eq, -ne, -lt, -le, -gt, and -ge >>> - The third parameter may be a text as in this example, but it may >>> also >>> - be an expanded variable or a macro. >>> + In this example, cflags-y will be assigned the value -foo if >>> $(CC) is gcc and >>> + $(CONFIG_GCC_VERSION) is >= 7.1. >>> + >>> + clang-min-version >>> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) >>> is greater >>> + than or equal to the provided value and evaluates to y if so. >>> + >>> + Example:: >>> + >>> + cflags-$(call clang-min-version, 110000) := -foo >>> + >>> + In this example, cflags-y will be assigned the value -foo if >>> $(CC) is clang >>> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. >>> cc-cross-prefix >>> cc-cross-prefix is used to check if there exists a $(CC) in path >>> with >>> diff --git a/Makefile b/Makefile >>> index 298f69060f10..411c8480b37e 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) >>> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror >>> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds >>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>> ifdef CONFIG_CC_IS_CLANG >>> KBUILD_CPPFLAGS += -Qunused-arguments >>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC >>> KBUILD_CFLAGS += -Wno-maybe-uninitialized >>> endif >>> -ifdef CONFIG_CC_IS_GCC >>> # The allocators already balk at large sizes, so silence the compiler >>> # warnings for bounds checks involving those possible values. While >>> # -Wno-alloc-size-larger-than would normally be used here, earlier >>> versions >>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC >>> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other >>> # choice, we must perform a versioned check to disable this warning. >>> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au >>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, >>> -Wno-alloc-size-larger-than) >>> -endif >>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += >>> -Wno-alloc-size-larger-than >>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>> # disable invalid "can't wrap" optimizations for signed / pointers >>> KBUILD_CFLAGS += -fno-strict-overflow >>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> index cb81ed2fbd53..d70838edba80 100644 >>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec >>> endif >>> ifdef CONFIG_CC_IS_GCC >>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) >>> +ifneq ($(call gcc-min-version, 70100),y) >>> IS_OLD_GCC = 1 >>> endif >>> endif >>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler >>> index 94d0d40cddb3..9d18fb91890e 100644 >>> --- a/scripts/Makefile.compiler >>> +++ b/scripts/Makefile.compiler >>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ >>> cc-disable-warning = $(call try-run,\ >>> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip >>> $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) >>> -# cc-ifversion >>> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) >>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && >>> echo $(3) || echo $(4)) >>> +# gcc-min-version >>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo >>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) >>> + >>> +# clang-min-version >>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo >>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && >>> echo y) >>> # ld-option >>> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) >>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn >>> index 6ae482158bc4..5769c1939d40 100644 >>> --- a/scripts/Makefile.extrawarn >>> +++ b/scripts/Makefile.extrawarn >>> @@ -48,7 +48,7 @@ else >>> ifdef CONFIG_CC_IS_CLANG >>> KBUILD_CFLAGS += -Wno-initializer-overrides >>> # Clang before clang-16 would warn on default argument promotions. >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) >>> +ifneq ($(call clang-min-version, 160000),y) >>> # Disable -Wformat >>> KBUILD_CFLAGS += -Wno-format >>> # Then re-enable flags that were part of the -Wformat group that aren't >>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format >>> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier >>> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull >>> # Requires clang-12+. >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) >>> +ifeq ($(call clang-min-version, 120000),y) >>> KBUILD_CFLAGS += -Wformat-insufficient-args >>> endif >>> endif >> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-02 9:48 ` Shreeya Patel @ 2023-05-02 11:58 ` Linux regression tracking (Thorsten Leemhuis) 0 siblings, 0 replies; 53+ messages in thread From: Linux regression tracking (Thorsten Leemhuis) @ 2023-05-02 11:58 UTC (permalink / raw) To: Shreeya Patel, Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, kernelci, gustavo.padovan, Guillaume Charles Tucker, ricardo.canuelo, denys.f On 02.05.23 11:48, Shreeya Patel wrote: > On 28/04/23 13:11, Thorsten Leemhuis wrote: >> On 27.04.23 13:53, Shreeya Patel wrote: >>> On 19/09/22 22:38, Nick Desaulniers wrote: >>>> cc-ifversion is GCC specific. Replace it with compiler specific >>>> variants. Update the users of cc-ifversion to use these new macros. >>>> >>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 >>>> Link: >>>> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ >>>> Suggested-by: Bill Wendling <morbo@google.com> >>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> >>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >>> KernelCI found this patch causes a regression in the >>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], >>> see the bisection report for more details [2]. >>> >>> Let me know if you have any questions. >>> >>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ >>> [2] https://groups.io/g/kernelci-results/message/40804> [...] >>> #regzbot introduced: 88b61e3bff93 >> How much of this text is auto generated? I ask for two reasons: > > None of this text is auto generated yet but we plan to do it soon once > we think the format of the reporting email is good enough for people to > understand > and look into it. Which is why your comments are really helpful here. Thx, glad to hear that. FWIW and YMMV, but I'm not sure if fully automating things is a good idea, as a bad report or two might be enough to make some developers start ignoring kernelci reports; a quick human review with small adjustments might help prevent that, as it's hard to get that ship turned around later (that's why regzbot up to this day doesn't send any mails automatically). >> * You made regzbot track this, which is great, but didn't specify a >> title. That is fine in general, if the subject round about says what the >> regression is about. But in this case it doesn't; hence it would be >> great if you in the future could specify one through "#regzbot title:" >> or adjust the mail's subject (I guess the former is what developers will >> prefer). > > Noted. If I think the title is not very explanatory then I'll change it > to reflect the problem in future. Many thx! > [...] > > I understand that it might be more helpful to atleast put little more > information about what is causing the regression here. > I'll provide some more details in future for it to be easy for > developers to look into it. Yeah, especially a obvious "what's the actual problem (in 10 words or less)" afaics would have been really good here. >> To check if those were old or new problems, I tired to go back in the >> history and on page 9 found an entry for the last succeeding test. But >> clicking on the logs got me a 404 :-/ >> >> Then I looked at the logs on [1] again and in the html view "Boot >> result: FAIL". Is that the actual problem? > > Unfortunately, we do have some broken links in the current KernelCI > dashboard [...] Happens :-D > What I can do from my side is to attach logs of the working kernel if I > can get them through LAVA. > But one thing to note is that even LAVA stores limited logs and we won't > be able to provide them always like in this case > since the regression has been happening from a long time. Before going down that route I'd work out with Nick why things work for him, maybe all that isn't needed. > Thanks for your input though, we will work on it and get a better format > ready for reporting the regressions. Thx for your work! Ciao, Thorsten ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-04-27 11:53 ` Shreeya Patel 2023-04-28 7:41 ` Thorsten Leemhuis @ 2023-04-28 17:27 ` Nick Desaulniers 2023-05-03 21:02 ` Shreeya Patel 1 sibling, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2023-04-28 17:27 UTC (permalink / raw) To: Shreeya Patel Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > Hi Nick, > > On 19/09/22 22:38, Nick Desaulniers wrote: > > cc-ifversion is GCC specific. Replace it with compiler specific > > variants. Update the users of cc-ifversion to use these new macros. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/350 > > Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > > Suggested-by: Bill Wendling <morbo@google.com> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > KernelCI found this patch causes a regression in the > baseline.logintest on qemu_arm-virt-gicv3-uefi [1], > see the bisection report for more details [2]. > > Let me know if you have any questions. > > > [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ Hi Shreeya, Thanks for the report. When I click the above link, then click `multi_v7_defconfig+debug` to get the config necessary to reproduce, I get an HTTP 404. https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config Same for zImage https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage If I click on the log https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt It looks like the machine powered up, then powered off. Is the test actually failing? I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc (Debian 10.2.1-6) in QEMU just fine. So I'm going to need some more information to help reproduce what specifically is failing. Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023 --- It does look like UBSAN is flagging an array OOB: <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in ../arch/arm/mach-sunxi/mc_smp.c:811:29 And potentially another issue with ftrace <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 > [2] https://groups.io/g/kernelci-results/message/40804 > > > Thanks, > Shreeya Patel > > #regzbot introduced: 88b61e3bff93 > > > --- > > Changes v3 -> v4: > > * Split into its own patch again from series, as per Masahiro. > > * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > > clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. > > > > Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- > > Makefile | 6 ++--- > > drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- > > scripts/Makefile.compiler | 10 ++++--- > > scripts/Makefile.extrawarn | 4 +-- > > 5 files changed, 29 insertions(+), 22 deletions(-) > > > > diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst > > index 11a296e52d68..ee7e3ea1fbe1 100644 > > --- a/Documentation/kbuild/makefiles.rst > > +++ b/Documentation/kbuild/makefiles.rst > > @@ -682,22 +682,27 @@ more details, with real examples. > > In the above example, -Wno-unused-but-set-variable will be added to > > KBUILD_CFLAGS only if gcc really accepts it. > > > > - cc-ifversion > > - cc-ifversion tests the version of $(CC) and equals the fourth parameter > > - if version expression is true, or the fifth (if given) if the version > > - expression is false. > > + gcc-min-version > > + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than > > + or equal to the provided value and evaluates to y if so. > > > > Example:: > > > > - #fs/reiserfs/Makefile > > - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) > > + cflags-$(call gcc-min-version, 70100) := -foo > > > > - In this example, ccflags-y will be assigned the value -O1 if the > > - $(CC) version is less than 4.2. > > - cc-ifversion takes all the shell operators: > > - -eq, -ne, -lt, -le, -gt, and -ge > > - The third parameter may be a text as in this example, but it may also > > - be an expanded variable or a macro. > > + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and > > + $(CONFIG_GCC_VERSION) is >= 7.1. > > + > > + clang-min-version > > + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater > > + than or equal to the provided value and evaluates to y if so. > > + > > + Example:: > > + > > + cflags-$(call clang-min-version, 110000) := -foo > > + > > + In this example, cflags-y will be assigned the value -foo if $(CC) is clang > > + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. > > > > cc-cross-prefix > > cc-cross-prefix is used to check if there exists a $(CC) in path with > > diff --git a/Makefile b/Makefile > > index 298f69060f10..411c8480b37e 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) > > > > KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror > > KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds > > -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > > > > ifdef CONFIG_CC_IS_CLANG > > KBUILD_CPPFLAGS += -Qunused-arguments > > @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC > > KBUILD_CFLAGS += -Wno-maybe-uninitialized > > endif > > > > -ifdef CONFIG_CC_IS_GCC > > # The allocators already balk at large sizes, so silence the compiler > > # warnings for bounds checks involving those possible values. While > > # -Wno-alloc-size-larger-than would normally be used here, earlier versions > > @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC > > # ignored, continuing to default to PTRDIFF_MAX. So, left with no other > > # choice, we must perform a versioned check to disable this warning. > > # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au > > -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) > > -endif > > +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than > > +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > > > > # disable invalid "can't wrap" optimizations for signed / pointers > > KBUILD_CFLAGS += -fno-strict-overflow > > diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile > > index cb81ed2fbd53..d70838edba80 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile > > +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile > > @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec > > endif > > > > ifdef CONFIG_CC_IS_GCC > > -ifeq ($(call cc-ifversion, -lt, 0701, y), y) > > +ifneq ($(call gcc-min-version, 70100),y) > > IS_OLD_GCC = 1 > > endif > > endif > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > > index 94d0d40cddb3..9d18fb91890e 100644 > > --- a/scripts/Makefile.compiler > > +++ b/scripts/Makefile.compiler > > @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ > > cc-disable-warning = $(call try-run,\ > > $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) > > > > -# cc-ifversion > > -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) > > -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) > > +# gcc-min-version > > +# Usage: cflags-$(call gcc-min-version, 70100) += -foo > > +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) > > + > > +# clang-min-version > > +# Usage: cflags-$(call clang-min-version, 110000) += -foo > > +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) > > > > # ld-option > > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > > index 6ae482158bc4..5769c1939d40 100644 > > --- a/scripts/Makefile.extrawarn > > +++ b/scripts/Makefile.extrawarn > > @@ -48,7 +48,7 @@ else > > ifdef CONFIG_CC_IS_CLANG > > KBUILD_CFLAGS += -Wno-initializer-overrides > > # Clang before clang-16 would warn on default argument promotions. > > -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) > > +ifneq ($(call clang-min-version, 160000),y) > > # Disable -Wformat > > KBUILD_CFLAGS += -Wno-format > > # Then re-enable flags that were part of the -Wformat group that aren't > > @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format > > KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier > > KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull > > # Requires clang-12+. > > -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) > > +ifeq ($(call clang-min-version, 120000),y) > > KBUILD_CFLAGS += -Wformat-insufficient-args > > endif > > endif -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-04-28 17:27 ` Nick Desaulniers @ 2023-05-03 21:02 ` Shreeya Patel 2023-05-03 21:15 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-05-03 21:02 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, ricardo.canuelo, kernelci Hi Nick, On 28/04/23 22:57, Nick Desaulniers wrote: > On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> Hi Nick, >> >> On 19/09/22 22:38, Nick Desaulniers wrote: >>> cc-ifversion is GCC specific. Replace it with compiler specific >>> variants. Update the users of cc-ifversion to use these new macros. >>> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 >>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ >>> Suggested-by: Bill Wendling <morbo@google.com> >>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >> KernelCI found this patch causes a regression in the >> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], >> see the bisection report for more details [2]. >> >> Let me know if you have any questions. >> >> >> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ > Hi Shreeya, > Thanks for the report. > > When I click the above link, then click `multi_v7_defconfig+debug` to > get the config necessary to reproduce, I get an HTTP 404. > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config > > Same for zImage > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage Apologies for the broken links. We will try to fix the important ones if we can but in the meantime, following is the correct link that you can refer. config :- https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config zImage :- https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage If you notice, they are present under the kernel directory and same way you can find links for other kernel builds if you'd like to check them out. > If I click on the log > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt > It looks like the machine powered up, then powered off. Is the test > actually failing? I recommend checking the html logs from the kernelci dashboard. Also, FYI baseline.login test failure means that the device failed to boot which I think is causing by the issues that you pointed out. <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in ../arch/arm/mach-sunxi/mc_smp.c:811:29 And potentially another issue with ftrace <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 Let me know if you need more information from my side to reproduce this on your end. Thanks, Shreeya Patel > I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc > (Debian 10.2.1-6) in QEMU just fine. So I'm going to need some more > information to help reproduce what specifically is failing. > > Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc > (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) > 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023 > > --- > > It does look like UBSAN is flagging an array OOB: > > <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in > ../arch/arm/mach-sunxi/mc_smp.c:811:29 > > And potentially another issue with ftrace > > <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at > kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 > > > >> [2] https://groups.io/g/kernelci-results/message/40804 >> >> >> Thanks, >> Shreeya Patel >> >> #regzbot introduced: 88b61e3bff93 >> >>> --- >>> Changes v3 -> v4: >>> * Split into its own patch again from series, as per Masahiro. >>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update >>> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. >>> >>> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- >>> Makefile | 6 ++--- >>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- >>> scripts/Makefile.compiler | 10 ++++--- >>> scripts/Makefile.extrawarn | 4 +-- >>> 5 files changed, 29 insertions(+), 22 deletions(-) >>> >>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst >>> index 11a296e52d68..ee7e3ea1fbe1 100644 >>> --- a/Documentation/kbuild/makefiles.rst >>> +++ b/Documentation/kbuild/makefiles.rst >>> @@ -682,22 +682,27 @@ more details, with real examples. >>> In the above example, -Wno-unused-but-set-variable will be added to >>> KBUILD_CFLAGS only if gcc really accepts it. >>> >>> - cc-ifversion >>> - cc-ifversion tests the version of $(CC) and equals the fourth parameter >>> - if version expression is true, or the fifth (if given) if the version >>> - expression is false. >>> + gcc-min-version >>> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than >>> + or equal to the provided value and evaluates to y if so. >>> >>> Example:: >>> >>> - #fs/reiserfs/Makefile >>> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) >>> + cflags-$(call gcc-min-version, 70100) := -foo >>> >>> - In this example, ccflags-y will be assigned the value -O1 if the >>> - $(CC) version is less than 4.2. >>> - cc-ifversion takes all the shell operators: >>> - -eq, -ne, -lt, -le, -gt, and -ge >>> - The third parameter may be a text as in this example, but it may also >>> - be an expanded variable or a macro. >>> + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and >>> + $(CONFIG_GCC_VERSION) is >= 7.1. >>> + >>> + clang-min-version >>> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater >>> + than or equal to the provided value and evaluates to y if so. >>> + >>> + Example:: >>> + >>> + cflags-$(call clang-min-version, 110000) := -foo >>> + >>> + In this example, cflags-y will be assigned the value -foo if $(CC) is clang >>> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. >>> >>> cc-cross-prefix >>> cc-cross-prefix is used to check if there exists a $(CC) in path with >>> diff --git a/Makefile b/Makefile >>> index 298f69060f10..411c8480b37e 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) >>> >>> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror >>> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds >>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>> >>> ifdef CONFIG_CC_IS_CLANG >>> KBUILD_CPPFLAGS += -Qunused-arguments >>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC >>> KBUILD_CFLAGS += -Wno-maybe-uninitialized >>> endif >>> >>> -ifdef CONFIG_CC_IS_GCC >>> # The allocators already balk at large sizes, so silence the compiler >>> # warnings for bounds checks involving those possible values. While >>> # -Wno-alloc-size-larger-than would normally be used here, earlier versions >>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC >>> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other >>> # choice, we must perform a versioned check to disable this warning. >>> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au >>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) >>> -endif >>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than >>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>> >>> # disable invalid "can't wrap" optimizations for signed / pointers >>> KBUILD_CFLAGS += -fno-strict-overflow >>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> index cb81ed2fbd53..d70838edba80 100644 >>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec >>> endif >>> >>> ifdef CONFIG_CC_IS_GCC >>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) >>> +ifneq ($(call gcc-min-version, 70100),y) >>> IS_OLD_GCC = 1 >>> endif >>> endif >>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler >>> index 94d0d40cddb3..9d18fb91890e 100644 >>> --- a/scripts/Makefile.compiler >>> +++ b/scripts/Makefile.compiler >>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ >>> cc-disable-warning = $(call try-run,\ >>> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) >>> >>> -# cc-ifversion >>> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) >>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) >>> +# gcc-min-version >>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo >>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) >>> + >>> +# clang-min-version >>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo >>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) >>> >>> # ld-option >>> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) >>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn >>> index 6ae482158bc4..5769c1939d40 100644 >>> --- a/scripts/Makefile.extrawarn >>> +++ b/scripts/Makefile.extrawarn >>> @@ -48,7 +48,7 @@ else >>> ifdef CONFIG_CC_IS_CLANG >>> KBUILD_CFLAGS += -Wno-initializer-overrides >>> # Clang before clang-16 would warn on default argument promotions. >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) >>> +ifneq ($(call clang-min-version, 160000),y) >>> # Disable -Wformat >>> KBUILD_CFLAGS += -Wno-format >>> # Then re-enable flags that were part of the -Wformat group that aren't >>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format >>> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier >>> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull >>> # Requires clang-12+. >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) >>> +ifeq ($(call clang-min-version, 120000),y) >>> KBUILD_CFLAGS += -Wformat-insufficient-args >>> endif >>> endif > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-03 21:02 ` Shreeya Patel @ 2023-05-03 21:15 ` Nick Desaulniers 2023-05-03 22:33 ` Shreeya Patel 0 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2023-05-03 21:15 UTC (permalink / raw) To: Shreeya Patel Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, ricardo.canuelo, kernelci On Wed, May 3, 2023 at 2:02 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > Hi Nick, > > On 28/04/23 22:57, Nick Desaulniers wrote: > > On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel > > <shreeya.patel@collabora.com> wrote: > >> Hi Nick, > >> > >> On 19/09/22 22:38, Nick Desaulniers wrote: > >>> cc-ifversion is GCC specific. Replace it with compiler specific > >>> variants. Update the users of cc-ifversion to use these new macros. > >>> > >>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 > >>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > >>> Suggested-by: Bill Wendling <morbo@google.com> > >>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > >> KernelCI found this patch causes a regression in the > >> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], > >> see the bisection report for more details [2]. > >> > >> Let me know if you have any questions. > >> > >> > >> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ > > Hi Shreeya, > > Thanks for the report. > > > > When I click the above link, then click `multi_v7_defconfig+debug` to > > get the config necessary to reproduce, I get an HTTP 404. > > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config > > > > Same for zImage > > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage > > Apologies for the broken links. We will try to fix the important ones if > we can but in the meantime, > following is the correct link that you can refer. > > config :- > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config > > zImage :- > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage > > If you notice, they are present under the kernel directory and same way > you can find links for other kernel > builds if you'd like to check them out. > > > If I click on the log > > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt > > It looks like the machine powered up, then powered off. Is the test > > actually failing? > > I recommend checking the html logs from the kernelci dashboard. > Also, FYI baseline.login test failure means that the device failed to > boot which I think is causing by the issues that you pointed out. > > <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in > ../arch/arm/mach-sunxi/mc_smp.c:811:29 > > And potentially another issue with ftrace > > <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at > kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 > > > Let me know if you need more information from my side to reproduce this > on your end. > > > Thanks, > Shreeya Patel Hi Shreeya, I may need your help to reproduce the failure. $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config -O .config $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s <launch qemu> / # mount -t proc /proc / # cat /proc/version Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2) #2 SMP Wed May 3 21:10:27 UTC 2023 I was able to boot the resulting kernel to a command line. Perhaps there's something about the userspace image that tickles this? Can you supply the rootfs that's used in testing? > > I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc > > (Debian 10.2.1-6) in QEMU just fine. So I'm going to need some more > > information to help reproduce what specifically is failing. > > > > Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc > > (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) > > 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023 > > > > --- > > > > It does look like UBSAN is flagging an array OOB: > > > > <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in > > ../arch/arm/mach-sunxi/mc_smp.c:811:29 > > > > And potentially another issue with ftrace > > > > <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at > > kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 > > > > > > > >> [2] https://groups.io/g/kernelci-results/message/40804 > >> > >> > >> Thanks, > >> Shreeya Patel > >> > >> #regzbot introduced: 88b61e3bff93 > >> > >>> --- > >>> Changes v3 -> v4: > >>> * Split into its own patch again from series, as per Masahiro. > >>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > >>> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. > >>> > >>> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- > >>> Makefile | 6 ++--- > >>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- > >>> scripts/Makefile.compiler | 10 ++++--- > >>> scripts/Makefile.extrawarn | 4 +-- > >>> 5 files changed, 29 insertions(+), 22 deletions(-) > >>> > >>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst > >>> index 11a296e52d68..ee7e3ea1fbe1 100644 > >>> --- a/Documentation/kbuild/makefiles.rst > >>> +++ b/Documentation/kbuild/makefiles.rst > >>> @@ -682,22 +682,27 @@ more details, with real examples. > >>> In the above example, -Wno-unused-but-set-variable will be added to > >>> KBUILD_CFLAGS only if gcc really accepts it. > >>> > >>> - cc-ifversion > >>> - cc-ifversion tests the version of $(CC) and equals the fourth parameter > >>> - if version expression is true, or the fifth (if given) if the version > >>> - expression is false. > >>> + gcc-min-version > >>> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than > >>> + or equal to the provided value and evaluates to y if so. > >>> > >>> Example:: > >>> > >>> - #fs/reiserfs/Makefile > >>> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) > >>> + cflags-$(call gcc-min-version, 70100) := -foo > >>> > >>> - In this example, ccflags-y will be assigned the value -O1 if the > >>> - $(CC) version is less than 4.2. > >>> - cc-ifversion takes all the shell operators: > >>> - -eq, -ne, -lt, -le, -gt, and -ge > >>> - The third parameter may be a text as in this example, but it may also > >>> - be an expanded variable or a macro. > >>> + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and > >>> + $(CONFIG_GCC_VERSION) is >= 7.1. > >>> + > >>> + clang-min-version > >>> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater > >>> + than or equal to the provided value and evaluates to y if so. > >>> + > >>> + Example:: > >>> + > >>> + cflags-$(call clang-min-version, 110000) := -foo > >>> + > >>> + In this example, cflags-y will be assigned the value -foo if $(CC) is clang > >>> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. > >>> > >>> cc-cross-prefix > >>> cc-cross-prefix is used to check if there exists a $(CC) in path with > >>> diff --git a/Makefile b/Makefile > >>> index 298f69060f10..411c8480b37e 100644 > >>> --- a/Makefile > >>> +++ b/Makefile > >>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) > >>> > >>> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror > >>> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds > >>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > >>> > >>> ifdef CONFIG_CC_IS_CLANG > >>> KBUILD_CPPFLAGS += -Qunused-arguments > >>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC > >>> KBUILD_CFLAGS += -Wno-maybe-uninitialized > >>> endif > >>> > >>> -ifdef CONFIG_CC_IS_GCC > >>> # The allocators already balk at large sizes, so silence the compiler > >>> # warnings for bounds checks involving those possible values. While > >>> # -Wno-alloc-size-larger-than would normally be used here, earlier versions > >>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC > >>> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other > >>> # choice, we must perform a versioned check to disable this warning. > >>> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au > >>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) > >>> -endif > >>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than > >>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > >>> > >>> # disable invalid "can't wrap" optimizations for signed / pointers > >>> KBUILD_CFLAGS += -fno-strict-overflow > >>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile > >>> index cb81ed2fbd53..d70838edba80 100644 > >>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile > >>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile > >>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec > >>> endif > >>> > >>> ifdef CONFIG_CC_IS_GCC > >>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) > >>> +ifneq ($(call gcc-min-version, 70100),y) > >>> IS_OLD_GCC = 1 > >>> endif > >>> endif > >>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > >>> index 94d0d40cddb3..9d18fb91890e 100644 > >>> --- a/scripts/Makefile.compiler > >>> +++ b/scripts/Makefile.compiler > >>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ > >>> cc-disable-warning = $(call try-run,\ > >>> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) > >>> > >>> -# cc-ifversion > >>> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) > >>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) > >>> +# gcc-min-version > >>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo > >>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) > >>> + > >>> +# clang-min-version > >>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo > >>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) > >>> > >>> # ld-option > >>> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) > >>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > >>> index 6ae482158bc4..5769c1939d40 100644 > >>> --- a/scripts/Makefile.extrawarn > >>> +++ b/scripts/Makefile.extrawarn > >>> @@ -48,7 +48,7 @@ else > >>> ifdef CONFIG_CC_IS_CLANG > >>> KBUILD_CFLAGS += -Wno-initializer-overrides > >>> # Clang before clang-16 would warn on default argument promotions. > >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) > >>> +ifneq ($(call clang-min-version, 160000),y) > >>> # Disable -Wformat > >>> KBUILD_CFLAGS += -Wno-format > >>> # Then re-enable flags that were part of the -Wformat group that aren't > >>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format > >>> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier > >>> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull > >>> # Requires clang-12+. > >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) > >>> +ifeq ($(call clang-min-version, 120000),y) > >>> KBUILD_CFLAGS += -Wformat-insufficient-args > >>> endif > >>> endif > > > > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-03 21:15 ` Nick Desaulniers @ 2023-05-03 22:33 ` Shreeya Patel 2023-05-15 23:01 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-05-03 22:33 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, ricardo.canuelo, kernelci On 04/05/23 02:45, Nick Desaulniers wrote: > On Wed, May 3, 2023 at 2:02 PM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> Hi Nick, >> >> On 28/04/23 22:57, Nick Desaulniers wrote: >>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel >>> <shreeya.patel@collabora.com> wrote: >>>> Hi Nick, >>>> >>>> On 19/09/22 22:38, Nick Desaulniers wrote: >>>>> cc-ifversion is GCC specific. Replace it with compiler specific >>>>> variants. Update the users of cc-ifversion to use these new macros. >>>>> >>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 >>>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ >>>>> Suggested-by: Bill Wendling <morbo@google.com> >>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> >>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >>>> KernelCI found this patch causes a regression in the >>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], >>>> see the bisection report for more details [2]. >>>> >>>> Let me know if you have any questions. >>>> >>>> >>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ >>> Hi Shreeya, >>> Thanks for the report. >>> >>> When I click the above link, then click `multi_v7_defconfig+debug` to >>> get the config necessary to reproduce, I get an HTTP 404. >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config >>> >>> Same for zImage >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage >> Apologies for the broken links. We will try to fix the important ones if >> we can but in the meantime, >> following is the correct link that you can refer. >> >> config :- >> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config >> >> zImage :- >> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage >> >> If you notice, they are present under the kernel directory and same way >> you can find links for other kernel >> builds if you'd like to check them out. >> >>> If I click on the log >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt >>> It looks like the machine powered up, then powered off. Is the test >>> actually failing? >> I recommend checking the html logs from the kernelci dashboard. >> Also, FYI baseline.login test failure means that the device failed to >> boot which I think is causing by the issues that you pointed out. >> >> <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in >> ../arch/arm/mach-sunxi/mc_smp.c:811:29 >> >> And potentially another issue with ftrace >> >> <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at >> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 >> >> >> Let me know if you need more information from my side to reproduce this >> on your end. >> >> >> Thanks, >> Shreeya Patel > Hi Shreeya, > I may need your help to reproduce the failure. > > $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config > -O .config > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s > <launch qemu> > / # mount -t proc /proc > / # cat /proc/version > Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc > (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) > 2.35.2) #2 SMP Wed May 3 21:10:27 UTC 2023 > > I was able to boot the resulting kernel to a command line. Perhaps > there's something about the userspace image that tickles this? Can you > supply the rootfs that's used in testing? Following is the link to the rootfs for a different kernel build which has the same test case failing. http://storage.kernelci.org/images/rootfs/buildroot/buildroot-baseline/20230414.0/armel/rootfs.cpio.gz Lava job :- https://lava.collabora.dev/scheduler/job/10135631 You can usually get the rootfs link and many more information through the "definition" present on Lava job dashboard. Kernelci test id for the above job :- https://linux.kernelci.org/test/case/id/64497865a4bab57def2e85e8/ Thanks, Shreeya Patel >>> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc >>> (Debian 10.2.1-6) in QEMU just fine. So I'm going to need some more >>> information to help reproduce what specifically is failing. >>> >>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc >>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) >>> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023 >>> >>> --- >>> >>> It does look like UBSAN is flagging an array OOB: >>> >>> <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in >>> ../arch/arm/mach-sunxi/mc_smp.c:811:29 >>> >>> And potentially another issue with ftrace >>> >>> <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at >>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 >>> >>> >>> >>>> [2] https://groups.io/g/kernelci-results/message/40804 >>>> >>>> >>>> Thanks, >>>> Shreeya Patel >>>> >>>> #regzbot introduced: 88b61e3bff93 >>>> >>>>> --- >>>>> Changes v3 -> v4: >>>>> * Split into its own patch again from series, as per Masahiro. >>>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update >>>>> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. >>>>> >>>>> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- >>>>> Makefile | 6 ++--- >>>>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- >>>>> scripts/Makefile.compiler | 10 ++++--- >>>>> scripts/Makefile.extrawarn | 4 +-- >>>>> 5 files changed, 29 insertions(+), 22 deletions(-) >>>>> >>>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst >>>>> index 11a296e52d68..ee7e3ea1fbe1 100644 >>>>> --- a/Documentation/kbuild/makefiles.rst >>>>> +++ b/Documentation/kbuild/makefiles.rst >>>>> @@ -682,22 +682,27 @@ more details, with real examples. >>>>> In the above example, -Wno-unused-but-set-variable will be added to >>>>> KBUILD_CFLAGS only if gcc really accepts it. >>>>> >>>>> - cc-ifversion >>>>> - cc-ifversion tests the version of $(CC) and equals the fourth parameter >>>>> - if version expression is true, or the fifth (if given) if the version >>>>> - expression is false. >>>>> + gcc-min-version >>>>> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than >>>>> + or equal to the provided value and evaluates to y if so. >>>>> >>>>> Example:: >>>>> >>>>> - #fs/reiserfs/Makefile >>>>> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) >>>>> + cflags-$(call gcc-min-version, 70100) := -foo >>>>> >>>>> - In this example, ccflags-y will be assigned the value -O1 if the >>>>> - $(CC) version is less than 4.2. >>>>> - cc-ifversion takes all the shell operators: >>>>> - -eq, -ne, -lt, -le, -gt, and -ge >>>>> - The third parameter may be a text as in this example, but it may also >>>>> - be an expanded variable or a macro. >>>>> + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and >>>>> + $(CONFIG_GCC_VERSION) is >= 7.1. >>>>> + >>>>> + clang-min-version >>>>> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater >>>>> + than or equal to the provided value and evaluates to y if so. >>>>> + >>>>> + Example:: >>>>> + >>>>> + cflags-$(call clang-min-version, 110000) := -foo >>>>> + >>>>> + In this example, cflags-y will be assigned the value -foo if $(CC) is clang >>>>> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. >>>>> >>>>> cc-cross-prefix >>>>> cc-cross-prefix is used to check if there exists a $(CC) in path with >>>>> diff --git a/Makefile b/Makefile >>>>> index 298f69060f10..411c8480b37e 100644 >>>>> --- a/Makefile >>>>> +++ b/Makefile >>>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) >>>>> >>>>> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror >>>>> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds >>>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>>>> >>>>> ifdef CONFIG_CC_IS_CLANG >>>>> KBUILD_CPPFLAGS += -Qunused-arguments >>>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC >>>>> KBUILD_CFLAGS += -Wno-maybe-uninitialized >>>>> endif >>>>> >>>>> -ifdef CONFIG_CC_IS_GCC >>>>> # The allocators already balk at large sizes, so silence the compiler >>>>> # warnings for bounds checks involving those possible values. While >>>>> # -Wno-alloc-size-larger-than would normally be used here, earlier versions >>>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC >>>>> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other >>>>> # choice, we must perform a versioned check to disable this warning. >>>>> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au >>>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) >>>>> -endif >>>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than >>>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>>>> >>>>> # disable invalid "can't wrap" optimizations for signed / pointers >>>>> KBUILD_CFLAGS += -fno-strict-overflow >>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>>>> index cb81ed2fbd53..d70838edba80 100644 >>>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile >>>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec >>>>> endif >>>>> >>>>> ifdef CONFIG_CC_IS_GCC >>>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) >>>>> +ifneq ($(call gcc-min-version, 70100),y) >>>>> IS_OLD_GCC = 1 >>>>> endif >>>>> endif >>>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler >>>>> index 94d0d40cddb3..9d18fb91890e 100644 >>>>> --- a/scripts/Makefile.compiler >>>>> +++ b/scripts/Makefile.compiler >>>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ >>>>> cc-disable-warning = $(call try-run,\ >>>>> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) >>>>> >>>>> -# cc-ifversion >>>>> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) >>>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) >>>>> +# gcc-min-version >>>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo >>>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) >>>>> + >>>>> +# clang-min-version >>>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo >>>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) >>>>> >>>>> # ld-option >>>>> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) >>>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn >>>>> index 6ae482158bc4..5769c1939d40 100644 >>>>> --- a/scripts/Makefile.extrawarn >>>>> +++ b/scripts/Makefile.extrawarn >>>>> @@ -48,7 +48,7 @@ else >>>>> ifdef CONFIG_CC_IS_CLANG >>>>> KBUILD_CFLAGS += -Wno-initializer-overrides >>>>> # Clang before clang-16 would warn on default argument promotions. >>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) >>>>> +ifneq ($(call clang-min-version, 160000),y) >>>>> # Disable -Wformat >>>>> KBUILD_CFLAGS += -Wno-format >>>>> # Then re-enable flags that were part of the -Wformat group that aren't >>>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format >>>>> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier >>>>> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull >>>>> # Requires clang-12+. >>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) >>>>> +ifeq ($(call clang-min-version, 120000),y) >>>>> KBUILD_CFLAGS += -Wformat-insufficient-args >>>>> endif >>>>> endif >>> > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-03 22:33 ` Shreeya Patel @ 2023-05-15 23:01 ` Nick Desaulniers 2023-05-17 8:34 ` Shreeya Patel 2023-05-17 15:39 ` Ricardo Cañuelo 0 siblings, 2 replies; 53+ messages in thread From: Nick Desaulniers @ 2023-05-15 23:01 UTC (permalink / raw) To: Shreeya Patel Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, ricardo.canuelo, kernelci On Wed, May 3, 2023 at 3:33 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > > On 04/05/23 02:45, Nick Desaulniers wrote: > > On Wed, May 3, 2023 at 2:02 PM Shreeya Patel > > <shreeya.patel@collabora.com> wrote: > >> Hi Nick, > >> > >> On 28/04/23 22:57, Nick Desaulniers wrote: > >>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel > >>> <shreeya.patel@collabora.com> wrote: > >>>> Hi Nick, > >>>> > >>>> On 19/09/22 22:38, Nick Desaulniers wrote: > >>>>> cc-ifversion is GCC specific. Replace it with compiler specific > >>>>> variants. Update the users of cc-ifversion to use these new macros. > >>>>> > >>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 > >>>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ > >>>>> Suggested-by: Bill Wendling <morbo@google.com> > >>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > >>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > >>>> KernelCI found this patch causes a regression in the > >>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], > >>>> see the bisection report for more details [2]. > >>>> > >>>> Let me know if you have any questions. > >>>> > >>>> > >>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ > >>> Hi Shreeya, > >>> Thanks for the report. > >>> > >>> When I click the above link, then click `multi_v7_defconfig+debug` to > >>> get the config necessary to reproduce, I get an HTTP 404. > >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config > >>> > >>> Same for zImage > >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage > >> Apologies for the broken links. We will try to fix the important ones if > >> we can but in the meantime, > >> following is the correct link that you can refer. > >> > >> config :- > >> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config > >> > >> zImage :- > >> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage > >> > >> If you notice, they are present under the kernel directory and same way > >> you can find links for other kernel > >> builds if you'd like to check them out. > >> > >>> If I click on the log > >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt > >>> It looks like the machine powered up, then powered off. Is the test > >>> actually failing? > >> I recommend checking the html logs from the kernelci dashboard. > >> Also, FYI baseline.login test failure means that the device failed to > >> boot which I think is causing by the issues that you pointed out. > >> > >> <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in > >> ../arch/arm/mach-sunxi/mc_smp.c:811:29 > >> > >> And potentially another issue with ftrace > >> > >> <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at > >> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 > >> > >> > >> Let me know if you need more information from my side to reproduce this > >> on your end. > >> > >> > >> Thanks, > >> Shreeya Patel > > Hi Shreeya, > > I may need your help to reproduce the failure. Hi Shreeya, Sorry for the delay, I was out traveling last week. I still need help reproducing. Trying this again from scratch, my VM is now failing to boot regardless of whether I revert the patch in question or not. Can you please help verify this failure by hand, and see if applying https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch helps? If you can repro; mind sharing your precise steps to reproduce? > > > > $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config > > -O .config > > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s > > <launch qemu> > > / # mount -t proc /proc > > / # cat /proc/version > > Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc > > (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) > > 2.35.2) #2 SMP Wed May 3 21:10:27 UTC 2023 > > > > I was able to boot the resulting kernel to a command line. Perhaps > > there's something about the userspace image that tickles this? Can you > > supply the rootfs that's used in testing? > > > Following is the link to the rootfs for a different kernel build which > has the same test case failing. > http://storage.kernelci.org/images/rootfs/buildroot/buildroot-baseline/20230414.0/armel/rootfs.cpio.gz > > > Lava job :- > https://lava.collabora.dev/scheduler/job/10135631 > > You can usually get the rootfs link and many more information through > the "definition" present on Lava job dashboard. > > Kernelci test id for the above job :- > https://linux.kernelci.org/test/case/id/64497865a4bab57def2e85e8/ > > > Thanks, > Shreeya Patel > > >>> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc > >>> (Debian 10.2.1-6) in QEMU just fine. So I'm going to need some more > >>> information to help reproduce what specifically is failing. > >>> > >>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc > >>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) > >>> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023 > >>> > >>> --- > >>> > >>> It does look like UBSAN is flagging an array OOB: > >>> > >>> <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in > >>> ../arch/arm/mach-sunxi/mc_smp.c:811:29 > >>> > >>> And potentially another issue with ftrace > >>> > >>> <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at > >>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 > >>> > >>> > >>> > >>>> [2] https://groups.io/g/kernelci-results/message/40804 > >>>> > >>>> > >>>> Thanks, > >>>> Shreeya Patel > >>>> > >>>> #regzbot introduced: 88b61e3bff93 > >>>> > >>>>> --- > >>>>> Changes v3 -> v4: > >>>>> * Split into its own patch again from series, as per Masahiro. > >>>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update > >>>>> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. > >>>>> > >>>>> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- > >>>>> Makefile | 6 ++--- > >>>>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- > >>>>> scripts/Makefile.compiler | 10 ++++--- > >>>>> scripts/Makefile.extrawarn | 4 +-- > >>>>> 5 files changed, 29 insertions(+), 22 deletions(-) > >>>>> > >>>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst > >>>>> index 11a296e52d68..ee7e3ea1fbe1 100644 > >>>>> --- a/Documentation/kbuild/makefiles.rst > >>>>> +++ b/Documentation/kbuild/makefiles.rst > >>>>> @@ -682,22 +682,27 @@ more details, with real examples. > >>>>> In the above example, -Wno-unused-but-set-variable will be added to > >>>>> KBUILD_CFLAGS only if gcc really accepts it. > >>>>> > >>>>> - cc-ifversion > >>>>> - cc-ifversion tests the version of $(CC) and equals the fourth parameter > >>>>> - if version expression is true, or the fifth (if given) if the version > >>>>> - expression is false. > >>>>> + gcc-min-version > >>>>> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than > >>>>> + or equal to the provided value and evaluates to y if so. > >>>>> > >>>>> Example:: > >>>>> > >>>>> - #fs/reiserfs/Makefile > >>>>> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) > >>>>> + cflags-$(call gcc-min-version, 70100) := -foo > >>>>> > >>>>> - In this example, ccflags-y will be assigned the value -O1 if the > >>>>> - $(CC) version is less than 4.2. > >>>>> - cc-ifversion takes all the shell operators: > >>>>> - -eq, -ne, -lt, -le, -gt, and -ge > >>>>> - The third parameter may be a text as in this example, but it may also > >>>>> - be an expanded variable or a macro. > >>>>> + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and > >>>>> + $(CONFIG_GCC_VERSION) is >= 7.1. > >>>>> + > >>>>> + clang-min-version > >>>>> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater > >>>>> + than or equal to the provided value and evaluates to y if so. > >>>>> + > >>>>> + Example:: > >>>>> + > >>>>> + cflags-$(call clang-min-version, 110000) := -foo > >>>>> + > >>>>> + In this example, cflags-y will be assigned the value -foo if $(CC) is clang > >>>>> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. > >>>>> > >>>>> cc-cross-prefix > >>>>> cc-cross-prefix is used to check if there exists a $(CC) in path with > >>>>> diff --git a/Makefile b/Makefile > >>>>> index 298f69060f10..411c8480b37e 100644 > >>>>> --- a/Makefile > >>>>> +++ b/Makefile > >>>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) > >>>>> > >>>>> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror > >>>>> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds > >>>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > >>>>> > >>>>> ifdef CONFIG_CC_IS_CLANG > >>>>> KBUILD_CPPFLAGS += -Qunused-arguments > >>>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC > >>>>> KBUILD_CFLAGS += -Wno-maybe-uninitialized > >>>>> endif > >>>>> > >>>>> -ifdef CONFIG_CC_IS_GCC > >>>>> # The allocators already balk at large sizes, so silence the compiler > >>>>> # warnings for bounds checks involving those possible values. While > >>>>> # -Wno-alloc-size-larger-than would normally be used here, earlier versions > >>>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC > >>>>> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other > >>>>> # choice, we must perform a versioned check to disable this warning. > >>>>> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au > >>>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) > >>>>> -endif > >>>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than > >>>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) > >>>>> > >>>>> # disable invalid "can't wrap" optimizations for signed / pointers > >>>>> KBUILD_CFLAGS += -fno-strict-overflow > >>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile > >>>>> index cb81ed2fbd53..d70838edba80 100644 > >>>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile > >>>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile > >>>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec > >>>>> endif > >>>>> > >>>>> ifdef CONFIG_CC_IS_GCC > >>>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) > >>>>> +ifneq ($(call gcc-min-version, 70100),y) > >>>>> IS_OLD_GCC = 1 > >>>>> endif > >>>>> endif > >>>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > >>>>> index 94d0d40cddb3..9d18fb91890e 100644 > >>>>> --- a/scripts/Makefile.compiler > >>>>> +++ b/scripts/Makefile.compiler > >>>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ > >>>>> cc-disable-warning = $(call try-run,\ > >>>>> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) > >>>>> > >>>>> -# cc-ifversion > >>>>> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) > >>>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) > >>>>> +# gcc-min-version > >>>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo > >>>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) > >>>>> + > >>>>> +# clang-min-version > >>>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo > >>>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) > >>>>> > >>>>> # ld-option > >>>>> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) > >>>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > >>>>> index 6ae482158bc4..5769c1939d40 100644 > >>>>> --- a/scripts/Makefile.extrawarn > >>>>> +++ b/scripts/Makefile.extrawarn > >>>>> @@ -48,7 +48,7 @@ else > >>>>> ifdef CONFIG_CC_IS_CLANG > >>>>> KBUILD_CFLAGS += -Wno-initializer-overrides > >>>>> # Clang before clang-16 would warn on default argument promotions. > >>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) > >>>>> +ifneq ($(call clang-min-version, 160000),y) > >>>>> # Disable -Wformat > >>>>> KBUILD_CFLAGS += -Wno-format > >>>>> # Then re-enable flags that were part of the -Wformat group that aren't > >>>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format > >>>>> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier > >>>>> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull > >>>>> # Requires clang-12+. > >>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) > >>>>> +ifeq ($(call clang-min-version, 120000),y) > >>>>> KBUILD_CFLAGS += -Wformat-insufficient-args > >>>>> endif > >>>>> endif > >>> > > > > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-15 23:01 ` Nick Desaulniers @ 2023-05-17 8:34 ` Shreeya Patel 2023-05-17 15:39 ` Ricardo Cañuelo 1 sibling, 0 replies; 53+ messages in thread From: Shreeya Patel @ 2023-05-17 8:34 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, ricardo.canuelo, kernelci Hi Nick, On 16/05/23 04:31, Nick Desaulniers wrote: > On Wed, May 3, 2023 at 3:33 PM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> >> On 04/05/23 02:45, Nick Desaulniers wrote: >>> On Wed, May 3, 2023 at 2:02 PM Shreeya Patel >>> <shreeya.patel@collabora.com> wrote: >>>> Hi Nick, >>>> >>>> On 28/04/23 22:57, Nick Desaulniers wrote: >>>>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel >>>>> <shreeya.patel@collabora.com> wrote: >>>>>> Hi Nick, >>>>>> >>>>>> On 19/09/22 22:38, Nick Desaulniers wrote: >>>>>>> cc-ifversion is GCC specific. Replace it with compiler specific >>>>>>> variants. Update the users of cc-ifversion to use these new macros. >>>>>>> >>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350 >>>>>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/ >>>>>>> Suggested-by: Bill Wendling <morbo@google.com> >>>>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org> >>>>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >>>>>> KernelCI found this patch causes a regression in the >>>>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1], >>>>>> see the bisection report for more details [2]. >>>>>> >>>>>> Let me know if you have any questions. >>>>>> >>>>>> >>>>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/ >>>>> Hi Shreeya, >>>>> Thanks for the report. >>>>> >>>>> When I click the above link, then click `multi_v7_defconfig+debug` to >>>>> get the config necessary to reproduce, I get an HTTP 404. >>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config >>>>> >>>>> Same for zImage >>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage >>>> Apologies for the broken links. We will try to fix the important ones if >>>> we can but in the meantime, >>>> following is the correct link that you can refer. >>>> >>>> config :- >>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config >>>> >>>> zImage :- >>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage >>>> >>>> If you notice, they are present under the kernel directory and same way >>>> you can find links for other kernel >>>> builds if you'd like to check them out. >>>> >>>>> If I click on the log >>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt >>>>> It looks like the machine powered up, then powered off. Is the test >>>>> actually failing? >>>> I recommend checking the html logs from the kernelci dashboard. >>>> Also, FYI baseline.login test failure means that the device failed to >>>> boot which I think is causing by the issues that you pointed out. >>>> >>>> <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in >>>> ../arch/arm/mach-sunxi/mc_smp.c:811:29 >>>> >>>> And potentially another issue with ftrace >>>> >>>> <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at >>>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 >>>> >>>> >>>> Let me know if you need more information from my side to reproduce this >>>> on your end. >>>> >>>> >>>> Thanks, >>>> Shreeya Patel >>> Hi Shreeya, >>> I may need your help to reproduce the failure. > Hi Shreeya, > Sorry for the delay, I was out traveling last week. I still need help > reproducing. Trying this again from scratch, my VM is now failing to > boot regardless of whether I revert the patch in question or not. > > Can you please help verify this failure by hand, and see if applying > https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch > helps? If you can repro; mind sharing your precise steps to reproduce? No worries, even I was out traveling last week so wouldn't have been able to help you either. Ricardo from my team is currently testing the patch that you have sent. We will let you know the results soon. I haven't really tried to manually reproduce it myself yet but I'll check and let you know what I can do on my end. Thanks, Shreeya Patel >>> $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config >>> -O .config >>> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s >>> <launch qemu> >>> / # mount -t proc /proc >>> / # cat /proc/version >>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc >>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) >>> 2.35.2) #2 SMP Wed May 3 21:10:27 UTC 2023 >>> >>> I was able to boot the resulting kernel to a command line. Perhaps >>> there's something about the userspace image that tickles this? Can you >>> supply the rootfs that's used in testing? >> >> Following is the link to the rootfs for a different kernel build which >> has the same test case failing. >> http://storage.kernelci.org/images/rootfs/buildroot/buildroot-baseline/20230414.0/armel/rootfs.cpio.gz >> >> >> Lava job :- >> https://lava.collabora.dev/scheduler/job/10135631 >> >> You can usually get the rootfs link and many more information through >> the "definition" present on Lava job dashboard. >> >> Kernelci test id for the above job :- >> https://linux.kernelci.org/test/case/id/64497865a4bab57def2e85e8/ >> >> >> Thanks, >> Shreeya Patel >> >>>>> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc >>>>> (Debian 10.2.1-6) in QEMU just fine. So I'm going to need some more >>>>> information to help reproduce what specifically is failing. >>>>> >>>>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc >>>>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) >>>>> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023 >>>>> >>>>> --- >>>>> >>>>> It does look like UBSAN is flagging an array OOB: >>>>> >>>>> <3>[ 0.417001][ T1] UBSAN: array-index-out-of-bounds in >>>>> ../arch/arm/mach-sunxi/mc_smp.c:811:29 >>>>> >>>>> And potentially another issue with ftrace >>>>> >>>>> <4>[ 0.000000][ T0] WARNING: CPU: 0 PID: 0 at >>>>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4 >>>>> >>>>> >>>>> >>>>>> [2] https://groups.io/g/kernelci-results/message/40804 >>>>>> >>>>>> >>>>>> Thanks, >>>>>> Shreeya Patel >>>>>> >>>>>> #regzbot introduced: 88b61e3bff93 >>>>>> >>>>>>> --- >>>>>>> Changes v3 -> v4: >>>>>>> * Split into its own patch again from series, as per Masahiro. >>>>>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update >>>>>>> clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro. >>>>>>> >>>>>>> Documentation/kbuild/makefiles.rst | 29 ++++++++++++--------- >>>>>>> Makefile | 6 ++--- >>>>>>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 +- >>>>>>> scripts/Makefile.compiler | 10 ++++--- >>>>>>> scripts/Makefile.extrawarn | 4 +-- >>>>>>> 5 files changed, 29 insertions(+), 22 deletions(-) >>>>>>> >>>>>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst >>>>>>> index 11a296e52d68..ee7e3ea1fbe1 100644 >>>>>>> --- a/Documentation/kbuild/makefiles.rst >>>>>>> +++ b/Documentation/kbuild/makefiles.rst >>>>>>> @@ -682,22 +682,27 @@ more details, with real examples. >>>>>>> In the above example, -Wno-unused-but-set-variable will be added to >>>>>>> KBUILD_CFLAGS only if gcc really accepts it. >>>>>>> >>>>>>> - cc-ifversion >>>>>>> - cc-ifversion tests the version of $(CC) and equals the fourth parameter >>>>>>> - if version expression is true, or the fifth (if given) if the version >>>>>>> - expression is false. >>>>>>> + gcc-min-version >>>>>>> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than >>>>>>> + or equal to the provided value and evaluates to y if so. >>>>>>> >>>>>>> Example:: >>>>>>> >>>>>>> - #fs/reiserfs/Makefile >>>>>>> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1) >>>>>>> + cflags-$(call gcc-min-version, 70100) := -foo >>>>>>> >>>>>>> - In this example, ccflags-y will be assigned the value -O1 if the >>>>>>> - $(CC) version is less than 4.2. >>>>>>> - cc-ifversion takes all the shell operators: >>>>>>> - -eq, -ne, -lt, -le, -gt, and -ge >>>>>>> - The third parameter may be a text as in this example, but it may also >>>>>>> - be an expanded variable or a macro. >>>>>>> + In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and >>>>>>> + $(CONFIG_GCC_VERSION) is >= 7.1. >>>>>>> + >>>>>>> + clang-min-version >>>>>>> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater >>>>>>> + than or equal to the provided value and evaluates to y if so. >>>>>>> + >>>>>>> + Example:: >>>>>>> + >>>>>>> + cflags-$(call clang-min-version, 110000) := -foo >>>>>>> + >>>>>>> + In this example, cflags-y will be assigned the value -foo if $(CC) is clang >>>>>>> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0. >>>>>>> >>>>>>> cc-cross-prefix >>>>>>> cc-cross-prefix is used to check if there exists a $(CC) in path with >>>>>>> diff --git a/Makefile b/Makefile >>>>>>> index 298f69060f10..411c8480b37e 100644 >>>>>>> --- a/Makefile >>>>>>> +++ b/Makefile >>>>>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y) >>>>>>> >>>>>>> KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror >>>>>>> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds >>>>>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>>>>>> >>>>>>> ifdef CONFIG_CC_IS_CLANG >>>>>>> KBUILD_CPPFLAGS += -Qunused-arguments >>>>>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC >>>>>>> KBUILD_CFLAGS += -Wno-maybe-uninitialized >>>>>>> endif >>>>>>> >>>>>>> -ifdef CONFIG_CC_IS_GCC >>>>>>> # The allocators already balk at large sizes, so silence the compiler >>>>>>> # warnings for bounds checks involving those possible values. While >>>>>>> # -Wno-alloc-size-larger-than would normally be used here, earlier versions >>>>>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC >>>>>>> # ignored, continuing to default to PTRDIFF_MAX. So, left with no other >>>>>>> # choice, we must perform a versioned check to disable this warning. >>>>>>> # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au >>>>>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than) >>>>>>> -endif >>>>>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than >>>>>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) >>>>>>> >>>>>>> # disable invalid "can't wrap" optimizations for signed / pointers >>>>>>> KBUILD_CFLAGS += -fno-strict-overflow >>>>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>>>>>> index cb81ed2fbd53..d70838edba80 100644 >>>>>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile >>>>>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile >>>>>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec >>>>>>> endif >>>>>>> >>>>>>> ifdef CONFIG_CC_IS_GCC >>>>>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y) >>>>>>> +ifneq ($(call gcc-min-version, 70100),y) >>>>>>> IS_OLD_GCC = 1 >>>>>>> endif >>>>>>> endif >>>>>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler >>>>>>> index 94d0d40cddb3..9d18fb91890e 100644 >>>>>>> --- a/scripts/Makefile.compiler >>>>>>> +++ b/scripts/Makefile.compiler >>>>>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\ >>>>>>> cc-disable-warning = $(call try-run,\ >>>>>>> $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) >>>>>>> >>>>>>> -# cc-ifversion >>>>>>> -# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) >>>>>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) >>>>>>> +# gcc-min-version >>>>>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo >>>>>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y) >>>>>>> + >>>>>>> +# clang-min-version >>>>>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo >>>>>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y) >>>>>>> >>>>>>> # ld-option >>>>>>> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) >>>>>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn >>>>>>> index 6ae482158bc4..5769c1939d40 100644 >>>>>>> --- a/scripts/Makefile.extrawarn >>>>>>> +++ b/scripts/Makefile.extrawarn >>>>>>> @@ -48,7 +48,7 @@ else >>>>>>> ifdef CONFIG_CC_IS_CLANG >>>>>>> KBUILD_CFLAGS += -Wno-initializer-overrides >>>>>>> # Clang before clang-16 would warn on default argument promotions. >>>>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y) >>>>>>> +ifneq ($(call clang-min-version, 160000),y) >>>>>>> # Disable -Wformat >>>>>>> KBUILD_CFLAGS += -Wno-format >>>>>>> # Then re-enable flags that were part of the -Wformat group that aren't >>>>>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format >>>>>>> KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier >>>>>>> KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull >>>>>>> # Requires clang-12+. >>>>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y) >>>>>>> +ifeq ($(call clang-min-version, 120000),y) >>>>>>> KBUILD_CFLAGS += -Wformat-insufficient-args >>>>>>> endif >>>>>>> endif >>> > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-15 23:01 ` Nick Desaulniers 2023-05-17 8:34 ` Shreeya Patel @ 2023-05-17 15:39 ` Ricardo Cañuelo 2023-05-17 16:27 ` Nick Desaulniers 1 sibling, 1 reply; 53+ messages in thread From: Ricardo Cañuelo @ 2023-05-17 15:39 UTC (permalink / raw) To: Nick Desaulniers, Shreeya Patel Cc: Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci Hi Nick, On 16/5/23 1:01, Nick Desaulniers wrote: > Can you please help verify this failure by hand, and see if applying > https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch > helps? If you can repro; mind sharing your precise steps to reproduce? I ran a few tests but the commit that introduced your changes passes every time. There's a chance that the bisector got misled due to the test runs failing for whatever reason unrelated to the patch. There's definitely something introducing a bug somewhere, as current mainline/master makes this test fail on this target when kernel/configs/debug.config is applied, but it must be somewhere else. I'll investigate this some more to see what I can find. About the steps to reproduce it, we're using the current KernelCI tools (kci_build) to generate the kernel. To actually launch the tests I'm submitting jobs to Collabora's LAVA lab, which is something that isn't available to external users, so it might be a bit hard for you to reproduce the exact environment from the original test. If you need to test something, I can do it for you. Thanks, Ricardo ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-17 15:39 ` Ricardo Cañuelo @ 2023-05-17 16:27 ` Nick Desaulniers 2023-05-18 14:23 ` Ricardo Cañuelo 0 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2023-05-17 16:27 UTC (permalink / raw) To: Ricardo Cañuelo Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Wed, May 17, 2023 at 8:39 AM Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote: > > Hi Nick, > > On 16/5/23 1:01, Nick Desaulniers wrote: > > Can you please help verify this failure by hand, and see if applying > > https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch > > helps? If you can repro; mind sharing your precise steps to reproduce? > > I ran a few tests but the commit that introduced your changes > passes every time. There's a chance that the bisector got misled > due to the test runs failing for whatever reason unrelated to the > patch. There's definitely something introducing a bug somewhere, > as current mainline/master makes this test fail on this target > when kernel/configs/debug.config is applied, but it must be > somewhere else. I'll investigate this some more to see what I can > find. Thanks for verifying/reporting. > > About the steps to reproduce it, we're using the current KernelCI > tools (kci_build) to generate the kernel. To actually launch the > tests I'm submitting jobs to Collabora's LAVA lab, which is > something that isn't available to external users, so it might be > a bit hard for you to reproduce the exact environment from the > original test. If you need to test something, I can do it for > you. Shreeya mentioned upthread that `qemu_arm-virt-gicv3-uefi` was failing, so I assume others should be able to repro in qemu. I'd guess that LAVA lets you have VMs adjacent to physical hardware. Having the qemu command line, kernel config, and perhaps the initramfs are going to be the three most useful things for any similar bug report. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-17 16:27 ` Nick Desaulniers @ 2023-05-18 14:23 ` Ricardo Cañuelo 2023-05-18 21:12 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Ricardo Cañuelo @ 2023-05-18 14:23 UTC (permalink / raw) To: Nick Desaulniers Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci Hi Nick, On mié, may 17 2023 at 09:27:41, Nick Desaulniers <ndesaulniers@google.com> wrote: > Shreeya mentioned upthread that `qemu_arm-virt-gicv3-uefi` was > failing, so I assume others should be able to repro in qemu. I'd guess > that LAVA lets you have VMs adjacent to physical hardware. Having the > qemu command line, kernel config, and perhaps the initramfs are going > to be the three most useful things for any similar bug report. Sure, I'm using LAVA because that's the safest way to match exactly the same setup from the original test, but anyone can try to reproduce it on their own. You can get the job info and setup from any of the related LAVA jobs. This one for example: https://lava.collabora.dev/scheduler/job/10373216 Trying to reproduce this type of setups is not as straightforward as we'd like, specially for people not familiar with KernelCI, but we're putting in some effort to improve that so that anyone can pick up a regression report and work on it right away. By the way, I found a breaking point in the commit right after the one that the bisector reported: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5750121ae7382ebac8d47ce6d68012d6cd1d7926 but I can't find anything there either that makes the boot hang, specifically for configs including kernel/configs/debug.config. I wouldn't rule out a problem with the qemu configuration. Anyway, this is not a critical problem in any way, although it'd be interesting to find the cause in case we can use the findings to improve the test setups. Cheers, Ricardo ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-18 14:23 ` Ricardo Cañuelo @ 2023-05-18 21:12 ` Nick Desaulniers 2023-05-19 8:35 ` Ricardo Cañuelo 0 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2023-05-18 21:12 UTC (permalink / raw) To: Ricardo Cañuelo Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Thu, May 18, 2023 at 7:23 AM Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote: > > > Hi Nick, > > On mié, may 17 2023 at 09:27:41, Nick Desaulniers <ndesaulniers@google.com> wrote: > > Shreeya mentioned upthread that `qemu_arm-virt-gicv3-uefi` was > > failing, so I assume others should be able to repro in qemu. I'd guess > > that LAVA lets you have VMs adjacent to physical hardware. Having the > > qemu command line, kernel config, and perhaps the initramfs are going > > to be the three most useful things for any similar bug report. > > Sure, I'm using LAVA because that's the safest way to match exactly the > same setup from the original test, but anyone can try to reproduce it on > their own. You can get the job info and setup from any of the related > LAVA jobs. This one for example: > https://lava.collabora.dev/scheduler/job/10373216 > > Trying to reproduce this type of setups is not as straightforward as > we'd like, specially for people not familiar with KernelCI, but we're > putting in some effort to improve that so that anyone can pick up a > regression report and work on it right away. Yeah makes sense. Having reports surface the most relevant info for developers to understand what went wrong where and how to reproduce is something I'm passionate about. We've been through this with Intel 0day bot folks; it takes a few iterations until everyone is happy. > > By the way, I found a breaking point in the commit right after the one > that the bisector reported: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5750121ae7382ebac8d47ce6d68012d6cd1d7926 That's a higher risk change (and has my name on the tested-by tag, yikes). So is that the culprit of the boot failure you're observing? > but I can't find anything there either that makes the boot hang, > specifically for configs including kernel/configs/debug.config. I > wouldn't rule out a problem with the qemu configuration. > > Anyway, this is not a critical problem in any way, although it'd be But you still have a boot failure/regression to track down for that config, right? > interesting to find the cause in case we can use the findings to improve > the test setups. > > Cheers, > Ricardo -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-18 21:12 ` Nick Desaulniers @ 2023-05-19 8:35 ` Ricardo Cañuelo 2023-05-19 15:57 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Ricardo Cañuelo @ 2023-05-19 8:35 UTC (permalink / raw) To: Nick Desaulniers Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On jue, may 18 2023 at 14:12:30, Nick Desaulniers <ndesaulniers@google.com> wrote: > That's a higher risk change (and has my name on the tested-by tag, yikes). > > So is that the culprit of the boot failure you're observing? Right now it is. Here's a test run using that commit (5750121ae7382ebac8d47ce6d68012d6cd1d7926): https://lava.collabora.dev/scheduler/job/10373216 Here's one with the commit right after that one (26ef40de5cbb24728a34a319e8d42cdec99f186c): https://lava.collabora.dev/scheduler/job/10371513 Then one with 26ef40de5cbb24728a34a319e8d42cdec99f186c with a revert commit for 5750121ae7382ebac8d47ce6d68012d6cd1d7926 on top: https://lava.collabora.dev/scheduler/job/10371882 But I'm not confident enough to jump ahead and call this a kernel regression, specially after the bisector confidently said that about your commit and then it turned out none of us could reproduce it. There have been some cases where a commit made a test fail (kernel failing to load, for instance) and the real problem was that the kernel got bigger than the target was capable of handling. So not a problem with the commit at all, it was just that the memory mappings needed to be redefined for that target. What I'm saying is that sometimes a regression report is really uncovering a problem in the test setup rather than introducing a bug. Maybe this is one of those cases. Cheers, Ricardo ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-19 8:35 ` Ricardo Cañuelo @ 2023-05-19 15:57 ` Nick Desaulniers 2023-05-22 10:09 ` Ricardo Cañuelo 0 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2023-05-19 15:57 UTC (permalink / raw) To: Ricardo Cañuelo Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci, Greg KH On Fri, May 19, 2023 at 1:35 AM Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote: > > On jue, may 18 2023 at 14:12:30, Nick Desaulniers <ndesaulniers@google.com> wrote: > > That's a higher risk change (and has my name on the tested-by tag, yikes). > > > > So is that the culprit of the boot failure you're observing? > > Right now it is. > > Here's a test run using that commit > (5750121ae7382ebac8d47ce6d68012d6cd1d7926): > https://lava.collabora.dev/scheduler/job/10373216 > > Here's one with the commit right after that one > (26ef40de5cbb24728a34a319e8d42cdec99f186c): > https://lava.collabora.dev/scheduler/job/10371513 > > Then one with 26ef40de5cbb24728a34a319e8d42cdec99f186c with a revert > commit for 5750121ae7382ebac8d47ce6d68012d6cd1d7926 on top: > https://lava.collabora.dev/scheduler/job/10371882 > > But I'm not confident enough to jump ahead and call this a kernel > regression, specially after the bisector confidently said that about > your commit and then it turned out none of us could reproduce it. It could be; if the link order was changed, it's possible that this target may be hitting something along the lines of: https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static initialization order fiasco" I'm struggling to think of how this appears in C codebases, but I swear years ago I had a discussion with GKH (maybe?) about this. I think I was playing with converting Kbuild to use Ninja rather than Make; the resulting kernel image wouldn't boot because I had modified the order the object files were linked in. If you were to randomly shuffle the object files in the kernel, I recall some hazard that may prevent boot. > > There have been some cases where a commit made a test fail (kernel > failing to load, for instance) and the real problem was that the kernel > got bigger than the target was capable of handling. So not a problem > with the commit at all, it was just that the memory mappings needed to > be redefined for that target. What I'm saying is that sometimes a > regression report is really uncovering a problem in the test setup > rather than introducing a bug. Maybe this is one of those cases. > > Cheers, > Ricardo -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-19 15:57 ` Nick Desaulniers @ 2023-05-22 10:09 ` Ricardo Cañuelo 2023-05-22 16:52 ` Greg KH 0 siblings, 1 reply; 53+ messages in thread From: Ricardo Cañuelo @ 2023-05-22 10:09 UTC (permalink / raw) To: Nick Desaulniers Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci, Greg KH On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > It could be; if the link order was changed, it's possible that this > target may be hitting something along the lines of: > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > initialization order fiasco" > > I'm struggling to think of how this appears in C codebases, but I > swear years ago I had a discussion with GKH (maybe?) about this. I > think I was playing with converting Kbuild to use Ninja rather than > Make; the resulting kernel image wouldn't boot because I had modified > the order the object files were linked in. If you were to randomly > shuffle the object files in the kernel, I recall some hazard that may > prevent boot. I thought that was specifically a C++ problem? But then again, the kernel docs explicitly say that the ordering of obj-y goals in kbuild is significant in some instances [1]: --- 3.2 Built-in object goals - obj-y [...] Link order is significant, because certain functions (module_init() / __initcall) will be called during boot in the order they appear. So keep in mind that changing the link order may e.g. change the order in which your SCSI controllers are detected, and thus your disks are renumbered. We'll dig deeper into this. Thanks for your insight. Cheers, Ricardo [1]: https://www.kernel.org/doc/Documentation/kbuild/makefiles.txt ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-22 10:09 ` Ricardo Cañuelo @ 2023-05-22 16:52 ` Greg KH 2023-05-22 19:52 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Greg KH @ 2023-05-22 16:52 UTC (permalink / raw) To: Ricardo Cañuelo Cc: Nick Desaulniers, Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > > It could be; if the link order was changed, it's possible that this > > target may be hitting something along the lines of: > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > > initialization order fiasco" > > > > I'm struggling to think of how this appears in C codebases, but I > > swear years ago I had a discussion with GKH (maybe?) about this. I > > think I was playing with converting Kbuild to use Ninja rather than > > Make; the resulting kernel image wouldn't boot because I had modified > > the order the object files were linked in. If you were to randomly > > shuffle the object files in the kernel, I recall some hazard that may > > prevent boot. > > I thought that was specifically a C++ problem? But then again, the > kernel docs explicitly say that the ordering of obj-y goals in kbuild is > significant in some instances [1]: Yes, it matters, you can not change it. If you do, systems will break. It is the only way we have of properly ordering our init calls within the same "level". thanks, greg k-h ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-22 16:52 ` Greg KH @ 2023-05-22 19:52 ` Nick Desaulniers 2023-05-22 20:01 ` Greg KH 2023-05-23 10:27 ` Shreeya Patel 0 siblings, 2 replies; 53+ messages in thread From: Nick Desaulniers @ 2023-05-22 19:52 UTC (permalink / raw) To: Greg KH, Maksim Panchenko, Ricardo Cañuelo Cc: Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: > > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > It could be; if the link order was changed, it's possible that this > > > target may be hitting something along the lines of: > > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > > > initialization order fiasco" > > > > > > I'm struggling to think of how this appears in C codebases, but I > > > swear years ago I had a discussion with GKH (maybe?) about this. I > > > think I was playing with converting Kbuild to use Ninja rather than > > > Make; the resulting kernel image wouldn't boot because I had modified > > > the order the object files were linked in. If you were to randomly > > > shuffle the object files in the kernel, I recall some hazard that may > > > prevent boot. > > > > I thought that was specifically a C++ problem? But then again, the > > kernel docs explicitly say that the ordering of obj-y goals in kbuild is > > significant in some instances [1]: > > Yes, it matters, you can not change it. If you do, systems will break. > It is the only way we have of properly ordering our init calls within > the same "level". Ah, right it was the initcall ordering. Thanks for the reminder. (There's a joke in there similar to the use of regexes to solve a problem resulting in two new problems; initcalls have levels for ordering, but we still have (unexpressed) dependencies between calls of the same level; brittle!). +Maksim, since that might be relevant info for the BOLT+Kernel work. Ricardo, https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf mentions that there's a kernel command line param `initcall_debug`. Perhaps that can be used to see if 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall ordering, resulting in a config that cannot boot? -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-22 19:52 ` Nick Desaulniers @ 2023-05-22 20:01 ` Greg KH 2023-05-22 20:16 ` Nick Desaulniers 2023-05-23 10:27 ` Shreeya Patel 1 sibling, 1 reply; 53+ messages in thread From: Greg KH @ 2023-05-22 20:01 UTC (permalink / raw) To: Nick Desaulniers Cc: Maksim Panchenko, Ricardo Cañuelo, Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Mon, May 22, 2023 at 12:52:13PM -0700, Nick Desaulniers wrote: > On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: > > > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > It could be; if the link order was changed, it's possible that this > > > > target may be hitting something along the lines of: > > > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > > > > initialization order fiasco" > > > > > > > > I'm struggling to think of how this appears in C codebases, but I > > > > swear years ago I had a discussion with GKH (maybe?) about this. I > > > > think I was playing with converting Kbuild to use Ninja rather than > > > > Make; the resulting kernel image wouldn't boot because I had modified > > > > the order the object files were linked in. If you were to randomly > > > > shuffle the object files in the kernel, I recall some hazard that may > > > > prevent boot. > > > > > > I thought that was specifically a C++ problem? But then again, the > > > kernel docs explicitly say that the ordering of obj-y goals in kbuild is > > > significant in some instances [1]: > > > > Yes, it matters, you can not change it. If you do, systems will break. > > It is the only way we have of properly ordering our init calls within > > the same "level". > > Ah, right it was the initcall ordering. Thanks for the reminder. > > (There's a joke in there similar to the use of regexes to solve a > problem resulting in two new problems; initcalls have levels for > ordering, but we still have (unexpressed) dependencies between calls > of the same level; brittle!). No, the dependencies are explicitly expressed with the linker order. So it's not brittle, but rather very deterministic. When linker order didn't work for all sorts of things, we added different levels, but due to the huge number of init calls, of course can not give each one their own level. It's always been this way with Linux, nothing new here at all :) thanks, greg k-h ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-22 20:01 ` Greg KH @ 2023-05-22 20:16 ` Nick Desaulniers 0 siblings, 0 replies; 53+ messages in thread From: Nick Desaulniers @ 2023-05-22 20:16 UTC (permalink / raw) To: Greg KH Cc: Maksim Panchenko, Ricardo Cañuelo, Shreeya Patel, Michal Marek, Masahiro Yamada, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Mon, May 22, 2023 at 1:01 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Mon, May 22, 2023 at 12:52:13PM -0700, Nick Desaulniers wrote: > > On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: > > > > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > It could be; if the link order was changed, it's possible that this > > > > > target may be hitting something along the lines of: > > > > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > > > > > initialization order fiasco" > > > > > > > > > > I'm struggling to think of how this appears in C codebases, but I > > > > > swear years ago I had a discussion with GKH (maybe?) about this. I > > > > > think I was playing with converting Kbuild to use Ninja rather than > > > > > Make; the resulting kernel image wouldn't boot because I had modified > > > > > the order the object files were linked in. If you were to randomly > > > > > shuffle the object files in the kernel, I recall some hazard that may > > > > > prevent boot. > > > > > > > > I thought that was specifically a C++ problem? But then again, the > > > > kernel docs explicitly say that the ordering of obj-y goals in kbuild is > > > > significant in some instances [1]: > > > > > > Yes, it matters, you can not change it. If you do, systems will break. > > > It is the only way we have of properly ordering our init calls within > > > the same "level". > > > > Ah, right it was the initcall ordering. Thanks for the reminder. > > > > (There's a joke in there similar to the use of regexes to solve a > > problem resulting in two new problems; initcalls have levels for > > ordering, but we still have (unexpressed) dependencies between calls > > of the same level; brittle!). > > No, the dependencies are explicitly expressed with the linker order. So I don't consider that "explicit." The link order of object files does not express what symbols (if any) are initcalls which are dependent on other symbols/initcalls from which object file. > it's not brittle, but rather very deterministic. Brittle != non-deterministic. We now have implicit dependencies between some init calls, but not all. Given two initcalls, are you confident that you could tell which must run before the other, if there is even such a dependency? It prevents us from reordering symbol layout for performance (or security via FGKASLR), safely. If such dependencies were *explicit*, we could do so safely since we'd have information about which initcalls are dependencies or not. The implicit nature of such dependencies is thus what I would consider brittle. Hopefully initcall ordering related changes isn't the root cause of the boot failure reported here, lest that lend more evidence to my claim. > > When linker order didn't work for all sorts of things, we added > different levels, but due to the huge number of init calls, of course > can not give each one their own level. > > It's always been this way with Linux, nothing new here at all :) :^) > > thanks, > > greg k-h -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-22 19:52 ` Nick Desaulniers 2023-05-22 20:01 ` Greg KH @ 2023-05-23 10:27 ` Shreeya Patel 2023-05-23 21:27 ` Nick Desaulniers 1 sibling, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-05-23 10:27 UTC (permalink / raw) To: Nick Desaulniers, Masahiro Yamada, Greg KH, Maksim Panchenko, Ricardo Cañuelo Cc: Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci Hi Nick and Masahiro, On 23/05/23 01:22, Nick Desaulniers wrote: > On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: >> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: >>>> It could be; if the link order was changed, it's possible that this >>>> target may be hitting something along the lines of: >>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static >>>> initialization order fiasco" >>>> >>>> I'm struggling to think of how this appears in C codebases, but I >>>> swear years ago I had a discussion with GKH (maybe?) about this. I >>>> think I was playing with converting Kbuild to use Ninja rather than >>>> Make; the resulting kernel image wouldn't boot because I had modified >>>> the order the object files were linked in. If you were to randomly >>>> shuffle the object files in the kernel, I recall some hazard that may >>>> prevent boot. >>> I thought that was specifically a C++ problem? But then again, the >>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is >>> significant in some instances [1]: >> Yes, it matters, you can not change it. If you do, systems will break. >> It is the only way we have of properly ordering our init calls within >> the same "level". > Ah, right it was the initcall ordering. Thanks for the reminder. > > (There's a joke in there similar to the use of regexes to solve a > problem resulting in two new problems; initcalls have levels for > ordering, but we still have (unexpressed) dependencies between calls > of the same level; brittle!). > > +Maksim, since that might be relevant info for the BOLT+Kernel work. > > Ricardo, > https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf > mentions that there's a kernel command line param `initcall_debug`. > Perhaps that can be used to see if > 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall > ordering, resulting in a config that cannot boot? Here are the links to Lava jobs ran with initcall_debug added to the kernel command line. 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926) https://lava.collabora.dev/scheduler/job/10417706 <https://lava.collabora.dev/scheduler/job/10417706> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926 https://lava.collabora.dev/scheduler/job/10418012 <https://lava.collabora.dev/scheduler/job/10418012> Thanks, Shreeya Patel ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-23 10:27 ` Shreeya Patel @ 2023-05-23 21:27 ` Nick Desaulniers 2023-06-12 10:10 ` Shreeya Patel 0 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2023-05-23 21:27 UTC (permalink / raw) To: Shreeya Patel, Masahiro Yamada Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, kernelci On Tue, May 23, 2023 at 3:27 AM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > Hi Nick and Masahiro, > > On 23/05/23 01:22, Nick Desaulniers wrote: > > On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: > >> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: > >>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > >>>> It could be; if the link order was changed, it's possible that this > >>>> target may be hitting something along the lines of: > >>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > >>>> initialization order fiasco" > >>>> > >>>> I'm struggling to think of how this appears in C codebases, but I > >>>> swear years ago I had a discussion with GKH (maybe?) about this. I > >>>> think I was playing with converting Kbuild to use Ninja rather than > >>>> Make; the resulting kernel image wouldn't boot because I had modified > >>>> the order the object files were linked in. If you were to randomly > >>>> shuffle the object files in the kernel, I recall some hazard that may > >>>> prevent boot. > >>> I thought that was specifically a C++ problem? But then again, the > >>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is > >>> significant in some instances [1]: > >> Yes, it matters, you can not change it. If you do, systems will break. > >> It is the only way we have of properly ordering our init calls within > >> the same "level". > > Ah, right it was the initcall ordering. Thanks for the reminder. > > > > (There's a joke in there similar to the use of regexes to solve a > > problem resulting in two new problems; initcalls have levels for > > ordering, but we still have (unexpressed) dependencies between calls > > of the same level; brittle!). > > > > +Maksim, since that might be relevant info for the BOLT+Kernel work. > > > > Ricardo, > > https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf > > mentions that there's a kernel command line param `initcall_debug`. > > Perhaps that can be used to see if > > 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall > > ordering, resulting in a config that cannot boot? > > > Here are the links to Lava jobs ran with initcall_debug added to the > kernel command line. > > 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926) > https://lava.collabora.dev/scheduler/job/10417706 > <https://lava.collabora.dev/scheduler/job/10417706> > > 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926 > https://lava.collabora.dev/scheduler/job/10418012 > <https://lava.collabora.dev/scheduler/job/10418012> Thanks! Yeah, I can see a diff in the initcall ordering as a result of commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 Not just different orderings, but some initcalls seem unique to the before vs. after, which is troubling. (example init_events and init_fs_sysctls respectively) That isn't conclusive evidence that changes to initcall ordering are to blame, but I suspect confirming that precisely to be very very time consuming. Masahiro, what are your thoughts on reverting 5750121ae738? There are conflicts in Kbuild and Makefile when reverting 5750121ae738 on mainline. > > > Thanks, > Shreeya Patel > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-05-23 21:27 ` Nick Desaulniers @ 2023-06-12 10:10 ` Shreeya Patel 2023-06-20 4:19 ` Masahiro Yamada 0 siblings, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-06-12 10:10 UTC (permalink / raw) To: Masahiro Yamada Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML Hi Masahiro, On 24/05/23 02:57, Nick Desaulniers wrote: > On Tue, May 23, 2023 at 3:27 AM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> Hi Nick and Masahiro, >> >> On 23/05/23 01:22, Nick Desaulniers wrote: >>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: >>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: >>>>>> It could be; if the link order was changed, it's possible that this >>>>>> target may be hitting something along the lines of: >>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static >>>>>> initialization order fiasco" >>>>>> >>>>>> I'm struggling to think of how this appears in C codebases, but I >>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I >>>>>> think I was playing with converting Kbuild to use Ninja rather than >>>>>> Make; the resulting kernel image wouldn't boot because I had modified >>>>>> the order the object files were linked in. If you were to randomly >>>>>> shuffle the object files in the kernel, I recall some hazard that may >>>>>> prevent boot. >>>>> I thought that was specifically a C++ problem? But then again, the >>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is >>>>> significant in some instances [1]: >>>> Yes, it matters, you can not change it. If you do, systems will break. >>>> It is the only way we have of properly ordering our init calls within >>>> the same "level". >>> Ah, right it was the initcall ordering. Thanks for the reminder. >>> >>> (There's a joke in there similar to the use of regexes to solve a >>> problem resulting in two new problems; initcalls have levels for >>> ordering, but we still have (unexpressed) dependencies between calls >>> of the same level; brittle!). >>> >>> +Maksim, since that might be relevant info for the BOLT+Kernel work. >>> >>> Ricardo, >>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf >>> mentions that there's a kernel command line param `initcall_debug`. >>> Perhaps that can be used to see if >>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall >>> ordering, resulting in a config that cannot boot? >> >> Here are the links to Lava jobs ran with initcall_debug added to the >> kernel command line. >> >> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926) >> https://lava.collabora.dev/scheduler/job/10417706 >> <https://lava.collabora.dev/scheduler/job/10417706> >> >> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926 >> https://lava.collabora.dev/scheduler/job/10418012 >> <https://lava.collabora.dev/scheduler/job/10418012> > Thanks! > > Yeah, I can see a diff in the initcall ordering as a result of > commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") > > https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 > > Not just different orderings, but some initcalls seem unique to the > before vs. after, which is troubling. (example init_events and > init_fs_sysctls respectively) > > That isn't conclusive evidence that changes to initcall ordering are > to blame, but I suspect confirming that precisely to be very very time > consuming. > > Masahiro, what are your thoughts on reverting 5750121ae738? There are > conflicts in Kbuild and Makefile when reverting 5750121ae738 on > mainline. I'm not sure if you followed the conversation but we are still seeing this regression with the latest kernel builds and would like to know if you plan to revert 5750121ae738? Thanks, Shreeya Patel >> >> Thanks, >> Shreeya Patel >> > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-06-12 10:10 ` Shreeya Patel @ 2023-06-20 4:19 ` Masahiro Yamada 2023-07-10 12:09 ` Linux regression tracking (Thorsten Leemhuis) 0 siblings, 1 reply; 53+ messages in thread From: Masahiro Yamada @ 2023-06-20 4:19 UTC (permalink / raw) To: Shreeya Patel Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > Hi Masahiro, > > > On 24/05/23 02:57, Nick Desaulniers wrote: > > On Tue, May 23, 2023 at 3:27 AM Shreeya Patel > > <shreeya.patel@collabora.com> wrote: > >> Hi Nick and Masahiro, > >> > >> On 23/05/23 01:22, Nick Desaulniers wrote: > >>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: > >>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: > >>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: > >>>>>> It could be; if the link order was changed, it's possible that this > >>>>>> target may be hitting something along the lines of: > >>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static > >>>>>> initialization order fiasco" > >>>>>> > >>>>>> I'm struggling to think of how this appears in C codebases, but I > >>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I > >>>>>> think I was playing with converting Kbuild to use Ninja rather than > >>>>>> Make; the resulting kernel image wouldn't boot because I had modified > >>>>>> the order the object files were linked in. If you were to randomly > >>>>>> shuffle the object files in the kernel, I recall some hazard that may > >>>>>> prevent boot. > >>>>> I thought that was specifically a C++ problem? But then again, the > >>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is > >>>>> significant in some instances [1]: > >>>> Yes, it matters, you can not change it. If you do, systems will break. > >>>> It is the only way we have of properly ordering our init calls within > >>>> the same "level". > >>> Ah, right it was the initcall ordering. Thanks for the reminder. > >>> > >>> (There's a joke in there similar to the use of regexes to solve a > >>> problem resulting in two new problems; initcalls have levels for > >>> ordering, but we still have (unexpressed) dependencies between calls > >>> of the same level; brittle!). > >>> > >>> +Maksim, since that might be relevant info for the BOLT+Kernel work. > >>> > >>> Ricardo, > >>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf > >>> mentions that there's a kernel command line param `initcall_debug`. > >>> Perhaps that can be used to see if > >>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall > >>> ordering, resulting in a config that cannot boot? > >> > >> Here are the links to Lava jobs ran with initcall_debug added to the > >> kernel command line. > >> > >> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926) > >> https://lava.collabora.dev/scheduler/job/10417706 > >> <https://lava.collabora.dev/scheduler/job/10417706> > >> > >> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926 > >> https://lava.collabora.dev/scheduler/job/10418012 > >> <https://lava.collabora.dev/scheduler/job/10418012> > > Thanks! > > > > Yeah, I can see a diff in the initcall ordering as a result of > > commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") > > > > https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 > > > > Not just different orderings, but some initcalls seem unique to the > > before vs. after, which is troubling. (example init_events and > > init_fs_sysctls respectively) > > > > That isn't conclusive evidence that changes to initcall ordering are > > to blame, but I suspect confirming that precisely to be very very time > > consuming. > > > > Masahiro, what are your thoughts on reverting 5750121ae738? There are > > conflicts in Kbuild and Makefile when reverting 5750121ae738 on > > mainline. > > I'm not sure if you followed the conversation but we are still seeing > this regression with the latest kernel builds and would like to know if > you plan to revert 5750121ae738? Reverting 5750121ae738 does not solve the issue because the issue happens even before 5750121ae738. multi_v7_defconfig + debug.config + CONFIG_MODULES=n fails to boot in the same way. The revert would hide the issue on a particular build setup. I submitted a patch to more pin-point the issue. Let's see how it goes. https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t (BTW, the initcall order is unrelated) > > > Thanks, > Shreeya Patel > > >> > >> Thanks, > >> Shreeya Patel > >> > > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-06-20 4:19 ` Masahiro Yamada @ 2023-07-10 12:09 ` Linux regression tracking (Thorsten Leemhuis) 2023-07-11 11:16 ` Shreeya Patel 0 siblings, 1 reply; 53+ messages in thread From: Linux regression tracking (Thorsten Leemhuis) @ 2023-07-10 12:09 UTC (permalink / raw) To: Masahiro Yamada, Shreeya Patel Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, regressions, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone. Shreeya Patel, Masahiro Yamada: what's the status of this? Was any progress made to address this? Or is this maybe (accidentally?) fixed with 6.5-rc1? Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke On 20.06.23 06:19, Masahiro Yamada wrote: > On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> On 24/05/23 02:57, Nick Desaulniers wrote: >>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel >>> <shreeya.patel@collabora.com> wrote: >>>> Hi Nick and Masahiro, >>>> >>>> On 23/05/23 01:22, Nick Desaulniers wrote: >>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: >>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: >>>>>>>> It could be; if the link order was changed, it's possible that this >>>>>>>> target may be hitting something along the lines of: >>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static >>>>>>>> initialization order fiasco" >>>>>>>> >>>>>>>> I'm struggling to think of how this appears in C codebases, but I >>>>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I >>>>>>>> think I was playing with converting Kbuild to use Ninja rather than >>>>>>>> Make; the resulting kernel image wouldn't boot because I had modified >>>>>>>> the order the object files were linked in. If you were to randomly >>>>>>>> shuffle the object files in the kernel, I recall some hazard that may >>>>>>>> prevent boot. >>>>>>> I thought that was specifically a C++ problem? But then again, the >>>>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is >>>>>>> significant in some instances [1]: >>>>>> Yes, it matters, you can not change it. If you do, systems will break. >>>>>> It is the only way we have of properly ordering our init calls within >>>>>> the same "level". >>>>> Ah, right it was the initcall ordering. Thanks for the reminder. >>>>> >>>>> (There's a joke in there similar to the use of regexes to solve a >>>>> problem resulting in two new problems; initcalls have levels for >>>>> ordering, but we still have (unexpressed) dependencies between calls >>>>> of the same level; brittle!). >>>>> >>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work. >>>>> >>>>> Ricardo, >>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf >>>>> mentions that there's a kernel command line param `initcall_debug`. >>>>> Perhaps that can be used to see if >>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall >>>>> ordering, resulting in a config that cannot boot? >>>> >>>> Here are the links to Lava jobs ran with initcall_debug added to the >>>> kernel command line. >>>> >>>> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926) >>>> https://lava.collabora.dev/scheduler/job/10417706 >>>> <https://lava.collabora.dev/scheduler/job/10417706> >>>> >>>> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926 >>>> https://lava.collabora.dev/scheduler/job/10418012 >>>> <https://lava.collabora.dev/scheduler/job/10418012> >>> Thanks! >>> >>> Yeah, I can see a diff in the initcall ordering as a result of >>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") >>> >>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 >>> >>> Not just different orderings, but some initcalls seem unique to the >>> before vs. after, which is troubling. (example init_events and >>> init_fs_sysctls respectively) >>> >>> That isn't conclusive evidence that changes to initcall ordering are >>> to blame, but I suspect confirming that precisely to be very very time >>> consuming. >>> >>> Masahiro, what are your thoughts on reverting 5750121ae738? There are >>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on >>> mainline. >> >> I'm not sure if you followed the conversation but we are still seeing >> this regression with the latest kernel builds and would like to know if >> you plan to revert 5750121ae738? > > > Reverting 5750121ae738 does not solve the issue > because the issue happens even before 5750121ae738. > multi_v7_defconfig + debug.config + CONFIG_MODULES=n > fails to boot in the same way. > > The revert would hide the issue on a particular build setup. > > > I submitted a patch to more pin-point the issue. > Let's see how it goes. > https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t > > > (BTW, the initcall order is unrelated) > > > > > >> >> >> Thanks, >> Shreeya Patel >> >>>> >>>> Thanks, >>>> Shreeya Patel >>>> >>> > > -- > Best Regards > Masahiro Yamada > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-07-10 12:09 ` Linux regression tracking (Thorsten Leemhuis) @ 2023-07-11 11:16 ` Shreeya Patel 2023-08-29 11:28 ` Linux regression tracking (Thorsten Leemhuis) 0 siblings, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-07-11 11:16 UTC (permalink / raw) To: Linux regressions mailing list, Masahiro Yamada Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote: > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting > for once, to make this easily accessible to everyone. > > Shreeya Patel, Masahiro Yamada: what's the status of this? Was any > progress made to address this? Or is this maybe (accidentally?) fixed > with 6.5-rc1? Hi Thorsten, I still see the regression happening so it doesn't seem to be fixed. https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/ Masahiro had submitted a fix for this issue here. https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t But I don't see any movement there. Masahiro, are you planning to send a v2 for it? Thanks, Shreeya Patel > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > #regzbot poke > > On 20.06.23 06:19, Masahiro Yamada wrote: >> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel >> <shreeya.patel@collabora.com> wrote: >>> On 24/05/23 02:57, Nick Desaulniers wrote: >>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel >>>> <shreeya.patel@collabora.com> wrote: >>>>> Hi Nick and Masahiro, >>>>> >>>>> On 23/05/23 01:22, Nick Desaulniers wrote: >>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote: >>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote: >>>>>>>>> It could be; if the link order was changed, it's possible that this >>>>>>>>> target may be hitting something along the lines of: >>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static >>>>>>>>> initialization order fiasco" >>>>>>>>> >>>>>>>>> I'm struggling to think of how this appears in C codebases, but I >>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I >>>>>>>>> think I was playing with converting Kbuild to use Ninja rather than >>>>>>>>> Make; the resulting kernel image wouldn't boot because I had modified >>>>>>>>> the order the object files were linked in. If you were to randomly >>>>>>>>> shuffle the object files in the kernel, I recall some hazard that may >>>>>>>>> prevent boot. >>>>>>>> I thought that was specifically a C++ problem? But then again, the >>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is >>>>>>>> significant in some instances [1]: >>>>>>> Yes, it matters, you can not change it. If you do, systems will break. >>>>>>> It is the only way we have of properly ordering our init calls within >>>>>>> the same "level". >>>>>> Ah, right it was the initcall ordering. Thanks for the reminder. >>>>>> >>>>>> (There's a joke in there similar to the use of regexes to solve a >>>>>> problem resulting in two new problems; initcalls have levels for >>>>>> ordering, but we still have (unexpressed) dependencies between calls >>>>>> of the same level; brittle!). >>>>>> >>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work. >>>>>> >>>>>> Ricardo, >>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf >>>>>> mentions that there's a kernel command line param `initcall_debug`. >>>>>> Perhaps that can be used to see if >>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall >>>>>> ordering, resulting in a config that cannot boot? >>>>> Here are the links to Lava jobs ran with initcall_debug added to the >>>>> kernel command line. >>>>> >>>>> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926) >>>>> https://lava.collabora.dev/scheduler/job/10417706 >>>>> <https://lava.collabora.dev/scheduler/job/10417706> >>>>> >>>>> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926 >>>>> https://lava.collabora.dev/scheduler/job/10418012 >>>>> <https://lava.collabora.dev/scheduler/job/10418012> >>>> Thanks! >>>> >>>> Yeah, I can see a diff in the initcall ordering as a result of >>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") >>>> >>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 >>>> >>>> Not just different orderings, but some initcalls seem unique to the >>>> before vs. after, which is troubling. (example init_events and >>>> init_fs_sysctls respectively) >>>> >>>> That isn't conclusive evidence that changes to initcall ordering are >>>> to blame, but I suspect confirming that precisely to be very very time >>>> consuming. >>>> >>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are >>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on >>>> mainline. >>> I'm not sure if you followed the conversation but we are still seeing >>> this regression with the latest kernel builds and would like to know if >>> you plan to revert 5750121ae738? >> >> Reverting 5750121ae738 does not solve the issue >> because the issue happens even before 5750121ae738. >> multi_v7_defconfig + debug.config + CONFIG_MODULES=n >> fails to boot in the same way. >> >> The revert would hide the issue on a particular build setup. >> >> >> I submitted a patch to more pin-point the issue. >> Let's see how it goes. >> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t >> >> >> (BTW, the initcall order is unrelated) >> >> >> >> >> >>> >>> Thanks, >>> Shreeya Patel >>> >>>>> Thanks, >>>>> Shreeya Patel >>>>> >> -- >> Best Regards >> Masahiro Yamada >> >> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-07-11 11:16 ` Shreeya Patel @ 2023-08-29 11:28 ` Linux regression tracking (Thorsten Leemhuis) 2023-09-11 10:05 ` Thorsten Leemhuis 0 siblings, 1 reply; 53+ messages in thread From: Linux regression tracking (Thorsten Leemhuis) @ 2023-08-29 11:28 UTC (permalink / raw) To: Shreeya Patel, Linux regressions mailing list, Masahiro Yamada Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML On 11.07.23 13:16, Shreeya Patel wrote: > On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote: >> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting >> for once, to make this easily accessible to everyone. >> >> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any >> progress made to address this? Or is this maybe (accidentally?) fixed >> with 6.5-rc1? > > I still see the regression happening so it doesn't seem to be fixed. > https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/ > > Masahiro had submitted a fix for this issue here. > > https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t > > But I don't see any movement there. Masahiro, are you planning to send a > v2 for it? That was weeks ago and we didn't get a answer. :-/ Was this fixed in between? Doesn't look like it from here, but I might be missing something. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke >> On 20.06.23 06:19, Masahiro Yamada wrote: >>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel >>> <shreeya.patel@collabora.com> wrote: >>>> On 24/05/23 02:57, Nick Desaulniers wrote: >>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel >>>>> <shreeya.patel@collabora.com> wrote: >>>>>> Hi Nick and Masahiro, >>>>>> >>>>>> On 23/05/23 01:22, Nick Desaulniers wrote: >>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH >>>>>>> <gregkh@linuxfoundation.org> wrote: >>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers >>>>>>>>> <ndesaulniers@google.com> wrote: >>>>>>>>>> It could be; if the link order was changed, it's possible that >>>>>>>>>> this >>>>>>>>>> target may be hitting something along the lines of: >>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the >>>>>>>>>> "static >>>>>>>>>> initialization order fiasco" >>>>>>>>>> >>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I >>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about >>>>>>>>>> this. I >>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather >>>>>>>>>> than >>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had >>>>>>>>>> modified >>>>>>>>>> the order the object files were linked in. If you were to >>>>>>>>>> randomly >>>>>>>>>> shuffle the object files in the kernel, I recall some hazard >>>>>>>>>> that may >>>>>>>>>> prevent boot. >>>>>>>>> I thought that was specifically a C++ problem? But then again, the >>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in >>>>>>>>> kbuild is >>>>>>>>> significant in some instances [1]: >>>>>>>> Yes, it matters, you can not change it. If you do, systems will >>>>>>>> break. >>>>>>>> It is the only way we have of properly ordering our init calls >>>>>>>> within >>>>>>>> the same "level". >>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder. >>>>>>> >>>>>>> (There's a joke in there similar to the use of regexes to solve a >>>>>>> problem resulting in two new problems; initcalls have levels for >>>>>>> ordering, but we still have (unexpressed) dependencies between calls >>>>>>> of the same level; brittle!). >>>>>>> >>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work. >>>>>>> >>>>>>> Ricardo, >>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf >>>>>>> mentions that there's a kernel command line param `initcall_debug`. >>>>>>> Perhaps that can be used to see if >>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall >>>>>>> ordering, resulting in a config that cannot boot? >>>>>> Here are the links to Lava jobs ran with initcall_debug added to the >>>>>> kernel command line. >>>>>> >>>>>> 1. Where regression happens >>>>>> (5750121ae7382ebac8d47ce6d68012d6cd1d7926) >>>>>> https://lava.collabora.dev/scheduler/job/10417706 >>>>>> <https://lava.collabora.dev/scheduler/job/10417706> >>>>>> >>>>>> 2. With a revert of the commit >>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 >>>>>> https://lava.collabora.dev/scheduler/job/10418012 >>>>>> <https://lava.collabora.dev/scheduler/job/10418012> >>>>> Thanks! >>>>> >>>>> Yeah, I can see a diff in the initcall ordering as a result of >>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") >>>>> >>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 >>>>> >>>>> Not just different orderings, but some initcalls seem unique to the >>>>> before vs. after, which is troubling. (example init_events and >>>>> init_fs_sysctls respectively) >>>>> >>>>> That isn't conclusive evidence that changes to initcall ordering are >>>>> to blame, but I suspect confirming that precisely to be very very time >>>>> consuming. >>>>> >>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are >>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on >>>>> mainline. >>>> I'm not sure if you followed the conversation but we are still seeing >>>> this regression with the latest kernel builds and would like to know if >>>> you plan to revert 5750121ae738? >>> >>> Reverting 5750121ae738 does not solve the issue >>> because the issue happens even before 5750121ae738. >>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n >>> fails to boot in the same way. >>> >>> The revert would hide the issue on a particular build setup. >>> >>> >>> I submitted a patch to more pin-point the issue. >>> Let's see how it goes. >>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t >>> >>> >>> (BTW, the initcall order is unrelated) >>> >>> >>> >>> >>> >>>> >>>> Thanks, >>>> Shreeya Patel >>>> >>>>>> Thanks, >>>>>> Shreeya Patel >>>>>> >>> -- >>> Best Regards >>> Masahiro Yamada >>> >>> > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-08-29 11:28 ` Linux regression tracking (Thorsten Leemhuis) @ 2023-09-11 10:05 ` Thorsten Leemhuis 2023-09-15 9:33 ` Shreeya Patel 0 siblings, 1 reply; 53+ messages in thread From: Thorsten Leemhuis @ 2023-09-11 10:05 UTC (permalink / raw) To: Shreeya Patel, Linux regressions mailing list, Masahiro Yamada Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML On 29.08.23 13:28, Linux regression tracking (Thorsten Leemhuis) wrote: > On 11.07.23 13:16, Shreeya Patel wrote: >> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote: >>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting >>> for once, to make this easily accessible to everyone. >>> >>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any >>> progress made to address this? Or is this maybe (accidentally?) fixed >>> with 6.5-rc1? >> >> I still see the regression happening so it doesn't seem to be fixed. >> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/ >> >> Masahiro had submitted a fix for this issue here. >> >> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t >> >> But I don't see any movement there. Masahiro, are you planning to send a >> v2 for it? > > That was weeks ago and we didn't get a answer. :-/ Was this fixed in > between? Doesn't look like it from here, but I might be missing something. Still no reply. :-/ Shreeya Patel, does the problem still happen with 6.6-rc1 and do you still want to see it fixed? In that case our only option to get things rolling again might be to involve Linus, unless someone in the CC list has a idea to resolve this. Might also be good to know if reverting the culprit fixes the problem. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke >>> On 20.06.23 06:19, Masahiro Yamada wrote: >>>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel >>>> <shreeya.patel@collabora.com> wrote: >>>>> On 24/05/23 02:57, Nick Desaulniers wrote: >>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel >>>>>> <shreeya.patel@collabora.com> wrote: >>>>>>> Hi Nick and Masahiro, >>>>>>> >>>>>>> On 23/05/23 01:22, Nick Desaulniers wrote: >>>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH >>>>>>>> <gregkh@linuxfoundation.org> wrote: >>>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers >>>>>>>>>> <ndesaulniers@google.com> wrote: >>>>>>>>>>> It could be; if the link order was changed, it's possible that >>>>>>>>>>> this >>>>>>>>>>> target may be hitting something along the lines of: >>>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the >>>>>>>>>>> "static >>>>>>>>>>> initialization order fiasco" >>>>>>>>>>> >>>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I >>>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about >>>>>>>>>>> this. I >>>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather >>>>>>>>>>> than >>>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had >>>>>>>>>>> modified >>>>>>>>>>> the order the object files were linked in. If you were to >>>>>>>>>>> randomly >>>>>>>>>>> shuffle the object files in the kernel, I recall some hazard >>>>>>>>>>> that may >>>>>>>>>>> prevent boot. >>>>>>>>>> I thought that was specifically a C++ problem? But then again, the >>>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in >>>>>>>>>> kbuild is >>>>>>>>>> significant in some instances [1]: >>>>>>>>> Yes, it matters, you can not change it. If you do, systems will >>>>>>>>> break. >>>>>>>>> It is the only way we have of properly ordering our init calls >>>>>>>>> within >>>>>>>>> the same "level". >>>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder. >>>>>>>> >>>>>>>> (There's a joke in there similar to the use of regexes to solve a >>>>>>>> problem resulting in two new problems; initcalls have levels for >>>>>>>> ordering, but we still have (unexpressed) dependencies between calls >>>>>>>> of the same level; brittle!). >>>>>>>> >>>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work. >>>>>>>> >>>>>>>> Ricardo, >>>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf >>>>>>>> mentions that there's a kernel command line param `initcall_debug`. >>>>>>>> Perhaps that can be used to see if >>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall >>>>>>>> ordering, resulting in a config that cannot boot? >>>>>>> Here are the links to Lava jobs ran with initcall_debug added to the >>>>>>> kernel command line. >>>>>>> >>>>>>> 1. Where regression happens >>>>>>> (5750121ae7382ebac8d47ce6d68012d6cd1d7926) >>>>>>> https://lava.collabora.dev/scheduler/job/10417706 >>>>>>> <https://lava.collabora.dev/scheduler/job/10417706> >>>>>>> >>>>>>> 2. With a revert of the commit >>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 >>>>>>> https://lava.collabora.dev/scheduler/job/10418012 >>>>>>> <https://lava.collabora.dev/scheduler/job/10418012> >>>>>> Thanks! >>>>>> >>>>>> Yeah, I can see a diff in the initcall ordering as a result of >>>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") >>>>>> >>>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 >>>>>> >>>>>> Not just different orderings, but some initcalls seem unique to the >>>>>> before vs. after, which is troubling. (example init_events and >>>>>> init_fs_sysctls respectively) >>>>>> >>>>>> That isn't conclusive evidence that changes to initcall ordering are >>>>>> to blame, but I suspect confirming that precisely to be very very time >>>>>> consuming. >>>>>> >>>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are >>>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on >>>>>> mainline. >>>>> I'm not sure if you followed the conversation but we are still seeing >>>>> this regression with the latest kernel builds and would like to know if >>>>> you plan to revert 5750121ae738? >>>> >>>> Reverting 5750121ae738 does not solve the issue >>>> because the issue happens even before 5750121ae738. >>>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n >>>> fails to boot in the same way. >>>> >>>> The revert would hide the issue on a particular build setup. >>>> >>>> >>>> I submitted a patch to more pin-point the issue. >>>> Let's see how it goes. >>>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t >>>> >>>> >>>> (BTW, the initcall order is unrelated) >>>> >>>> >>>> >>>> >>>> >>>>> >>>>> Thanks, >>>>> Shreeya Patel >>>>> >>>>>>> Thanks, >>>>>>> Shreeya Patel >>>>>>> >>>> -- >>>> Best Regards >>>> Masahiro Yamada >>>> >>>> >> >> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-09-11 10:05 ` Thorsten Leemhuis @ 2023-09-15 9:33 ` Shreeya Patel 2023-09-30 10:24 ` Masahiro Yamada 0 siblings, 1 reply; 53+ messages in thread From: Shreeya Patel @ 2023-09-15 9:33 UTC (permalink / raw) To: Linux regressions mailing list, Masahiro Yamada Cc: Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML On 11/09/23 15:35, Thorsten Leemhuis wrote: Hi Thorsten, > On 29.08.23 13:28, Linux regression tracking (Thorsten Leemhuis) wrote: >> On 11.07.23 13:16, Shreeya Patel wrote: >>> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote: >>>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting >>>> for once, to make this easily accessible to everyone. >>>> >>>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any >>>> progress made to address this? Or is this maybe (accidentally?) fixed >>>> with 6.5-rc1? >>> I still see the regression happening so it doesn't seem to be fixed. >>> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/ >>> >>> Masahiro had submitted a fix for this issue here. >>> >>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t >>> >>> But I don't see any movement there. Masahiro, are you planning to send a >>> v2 for it? >> That was weeks ago and we didn't get a answer. :-/ Was this fixed in >> between? Doesn't look like it from here, but I might be missing something. > Still no reply. :-/ > > Shreeya Patel, does the problem still happen with 6.6-rc1 and do you > still want to see it fixed? In that case our only option to get things > rolling again might be to involve Linus, unless someone in the CC list > has a idea to resolve this. Might also be good to know if reverting the > culprit fixes the problem. I don't see this issue happening on 6.6-rc1 kernel and it was only last seen in 6.5 kernel. But there was no fix added to Kbuild in the meantime so not sure which commit really fixed this issue. For now we can mark this as resolved and I'll keep an eye on the future test results to see if this pops up again. Thanks, Shreeya Patel #regzbot resolve: Fixed in 6.6-rc1 kernel, fix commit is unknown. > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > #regzbot poke > >>>> On 20.06.23 06:19, Masahiro Yamada wrote: >>>>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel >>>>> <shreeya.patel@collabora.com> wrote: >>>>>> On 24/05/23 02:57, Nick Desaulniers wrote: >>>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel >>>>>>> <shreeya.patel@collabora.com> wrote: >>>>>>>> Hi Nick and Masahiro, >>>>>>>> >>>>>>>> On 23/05/23 01:22, Nick Desaulniers wrote: >>>>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH >>>>>>>>> <gregkh@linuxfoundation.org> wrote: >>>>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote: >>>>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers >>>>>>>>>>> <ndesaulniers@google.com> wrote: >>>>>>>>>>>> It could be; if the link order was changed, it's possible that >>>>>>>>>>>> this >>>>>>>>>>>> target may be hitting something along the lines of: >>>>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the >>>>>>>>>>>> "static >>>>>>>>>>>> initialization order fiasco" >>>>>>>>>>>> >>>>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I >>>>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about >>>>>>>>>>>> this. I >>>>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather >>>>>>>>>>>> than >>>>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had >>>>>>>>>>>> modified >>>>>>>>>>>> the order the object files were linked in. If you were to >>>>>>>>>>>> randomly >>>>>>>>>>>> shuffle the object files in the kernel, I recall some hazard >>>>>>>>>>>> that may >>>>>>>>>>>> prevent boot. >>>>>>>>>>> I thought that was specifically a C++ problem? But then again, the >>>>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in >>>>>>>>>>> kbuild is >>>>>>>>>>> significant in some instances [1]: >>>>>>>>>> Yes, it matters, you can not change it. If you do, systems will >>>>>>>>>> break. >>>>>>>>>> It is the only way we have of properly ordering our init calls >>>>>>>>>> within >>>>>>>>>> the same "level". >>>>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder. >>>>>>>>> >>>>>>>>> (There's a joke in there similar to the use of regexes to solve a >>>>>>>>> problem resulting in two new problems; initcalls have levels for >>>>>>>>> ordering, but we still have (unexpressed) dependencies between calls >>>>>>>>> of the same level; brittle!). >>>>>>>>> >>>>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work. >>>>>>>>> >>>>>>>>> Ricardo, >>>>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf >>>>>>>>> mentions that there's a kernel command line param `initcall_debug`. >>>>>>>>> Perhaps that can be used to see if >>>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall >>>>>>>>> ordering, resulting in a config that cannot boot? >>>>>>>> Here are the links to Lava jobs ran with initcall_debug added to the >>>>>>>> kernel command line. >>>>>>>> >>>>>>>> 1. Where regression happens >>>>>>>> (5750121ae7382ebac8d47ce6d68012d6cd1d7926) >>>>>>>> https://lava.collabora.dev/scheduler/job/10417706 >>>>>>>> <https://lava.collabora.dev/scheduler/job/10417706> >>>>>>>> >>>>>>>> 2. With a revert of the commit >>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 >>>>>>>> https://lava.collabora.dev/scheduler/job/10418012 >>>>>>>> <https://lava.collabora.dev/scheduler/job/10418012> >>>>>>> Thanks! >>>>>>> >>>>>>> Yeah, I can see a diff in the initcall ordering as a result of >>>>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild") >>>>>>> >>>>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4 >>>>>>> >>>>>>> Not just different orderings, but some initcalls seem unique to the >>>>>>> before vs. after, which is troubling. (example init_events and >>>>>>> init_fs_sysctls respectively) >>>>>>> >>>>>>> That isn't conclusive evidence that changes to initcall ordering are >>>>>>> to blame, but I suspect confirming that precisely to be very very time >>>>>>> consuming. >>>>>>> >>>>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are >>>>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on >>>>>>> mainline. >>>>>> I'm not sure if you followed the conversation but we are still seeing >>>>>> this regression with the latest kernel builds and would like to know if >>>>>> you plan to revert 5750121ae738? >>>>> Reverting 5750121ae738 does not solve the issue >>>>> because the issue happens even before 5750121ae738. >>>>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n >>>>> fails to boot in the same way. >>>>> >>>>> The revert would hide the issue on a particular build setup. >>>>> >>>>> >>>>> I submitted a patch to more pin-point the issue. >>>>> Let's see how it goes. >>>>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t >>>>> >>>>> >>>>> (BTW, the initcall order is unrelated) >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>> Thanks, >>>>>> Shreeya Patel >>>>>> >>>>>>>> Thanks, >>>>>>>> Shreeya Patel >>>>>>>> >>>>> -- >>>>> Best Regards >>>>> Masahiro Yamada >>>>> >>>>> >>> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros 2023-09-15 9:33 ` Shreeya Patel @ 2023-09-30 10:24 ` Masahiro Yamada 0 siblings, 0 replies; 53+ messages in thread From: Masahiro Yamada @ 2023-09-30 10:24 UTC (permalink / raw) To: Shreeya Patel Cc: Linux regressions mailing list, Greg KH, Maksim Panchenko, Ricardo Cañuelo, Michal Marek, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Nathan Chancellor, gustavo.padovan, Guillaume Charles Tucker, denys.f, Nick Desaulniers, kernelci, Collabora Kernel ML On Fri, Sep 15, 2023 at 6:33 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > On 11/09/23 15:35, Thorsten Leemhuis wrote: > > Hi Thorsten, > > > On 29.08.23 13:28, Linux regression tracking (Thorsten Leemhuis) wrote: > >> On 11.07.23 13:16, Shreeya Patel wrote: > >>> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote: > >>>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting > >>>> for once, to make this easily accessible to everyone. > >>>> > >>>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any > >>>> progress made to address this? Or is this maybe (accidentally?) fixed > >>>> with 6.5-rc1? > >>> I still see the regression happening so it doesn't seem to be fixed. > >>> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/ > >>> > >>> Masahiro had submitted a fix for this issue here. > >>> > >>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t > >>> > >>> But I don't see any movement there. Masahiro, are you planning to send a > >>> v2 for it? > >> That was weeks ago and we didn't get a answer. :-/ Was this fixed in > >> between? Doesn't look like it from here, but I might be missing something. > > Still no reply. :-/ > > > > Shreeya Patel, does the problem still happen with 6.6-rc1 and do you > > still want to see it fixed? In that case our only option to get things > > rolling again might be to involve Linus, unless someone in the CC list > > has a idea to resolve this. Might also be good to know if reverting the > > culprit fixes the problem. > > > I don't see this issue happening on 6.6-rc1 kernel and it was only last > seen in 6.5 kernel. > But there was no fix added to Kbuild in the meantime so not sure which > commit really fixed this issue. > > For now we can mark this as resolved and I'll keep an eye on the future > test results to see if this pops up again. > > > Thanks, > Shreeya Patel > > #regzbot resolve: Fixed in 6.6-rc1 kernel, fix commit is unknown. Not fixed in 6.6-rc1. I submitted a patch to shoot the root cause. https://lore.kernel.org/all/20230925110023.1796789-1-masahiroy@kernel.org/ Now it is in Russell's patch tracker. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT 2022-09-09 21:02 ` Masahiro Yamada 2022-09-19 17:08 ` [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers @ 2022-09-19 17:30 ` Nick Desaulniers 2022-09-24 1:34 ` Masahiro Yamada 2022-09-19 17:45 ` [PATCH v4] Makefile.debug: re-enable debug info for .S files Nick Desaulniers 2 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-19 17:30 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers, Andi Kleen, Dmitrii Bundin, Fangrui Song, Nathan Chancellor Dmitrii, Fangrui, and Mashahiro note: Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2. Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g specified in order for -gsplit-dwarf to work at all. -gsplit-dwarf has been mutually exclusive with -g since support for CONFIG_DEBUG_INFO_SPLIT was introduced in commit 866ced950bcd ("kbuild: Support split debug info v4") I don't think it ever needed to be. Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/ Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/ Link: https://reviews.llvm.org/D80391 Cc: Andi Kleen <ak@linux.intel.com> Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> Reported-by: Fangrui Song <maskray@google.com> Reported-by: Masahiro Yamada <masahiroy@kernel.org> Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v3 -> v4: * Split into its own patch; we don't have any out of line assembler files using .debug_* or .cfi_* directives that would produce meaningful debug info in .dwo files. scripts/Makefile.debug | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug index 9f39b0130551..26d6a9d97a20 100644 --- a/scripts/Makefile.debug +++ b/scripts/Makefile.debug @@ -1,9 +1,7 @@ -DEBUG_CFLAGS := +DEBUG_CFLAGS := -g ifdef CONFIG_DEBUG_INFO_SPLIT DEBUG_CFLAGS += -gsplit-dwarf -else -DEBUG_CFLAGS += -g endif ifndef CONFIG_AS_IS_LLVM -- 2.37.3.968.ga6b4b080e4-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT 2022-09-19 17:30 ` [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers @ 2022-09-24 1:34 ` Masahiro Yamada 0 siblings, 0 replies; 53+ messages in thread From: Masahiro Yamada @ 2022-09-24 1:34 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, Andi Kleen, Dmitrii Bundin, Fangrui Song, Nathan Chancellor On Tue, Sep 20, 2022 at 2:30 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > Dmitrii, Fangrui, and Mashahiro note: > > Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2. > > Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g > specified in order for -gsplit-dwarf to work at all. > > -gsplit-dwarf has been mutually exclusive with -g since support for > CONFIG_DEBUG_INFO_SPLIT was introduced in > commit 866ced950bcd ("kbuild: Support split debug info v4") > I don't think it ever needed to be. > > Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/ > Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/ > Link: https://reviews.llvm.org/D80391 > Cc: Andi Kleen <ak@linux.intel.com> > Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> > Reported-by: Fangrui Song <maskray@google.com> > Reported-by: Masahiro Yamada <masahiroy@kernel.org> > Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- Applied to linux-kbuild/fixes. Thanks. > Changes v3 -> v4: > * Split into its own patch; we don't have any out of line assembler > files using .debug_* or .cfi_* directives that would produce > meaningful debug info in .dwo files. > > scripts/Makefile.debug | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 9f39b0130551..26d6a9d97a20 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,9 +1,7 @@ > -DEBUG_CFLAGS := > +DEBUG_CFLAGS := -g > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > -else > -DEBUG_CFLAGS += -g > endif > > ifndef CONFIG_AS_IS_LLVM > -- > 2.37.3.968.ga6b4b080e4-goog > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v4] Makefile.debug: re-enable debug info for .S files 2022-09-09 21:02 ` Masahiro Yamada 2022-09-19 17:08 ` [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers 2022-09-19 17:30 ` [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers @ 2022-09-19 17:45 ` Nick Desaulniers 2022-09-24 2:11 ` Masahiro Yamada 2 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-19 17:45 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Greg Thelen, Alexey Alexandrov, Nick Desaulniers, Nathan Chancellor Alexey reported that the fraction of unknown filename instances in kallsyms grew from ~0.3% to ~10% recently; Bill and Greg tracked it down to assembler defined symbols, which regressed as a result of: commit b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") In that commit, I allude to restoring debug info for assembler defined symbols in a follow up patch, but it seems I forgot to do so in commit a66049e2cf0e ("Kbuild: make DWARF version a choice") Link: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=31bf18645d98b4d3d7357353be840e320649a67d Fixes: b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") Reported-by: Alexey Alexandrov <aalexand@google.com> Reported-by: Bill Wendling <morbo@google.com> Reported-by: Greg Thelen <gthelen@google.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Suggested-by: Masahiro Yamada <masahiroy@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v3 -> v4: * Break out into its own patch. * Move addition of debug-flags-y to DEBUG_CFLAGS and KBUILD_AFLAGS up grouping the -gdwarf-* handling together. lib/Kconfig.debug | 4 +++- scripts/Makefile.debug | 18 +++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index bcbe60d6c80c..d3e5f36bb01e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -264,8 +264,10 @@ config DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT config DEBUG_INFO_DWARF4 bool "Generate DWARF Version 4 debuginfo" select DEBUG_INFO + depends on !CC_IS_CLANG || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502))) help - Generate DWARF v4 debug info. This requires gcc 4.5+ and gdb 7.0+. + Generate DWARF v4 debug info. This requires gcc 4.5+, binutils 2.35.2 + if using clang without clang's integrated assembler, and gdb 7.0+. If you have consumers of DWARF debug info that are not ready for newer revisions of DWARF, you may wish to choose this or have your diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug index 26d6a9d97a20..d6aecd78b942 100644 --- a/scripts/Makefile.debug +++ b/scripts/Makefile.debug @@ -4,15 +4,15 @@ ifdef CONFIG_DEBUG_INFO_SPLIT DEBUG_CFLAGS += -gsplit-dwarf endif -ifndef CONFIG_AS_IS_LLVM -KBUILD_AFLAGS += -Wa,-gdwarf-2 -endif - -ifndef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT -dwarf-version-$(CONFIG_DEBUG_INFO_DWARF4) := 4 -dwarf-version-$(CONFIG_DEBUG_INFO_DWARF5) := 5 -DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y) +debug-flags-$(CONFIG_DEBUG_INFO_DWARF4) += -gdwarf-4 +debug-flags-$(CONFIG_DEBUG_INFO_DWARF5) += -gdwarf-5 +ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_AS_IS_GNU),yy) +# Clang does not pass -g or -gdwarf-* option down to GAS. +# Add -Wa, prefix to explicitly specify the flags. +KBUILD_AFLAGS += $(addprefix -Wa$(comma), $(debug-flags-y)) endif +DEBUG_CFLAGS += $(debug-flags-y) +KBUILD_AFLAGS += $(debug-flags-y) ifdef CONFIG_DEBUG_INFO_REDUCED DEBUG_CFLAGS += -fno-var-tracking @@ -27,5 +27,5 @@ KBUILD_AFLAGS += -gz=zlib KBUILD_LDFLAGS += --compress-debug-sections=zlib endif -KBUILD_CFLAGS += $(DEBUG_CFLAGS) +KBUILD_CFLAGS += $(DEBUG_CFLAGS) export DEBUG_CFLAGS -- 2.37.3.968.ga6b4b080e4-goog ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.debug: re-enable debug info for .S files 2022-09-19 17:45 ` [PATCH v4] Makefile.debug: re-enable debug info for .S files Nick Desaulniers @ 2022-09-24 2:11 ` Masahiro Yamada 2022-09-24 2:20 ` Nick Desaulniers 0 siblings, 1 reply; 53+ messages in thread From: Masahiro Yamada @ 2022-09-24 2:11 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Greg Thelen, Alexey Alexandrov, Nathan Chancellor On Tue, Sep 20, 2022 at 2:45 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > Alexey reported that the fraction of unknown filename instances in > kallsyms grew from ~0.3% to ~10% recently; Bill and Greg tracked it down > to assembler defined symbols, which regressed as a result of: > > commit b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") > > In that commit, I allude to restoring debug info for assembler defined > symbols in a follow up patch, but it seems I forgot to do so in > > commit a66049e2cf0e ("Kbuild: make DWARF version a choice") > > Link: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=31bf18645d98b4d3d7357353be840e320649a67d > Fixes: b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1") > Reported-by: Alexey Alexandrov <aalexand@google.com> > Reported-by: Bill Wendling <morbo@google.com> > Reported-by: Greg Thelen <gthelen@google.com> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > Suggested-by: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes v3 -> v4: > * Break out into its own patch. > * Move addition of debug-flags-y to DEBUG_CFLAGS and KBUILD_AFLAGS up > grouping the -gdwarf-* handling together. > > lib/Kconfig.debug | 4 +++- > scripts/Makefile.debug | 18 +++++++++--------- > 2 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index bcbe60d6c80c..d3e5f36bb01e 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -264,8 +264,10 @@ config DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT > config DEBUG_INFO_DWARF4 > bool "Generate DWARF Version 4 debuginfo" > select DEBUG_INFO > + depends on !CC_IS_CLANG || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502))) > help > - Generate DWARF v4 debug info. This requires gcc 4.5+ and gdb 7.0+. > + Generate DWARF v4 debug info. This requires gcc 4.5+, binutils 2.35.2 > + if using clang without clang's integrated assembler, and gdb 7.0+. > > If you have consumers of DWARF debug info that are not ready for > newer revisions of DWARF, you may wish to choose this or have your > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 26d6a9d97a20..d6aecd78b942 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -4,15 +4,15 @@ ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > endif > > -ifndef CONFIG_AS_IS_LLVM > -KBUILD_AFLAGS += -Wa,-gdwarf-2 > -endif > - > -ifndef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT > -dwarf-version-$(CONFIG_DEBUG_INFO_DWARF4) := 4 > -dwarf-version-$(CONFIG_DEBUG_INFO_DWARF5) := 5 > -DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y) > +debug-flags-$(CONFIG_DEBUG_INFO_DWARF4) += -gdwarf-4 > +debug-flags-$(CONFIG_DEBUG_INFO_DWARF5) += -gdwarf-5 > +ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_AS_IS_GNU),yy) > +# Clang does not pass -g or -gdwarf-* option down to GAS. This patch still misses the debug info for *.S files for the combination of LLVM_IAS=0 and CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y because, as the comment says, Clang does not pass -g down to GAS. With "[v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT" and this one applied, $ grep CONFIG_DEBUG_INFO_DWARF .config CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y # CONFIG_DEBUG_INFO_DWARF4 is not set # CONFIG_DEBUG_INFO_DWARF5 is not set $ make LLVM=1 LLVM_IAS=0 arch/x86/kernel/irqflags.o SYNC include/config/auto.conf.cmd SYSHDR arch/x86/include/generated/asm/unistd_32_ia32.h SYSHDR arch/x86/include/generated/asm/unistd_64_x32.h SYSTBL arch/x86/include/generated/asm/syscalls_64.h HOSTCC arch/x86/tools/relocs_32.o [snip] AS arch/x86/kernel/irqflags.o $ objdump -h arch/x86/kernel/irqflags.o | grep debug $ I think the following fix-up is needed on top. diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug index d6aecd78b942..8cf1cb22dd93 100644 --- a/scripts/Makefile.debug +++ b/scripts/Makefile.debug @@ -1,4 +1,5 @@ -DEBUG_CFLAGS := -g +DEBUG_CFLAGS := +debug-flags-y := -g ifdef CONFIG_DEBUG_INFO_SPLIT DEBUG_CFLAGS += -gsplit-dwarf Then, I can see the debug sections. $ make LLVM=1 LLVM_IAS=0 arch/x86/kernel/irqflags.o CALL scripts/checksyscalls.sh DESCEND objtool AS arch/x86/kernel/irqflags.o $ objdump -h arch/x86/kernel/irqflags.o | grep debug 6 .debug_line 00000050 0000000000000000 0000000000000000 0000008f 2**0 7 .debug_info 0000002e 0000000000000000 0000000000000000 000000f8 2**0 8 .debug_abbrev 00000014 0000000000000000 0000000000000000 000001d0 2**0 9 .debug_aranges 00000030 0000000000000000 0000000000000000 000001f0 2**4 10 .debug_str 0000004d 0000000000000000 0000000000000000 00000250 2**0 If you agree, I can locally fix it up as such. -- Best Regards Masahiro Yamada ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.debug: re-enable debug info for .S files 2022-09-24 2:11 ` Masahiro Yamada @ 2022-09-24 2:20 ` Nick Desaulniers 2022-09-24 6:43 ` Masahiro Yamada 0 siblings, 1 reply; 53+ messages in thread From: Nick Desaulniers @ 2022-09-24 2:20 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Greg Thelen, Alexey Alexandrov, Nathan Chancellor On Fri, Sep 23, 2022 at 7:12 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > This patch still misses the debug info for *.S files > for the combination of LLVM_IAS=0 and > CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y > because, as the comment says, Clang does not pass -g down to GAS. > > > With "[v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT" > and this one applied, > > > > $ grep CONFIG_DEBUG_INFO_DWARF .config > CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y > # CONFIG_DEBUG_INFO_DWARF4 is not set > # CONFIG_DEBUG_INFO_DWARF5 is not set > $ make LLVM=1 LLVM_IAS=0 arch/x86/kernel/irqflags.o > SYNC include/config/auto.conf.cmd > SYSHDR arch/x86/include/generated/asm/unistd_32_ia32.h > SYSHDR arch/x86/include/generated/asm/unistd_64_x32.h > SYSTBL arch/x86/include/generated/asm/syscalls_64.h > HOSTCC arch/x86/tools/relocs_32.o > [snip] > AS arch/x86/kernel/irqflags.o > $ objdump -h arch/x86/kernel/irqflags.o | grep debug > $ > > > > > > > > > I think the following fix-up is needed on top. > > > > > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index d6aecd78b942..8cf1cb22dd93 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,4 +1,5 @@ > -DEBUG_CFLAGS := -g > +DEBUG_CFLAGS := > +debug-flags-y := -g > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > > > > > Then, I can see the debug sections. > > > > $ make LLVM=1 LLVM_IAS=0 arch/x86/kernel/irqflags.o > CALL scripts/checksyscalls.sh > DESCEND objtool > AS arch/x86/kernel/irqflags.o > $ objdump -h arch/x86/kernel/irqflags.o | grep debug > 6 .debug_line 00000050 0000000000000000 0000000000000000 0000008f 2**0 > 7 .debug_info 0000002e 0000000000000000 0000000000000000 000000f8 2**0 > 8 .debug_abbrev 00000014 0000000000000000 0000000000000000 000001d0 2**0 > 9 .debug_aranges 00000030 0000000000000000 0000000000000000 000001f0 2**4 > 10 .debug_str 0000004d 0000000000000000 0000000000000000 00000250 2**0 > > > > > > If you agree, I can locally fix it up as such. Ah, sorry I missed testing that combination. Thanks for your thoroughness. Yes please apply that diff on top. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4] Makefile.debug: re-enable debug info for .S files 2022-09-24 2:20 ` Nick Desaulniers @ 2022-09-24 6:43 ` Masahiro Yamada 0 siblings, 0 replies; 53+ messages in thread From: Masahiro Yamada @ 2022-09-24 6:43 UTC (permalink / raw) To: Nick Desaulniers Cc: Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, clang-built-linux, Bill Wendling, Greg Thelen, Alexey Alexandrov, Nathan Chancellor On Sat, Sep 24, 2022 at 11:20 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Fri, Sep 23, 2022 at 7:12 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > This patch still misses the debug info for *.S files > > for the combination of LLVM_IAS=0 and > > CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y > > because, as the comment says, Clang does not pass -g down to GAS. > > > > > > With "[v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT" > > and this one applied, > > > > > > > > $ grep CONFIG_DEBUG_INFO_DWARF .config > > CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y > > # CONFIG_DEBUG_INFO_DWARF4 is not set > > # CONFIG_DEBUG_INFO_DWARF5 is not set > > $ make LLVM=1 LLVM_IAS=0 arch/x86/kernel/irqflags.o > > SYNC include/config/auto.conf.cmd > > SYSHDR arch/x86/include/generated/asm/unistd_32_ia32.h > > SYSHDR arch/x86/include/generated/asm/unistd_64_x32.h > > SYSTBL arch/x86/include/generated/asm/syscalls_64.h > > HOSTCC arch/x86/tools/relocs_32.o > > [snip] > > AS arch/x86/kernel/irqflags.o > > $ objdump -h arch/x86/kernel/irqflags.o | grep debug > > $ > > > > > > > > > > > > > > > > > > I think the following fix-up is needed on top. > > > > > > > > > > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > > index d6aecd78b942..8cf1cb22dd93 100644 > > --- a/scripts/Makefile.debug > > +++ b/scripts/Makefile.debug > > @@ -1,4 +1,5 @@ > > -DEBUG_CFLAGS := -g > > +DEBUG_CFLAGS := > > +debug-flags-y := -g > > > > ifdef CONFIG_DEBUG_INFO_SPLIT > > DEBUG_CFLAGS += -gsplit-dwarf > > > > > > > > > > Then, I can see the debug sections. > > > > > > > > $ make LLVM=1 LLVM_IAS=0 arch/x86/kernel/irqflags.o > > CALL scripts/checksyscalls.sh > > DESCEND objtool > > AS arch/x86/kernel/irqflags.o > > $ objdump -h arch/x86/kernel/irqflags.o | grep debug > > 6 .debug_line 00000050 0000000000000000 0000000000000000 0000008f 2**0 > > 7 .debug_info 0000002e 0000000000000000 0000000000000000 000000f8 2**0 > > 8 .debug_abbrev 00000014 0000000000000000 0000000000000000 000001d0 2**0 > > 9 .debug_aranges 00000030 0000000000000000 0000000000000000 000001f0 2**4 > > 10 .debug_str 0000004d 0000000000000000 0000000000000000 00000250 2**0 > > > > > > > > > > > > If you agree, I can locally fix it up as such. > > Ah, sorry I missed testing that combination. Thanks for your > thoroughness. Yes please apply that diff on top. > -- > Thanks, > ~Nick Desaulniers Applied to linux-kbuild with the fixup. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2023-09-30 10:24 UTC | newest] Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-07 4:59 [PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions Nick Desaulniers 2022-09-09 21:05 ` Masahiro Yamada 2022-09-07 4:59 ` [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option Nick Desaulniers 2022-09-09 21:06 ` Masahiro Yamada 2022-09-07 4:59 ` [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers 2022-09-09 21:59 ` Masahiro Yamada 2022-09-07 4:59 ` [PATCH v3 4/5] Makefile.debug: re-enable debug info for .S files Nick Desaulniers 2022-09-07 4:59 ` [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers 2022-09-09 20:56 ` Masahiro Yamada 2022-09-09 21:02 ` Masahiro Yamada 2022-09-19 17:08 ` [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros Nick Desaulniers 2022-09-23 19:44 ` Masahiro Yamada 2022-09-24 14:28 ` Masahiro Yamada 2022-09-25 1:22 ` Masahiro Yamada 2022-09-26 21:41 ` Nick Desaulniers 2023-04-27 11:53 ` Shreeya Patel 2023-04-28 7:41 ` Thorsten Leemhuis 2023-05-02 9:48 ` Shreeya Patel 2023-05-02 11:58 ` Linux regression tracking (Thorsten Leemhuis) 2023-04-28 17:27 ` Nick Desaulniers 2023-05-03 21:02 ` Shreeya Patel 2023-05-03 21:15 ` Nick Desaulniers 2023-05-03 22:33 ` Shreeya Patel 2023-05-15 23:01 ` Nick Desaulniers 2023-05-17 8:34 ` Shreeya Patel 2023-05-17 15:39 ` Ricardo Cañuelo 2023-05-17 16:27 ` Nick Desaulniers 2023-05-18 14:23 ` Ricardo Cañuelo 2023-05-18 21:12 ` Nick Desaulniers 2023-05-19 8:35 ` Ricardo Cañuelo 2023-05-19 15:57 ` Nick Desaulniers 2023-05-22 10:09 ` Ricardo Cañuelo 2023-05-22 16:52 ` Greg KH 2023-05-22 19:52 ` Nick Desaulniers 2023-05-22 20:01 ` Greg KH 2023-05-22 20:16 ` Nick Desaulniers 2023-05-23 10:27 ` Shreeya Patel 2023-05-23 21:27 ` Nick Desaulniers 2023-06-12 10:10 ` Shreeya Patel 2023-06-20 4:19 ` Masahiro Yamada 2023-07-10 12:09 ` Linux regression tracking (Thorsten Leemhuis) 2023-07-11 11:16 ` Shreeya Patel 2023-08-29 11:28 ` Linux regression tracking (Thorsten Leemhuis) 2023-09-11 10:05 ` Thorsten Leemhuis 2023-09-15 9:33 ` Shreeya Patel 2023-09-30 10:24 ` Masahiro Yamada 2022-09-19 17:30 ` [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT Nick Desaulniers 2022-09-24 1:34 ` Masahiro Yamada 2022-09-19 17:45 ` [PATCH v4] Makefile.debug: re-enable debug info for .S files Nick Desaulniers 2022-09-24 2:11 ` Masahiro Yamada 2022-09-24 2:20 ` Nick Desaulniers 2022-09-24 6:43 ` Masahiro Yamada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).