[PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT

[PATCH v3 0/5] fix debug info for asm and DEBUG_INFO_SPLIT

[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

[PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions

[Nick Desaulniers]

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

Re: [PATCH v3 1/5] x86/boot/compressed: prefer cc-option for CFLAGS additions

[Masahiro Yamada]

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

[PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option

[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

Re: [PATCH v3 2/5] Makefile.compiler: Use KBUILD_AFLAGS for as-option

[Masahiro Yamada]

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

[PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[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

Re: [PATCH v3 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Masahiro Yamada]

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

[PATCH v3 4/5] Makefile.debug: re-enable debug info for .S files

[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

[PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Nick Desaulniers]

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

Re: [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Masahiro Yamada]

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

Re: [PATCH v3 5/5] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Masahiro Yamada]

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

[PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[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 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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Masahiro Yamada]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Masahiro Yamada]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Masahiro Yamada]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Thorsten Leemhuis]

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
> 
> 

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Linux regression tracking (Thorsten Leemhuis)]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>
>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>>>
>
>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>>>
>
>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Ricardo Cañuelo]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Ricardo Cañuelo]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Ricardo Cañuelo]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Ricardo Cañuelo]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Greg KH]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Greg KH]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>>
>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Masahiro Yamada]

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

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Linux regression tracking (Thorsten Leemhuis)]

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
> 
> 

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>>
>>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Linux regression tracking (Thorsten Leemhuis)]

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
>>>
>>>
> 
> 

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Thorsten Leemhuis]

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
>>>>
>>>>
>>
>>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Shreeya Patel]

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
>>>>>
>>>>>
>>>

Re: [PATCH v4] Makefile.compiler: replace cc-ifversion with compiler-specific macros

[Masahiro Yamada]

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

[PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Masahiro Yamada]

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

[PATCH v4] Makefile.debug: re-enable debug info for .S files

[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 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

Re: [PATCH v4] Makefile.debug: re-enable debug info for .S files

[Masahiro Yamada]

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

Re: [PATCH v4] Makefile.debug: re-enable debug info for .S files

[Nick Desaulniers]

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

Re: [PATCH v4] Makefile.debug: re-enable debug info for .S files

[Masahiro Yamada]

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