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/[email protected]/
v1: https://lore.kernel.org/llvm/[email protected]/
*** 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
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 <[email protected]>
Reported-by: Bill Wendling <[email protected]>
Reported-by: Greg Thelen <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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
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/[email protected]/
Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
Link: https://reviews.llvm.org/D80391
Cc: Andi Kleen <[email protected]>
Reported-by: Dmitrii Bundin <[email protected]>
Reported-by: Fangrui Song <[email protected]>
Reported-by: Masahiro Yamada <[email protected]>
Suggested-by: Dmitrii Bundin <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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
On Sat, Sep 10, 2022 at 5:56 AM Masahiro Yamada <[email protected]> wrote:
>
> On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <[email protected]> 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/[email protected]/
> > Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
> > Link: https://reviews.llvm.org/D80391
> > Cc: Andi Kleen <[email protected]>
> > Reported-by: Dmitrii Bundin <[email protected]>
> > Reported-by: Fangrui Song <[email protected]>
> > Reported-by: Masahiro Yamada <[email protected]>
> > Suggested-by: Dmitrii Bundin <[email protected]>
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > 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
On Wed, Sep 7, 2022 at 1:59 PM Nick Desaulniers <[email protected]> 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/[email protected]/
> Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
> Link: https://reviews.llvm.org/D80391
> Cc: Andi Kleen <[email protected]>
> Reported-by: Dmitrii Bundin <[email protected]>
> Reported-by: Fangrui Song <[email protected]>
> Reported-by: Masahiro Yamada <[email protected]>
> Suggested-by: Dmitrii Bundin <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> 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
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 <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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/[email protected]
-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
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/[email protected]/
Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
Link: https://reviews.llvm.org/D80391
Cc: Andi Kleen <[email protected]>
Reported-by: Dmitrii Bundin <[email protected]>
Reported-by: Fangrui Song <[email protected]>
Reported-by: Masahiro Yamada <[email protected]>
Suggested-by: Dmitrii Bundin <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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
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 <[email protected]>
Reported-by: Bill Wendling <[email protected]>
Reported-by: Greg Thelen <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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
On Tue, Sep 20, 2022 at 2:08 AM Nick Desaulniers
<[email protected]> 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 <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> 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
On Tue, Sep 20, 2022 at 2:30 AM Nick Desaulniers
<[email protected]> 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/[email protected]/
> Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
> Link: https://reviews.llvm.org/D80391
> Cc: Andi Kleen <[email protected]>
> Reported-by: Dmitrii Bundin <[email protected]>
> Reported-by: Fangrui Song <[email protected]>
> Reported-by: Masahiro Yamada <[email protected]>
> Suggested-by: Dmitrii Bundin <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
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
On Tue, Sep 20, 2022 at 2:45 AM Nick Desaulniers
<[email protected]> 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 <[email protected]>
> Reported-by: Bill Wendling <[email protected]>
> Reported-by: Greg Thelen <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> 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
On Fri, Sep 23, 2022 at 7:12 PM Masahiro Yamada <[email protected]> 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
On Sat, Sep 24, 2022 at 11:20 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Fri, Sep 23, 2022 at 7:12 PM Masahiro Yamada <[email protected]> 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
On Sat, Sep 24, 2022 at 4:44 AM Masahiro Yamada <[email protected]> wrote:
>
> On Tue, Sep 20, 2022 at 2:08 AM Nick Desaulniers
> <[email protected]> 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 <[email protected]>
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > 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
On Sat, Sep 24, 2022 at 11:28 PM Masahiro Yamada <[email protected]> wrote:
>
> On Sat, Sep 24, 2022 at 4:44 AM Masahiro Yamada <[email protected]> wrote:
> >
> > On Tue, Sep 20, 2022 at 2:08 AM Nick Desaulniers
> > <[email protected]> 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 <[email protected]>
> > > Reviewed-by: Nathan Chancellor <[email protected]>
> > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > ---
> > > 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
On Sat, Sep 24, 2022 at 6:23 PM Masahiro Yamada <[email protected]> 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
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 <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
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/[email protected]
> -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
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 <[email protected]>
>> Reviewed-by: Nathan Chancellor <[email protected]>
>> Signed-off-by: Nick Desaulniers <[email protected]>
>
> 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/[email protected]
>> -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
>
>
On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
<[email protected]> 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 <[email protected]>
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
>
> 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/[email protected]
> > -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
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 <[email protected]>
>>> Reviewed-by: Nathan Chancellor <[email protected]>
>>> Signed-off-by: Nick Desaulniers <[email protected]>
>> 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/[email protected]
>>> -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
>>
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 <[email protected]>
>>>> Reviewed-by: Nathan Chancellor <[email protected]>
>>>> Signed-off-by: Nick Desaulniers <[email protected]>
>>> 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
Hi Nick,
On 28/04/23 22:57, Nick Desaulniers wrote:
> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
> <[email protected]> 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 <[email protected]>
>>> Reviewed-by: Nathan Chancellor <[email protected]>
>>> Signed-off-by: Nick Desaulniers <[email protected]>
>> 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/[email protected]
>>> -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
>
>
On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
<[email protected]> wrote:
>
> Hi Nick,
>
> On 28/04/23 22:57, Nick Desaulniers wrote:
> > On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
> > <[email protected]> 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 <[email protected]>
> >>> Reviewed-by: Nathan Chancellor <[email protected]>
> >>> Signed-off-by: Nick Desaulniers <[email protected]>
> >> 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/[email protected]
> >>> -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
On 04/05/23 02:45, Nick Desaulniers wrote:
> On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
> <[email protected]> wrote:
>> Hi Nick,
>>
>> On 28/04/23 22:57, Nick Desaulniers wrote:
>>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
>>> <[email protected]> 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 <[email protected]>
>>>>> Reviewed-by: Nathan Chancellor <[email protected]>
>>>>> Signed-off-by: Nick Desaulniers <[email protected]>
>>>> 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/[email protected]
>>>>> -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
>>>
>
>
On Wed, May 3, 2023 at 3:33 PM Shreeya Patel
<[email protected]> wrote:
>
>
> On 04/05/23 02:45, Nick Desaulniers wrote:
> > On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
> > <[email protected]> wrote:
> >> Hi Nick,
> >>
> >> On 28/04/23 22:57, Nick Desaulniers wrote:
> >>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
> >>> <[email protected]> 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 <[email protected]>
> >>>>> Reviewed-by: Nathan Chancellor <[email protected]>
> >>>>> Signed-off-by: Nick Desaulniers <[email protected]>
> >>>> 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/[email protected]
> >>>>> -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
Hi Nick,
On 16/05/23 04:31, Nick Desaulniers wrote:
> On Wed, May 3, 2023 at 3:33 PM Shreeya Patel
> <[email protected]> wrote:
>>
>> On 04/05/23 02:45, Nick Desaulniers wrote:
>>> On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
>>> <[email protected]> wrote:
>>>> Hi Nick,
>>>>
>>>> On 28/04/23 22:57, Nick Desaulniers wrote:
>>>>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
>>>>> <[email protected]> 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 <[email protected]>
>>>>>>> Reviewed-by: Nathan Chancellor <[email protected]>
>>>>>>> Signed-off-by: Nick Desaulniers <[email protected]>
>>>>>> 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/[email protected]
>>>>>>> -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
>>>
>
>
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
On Wed, May 17, 2023 at 8:39 AM Ricardo Cañuelo
<[email protected]> 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
Hi Nick,
On mié, may 17 2023 at 09:27:41, Nick Desaulniers <[email protected]> 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
On Thu, May 18, 2023 at 7:23 AM Ricardo Cañuelo
<[email protected]> wrote:
>
>
> Hi Nick,
>
> On mié, may 17 2023 at 09:27:41, Nick Desaulniers <[email protected]> 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
On jue, may 18 2023 at 14:12:30, Nick Desaulniers <[email protected]> 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
On Fri, May 19, 2023 at 1:35 AM Ricardo Cañuelo
<[email protected]> wrote:
>
> On jue, may 18 2023 at 14:12:30, Nick Desaulniers <[email protected]> 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
On vie, may 19 2023 at 08:57:24, Nick Desaulniers <[email protected]> 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
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 <[email protected]> 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
On Mon, May 22, 2023 at 9:52 AM Greg KH <[email protected]> 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 <[email protected]> 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
On Mon, May 22, 2023 at 1:01 PM Greg KH <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
On Mon, May 22, 2023 at 12:52:13PM -0700, Nick Desaulniers wrote:
> On Mon, May 22, 2023 at 9:52 AM Greg KH <[email protected]> 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 <[email protected]> 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
Hi Nick and Masahiro,
On 23/05/23 01:22, Nick Desaulniers wrote:
> On Mon, May 22, 2023 at 9:52 AM Greg KH <[email protected]> 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 <[email protected]> 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
On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
<[email protected]> 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 <[email protected]> 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 <[email protected]> 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
Hi Masahiro,
On 24/05/23 02:57, Nick Desaulniers wrote:
> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
> <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
>>
>
On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
<[email protected]> wrote:
>
> Hi Masahiro,
>
>
> On 24/05/23 02:57, Nick Desaulniers wrote:
> > On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
> > <[email protected]> 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 <[email protected]> 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 <[email protected]> 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%[email protected]/T/#t
(BTW, the initcall order is unrelated)
>
>
> Thanks,
> Shreeya Patel
>
> >>
> >> Thanks,
> >> Shreeya Patel
> >>
> >
--
Best Regards
Masahiro Yamada
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
> <[email protected]> wrote:
>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>> <[email protected]> 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 <[email protected]> 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 <[email protected]> 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%[email protected]/T/#t
>
>
> (BTW, the initcall order is unrelated)
>
>
>
>
>
>>
>>
>> Thanks,
>> Shreeya Patel
>>
>>>>
>>>> Thanks,
>>>> Shreeya Patel
>>>>
>>>
>
> --
> Best Regards
> Masahiro Yamada
>
>
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%[email protected]/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
>> <[email protected]> wrote:
>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>> <[email protected]> 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 <[email protected]> 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 <[email protected]> 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%[email protected]/T/#t
>>
>>
>> (BTW, the initcall order is unrelated)
>>
>>
>>
>>
>>
>>>
>>> Thanks,
>>> Shreeya Patel
>>>
>>>>> Thanks,
>>>>> Shreeya Patel
>>>>>
>> --
>> Best Regards
>> Masahiro Yamada
>>
>>
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%[email protected]/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
>>> <[email protected]> wrote:
>>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>>> <[email protected]> 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
>>>>>>> <[email protected]> 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
>>>>>>>>> <[email protected]> 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%[email protected]/T/#t
>>>
>>>
>>> (BTW, the initcall order is unrelated)
>>>
>>>
>>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> Shreeya Patel
>>>>
>>>>>> Thanks,
>>>>>> Shreeya Patel
>>>>>>
>>> --
>>> Best Regards
>>> Masahiro Yamada
>>>
>>>
>
>
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%[email protected]/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
>>>>> <[email protected]> wrote:
>>>>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>>>>> <[email protected]> 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
>>>>>>>>> <[email protected]> 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
>>>>>>>>>>> <[email protected]> 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%[email protected]/T/#t
>>>>>
>>>>>
>>>>> (BTW, the initcall order is unrelated)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Shreeya Patel
>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Shreeya Patel
>>>>>>>>
>>>>> --
>>>>> Best Regards
>>>>> Masahiro Yamada
>>>>>
>>>>>
>>>
On Fri, Sep 15, 2023 at 6:33 PM Shreeya Patel
<[email protected]> 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%[email protected]/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/[email protected]/
Now it is in Russell's patch tracker.
--
Best Regards
Masahiro Yamada