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

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

[Shreeya Patel]

Hi Thorsten,

On 28/04/23 13:11, Thorsten Leemhuis wrote:
> Hi Shreeya!
>
> On 27.04.23 13:53, Shreeya Patel wrote:
>> On 19/09/22 22:38, Nick Desaulniers wrote:
>>> cc-ifversion is GCC specific. Replace it with compiler specific
>>> variants. Update the users of cc-ifversion to use these new macros.
>>>
>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
>>> Link:
>>> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
>>> Suggested-by: Bill Wendling <morbo@google.com>
>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>> KernelCI found this patch causes a regression in the
>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
>> see the bisection report for more details [2].
>>
>> Let me know if you have any questions.
>>
>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
>> [2] https://groups.io/g/kernelci-results/message/40804> [...]
>> #regzbot introduced: 88b61e3bff93
> How much of this text is auto generated? I ask for two reasons:

None of this text is auto generated yet but we plan to do it soon once
we think the format of the reporting email is good enough for people to 
understand
and look into it. Which is why your comments are really helpful here.


>
> * You made regzbot track this, which is great, but didn't specify a
> title. That is fine in general, if the subject round about says what the
> regression is about. But in this case it doesn't; hence it would be
> great if you in the future could specify one through "#regzbot title:"
> or adjust the mail's subject (I guess the former is what developers will
> prefer).


Noted. If I think the title is not very explanatory then I'll change it 
to reflect the problem in future.


> * I'm not a developer that has to look into bugs like this, but from my
> experience I expect a lot more developers are likely willing to look
> into reports like this if you specified what the actual problem is --
> ideally with the relevant error messages.
>
> Side note: I for one still am unsure what this is actually about after
> looking at the page you provided as [1] and clicking on a few links
> there (which took me a few minutes, which I guess not everyone is
> willing to invest). I noticed two kernel warning in the log (one from
> "arch/arm/kernel/insn.c:48 __arm_gen_branch+0x88/0xa4", the other
> "kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4") and a complaint
> UBSAN (btw: those newlines in the logs make things harder to read, at
> least for me).

For this particular regression, it is possible that the following kernel 
warnings/errors could be the reason of failure.


   572 20:30:46.811318  <3>[    0.417001][    T1] UBSAN: 
array-index-out-of-bounds in ../arch/arm/mach-sunxi/mc_smp.c:811:29
   574 20:30:46.812203  <3>[    0.417963][    T1] index 2 is out of 
range for type 'sunxi_mc_smp_data [2]'


and

   426 20:30:44.726421  <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 
at kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
   428 20:30:44.727284  <4>[    0.000000][    T0] Modules linked in:
   430 20:30:44.728487  <4>[    0.000000][    T0] CPU: 0 PID: 0 Comm: 
swapper Tainted: G        W          6.3.0 #1
.........

I understand that it might be more helpful to atleast put little more 
information about what is causing the regression here.
I'll provide some more details in future for it to be easy for 
developers to look into it.

> To check if those were old or new problems, I tired to go back in the
> history and on page 9 found an entry for the last succeeding test. But
> clicking on the logs got me a 404 :-/
>
> Then I looked at the logs on [1] again and in the html view "Boot
> result: FAIL". Is that the actual problem?

Unfortunately, we do have some broken links in the current KernelCI 
dashboard but the KernelCI team
is working on a new API and database interface which will fix these issues.
It might not be worth to spend time to fix issues on the current 
dashboard since the process for getting the archived
logs is not very straightforward.

What I can do from my side is to attach logs of the working kernel if I 
can get them through LAVA.
But one thing to note is that even LAVA stores limited logs and we won't 
be able to provide them always like in this case
since the regression has been happening from a long time.

Thanks for your input though, we will work on it and get a better format 
ready for reporting the regressions.


Thanks,
Shreeya Patel

>
> Ciao, Thorsten
>
> P.S.: let me update the regzbot title while at it:
>
> #regzbot title: kernelci: qemu_arm-virt-gicv3-uefi stopped booting
>
>>> ---
>>> Changes v3 -> v4:
>>> * Split into its own patch again from series, as per Masahiro.
>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update
>>>     clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro.
>>>
>>>    Documentation/kbuild/makefiles.rst          | 29 ++++++++++++---------
>>>    Makefile                                    |  6 ++---
>>>    drivers/gpu/drm/amd/display/dc/dml/Makefile |  2 +-
>>>    scripts/Makefile.compiler                   | 10 ++++---
>>>    scripts/Makefile.extrawarn                  |  4 +--
>>>    5 files changed, 29 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/Documentation/kbuild/makefiles.rst
>>> b/Documentation/kbuild/makefiles.rst
>>> index 11a296e52d68..ee7e3ea1fbe1 100644
>>> --- a/Documentation/kbuild/makefiles.rst
>>> +++ b/Documentation/kbuild/makefiles.rst
>>> @@ -682,22 +682,27 @@ more details, with real examples.
>>>        In the above example, -Wno-unused-but-set-variable will be added to
>>>        KBUILD_CFLAGS only if gcc really accepts it.
>>>    -    cc-ifversion
>>> -    cc-ifversion tests the version of $(CC) and equals the fourth
>>> parameter
>>> -    if version expression is true, or the fifth (if given) if the
>>> version
>>> -    expression is false.
>>> +    gcc-min-version
>>> +    gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is
>>> greater than
>>> +    or equal to the provided value and evaluates to y if so.
>>>          Example::
>>>    -        #fs/reiserfs/Makefile
>>> -        ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
>>> +        cflags-$(call gcc-min-version, 70100) := -foo
>>>    -    In this example, ccflags-y will be assigned the value -O1 if the
>>> -    $(CC) version is less than 4.2.
>>> -    cc-ifversion takes all the shell operators:
>>> -    -eq, -ne, -lt, -le, -gt, and -ge
>>> -    The third parameter may be a text as in this example, but it may
>>> also
>>> -    be an expanded variable or a macro.
>>> +    In this example, cflags-y will be assigned the value -foo if
>>> $(CC) is gcc and
>>> +    $(CONFIG_GCC_VERSION) is >= 7.1.
>>> +
>>> +    clang-min-version
>>> +    clang-min-version tests if the value of $(CONFIG_CLANG_VERSION)
>>> is greater
>>> +    than or equal to the provided value and evaluates to y if so.
>>> +
>>> +    Example::
>>> +
>>> +        cflags-$(call clang-min-version, 110000) := -foo
>>> +
>>> +    In this example, cflags-y will be assigned the value -foo if
>>> $(CC) is clang
>>> +    and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
>>>          cc-cross-prefix
>>>        cc-cross-prefix is used to check if there exists a $(CC) in path
>>> with
>>> diff --git a/Makefile b/Makefile
>>> index 298f69060f10..411c8480b37e 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y)
>>>      KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
>>>    KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>      ifdef CONFIG_CC_IS_CLANG
>>>    KBUILD_CPPFLAGS += -Qunused-arguments
>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC
>>>    KBUILD_CFLAGS += -Wno-maybe-uninitialized
>>>    endif
>>>    -ifdef CONFIG_CC_IS_GCC
>>>    # The allocators already balk at large sizes, so silence the compiler
>>>    # warnings for bounds checks involving those possible values. While
>>>    # -Wno-alloc-size-larger-than would normally be used here, earlier
>>> versions
>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC
>>>    # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
>>>    # choice, we must perform a versioned check to disable this warning.
>>>    # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901,
>>> -Wno-alloc-size-larger-than)
>>> -endif
>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) +=
>>> -Wno-alloc-size-larger-than
>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>      # disable invalid "can't wrap" optimizations for signed / pointers
>>>    KBUILD_CFLAGS    += -fno-strict-overflow
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> index cb81ed2fbd53..d70838edba80 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
>>>    endif
>>>      ifdef CONFIG_CC_IS_GCC
>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y)
>>> +ifneq ($(call gcc-min-version, 70100),y)
>>>    IS_OLD_GCC = 1
>>>    endif
>>>    endif
>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
>>> index 94d0d40cddb3..9d18fb91890e 100644
>>> --- a/scripts/Makefile.compiler
>>> +++ b/scripts/Makefile.compiler
>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\
>>>    cc-disable-warning = $(call try-run,\
>>>        $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip
>>> $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
>>>    -# cc-ifversion
>>> -# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] &&
>>> echo $(3) || echo $(4))
>>> +# gcc-min-version
>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo
>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
>>> +
>>> +# clang-min-version
>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo
>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] &&
>>> echo y)
>>>      # ld-option
>>>    # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
>>> index 6ae482158bc4..5769c1939d40 100644
>>> --- a/scripts/Makefile.extrawarn
>>> +++ b/scripts/Makefile.extrawarn
>>> @@ -48,7 +48,7 @@ else
>>>    ifdef CONFIG_CC_IS_CLANG
>>>    KBUILD_CFLAGS += -Wno-initializer-overrides
>>>    # Clang before clang-16 would warn on default argument promotions.
>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
>>> +ifneq ($(call clang-min-version, 160000),y)
>>>    # Disable -Wformat
>>>    KBUILD_CFLAGS += -Wno-format
>>>    # Then re-enable flags that were part of the -Wformat group that aren't
>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format
>>>    KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
>>>    KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
>>>    # Requires clang-12+.
>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
>>> +ifeq ($(call clang-min-version, 120000),y)
>>>    KBUILD_CFLAGS += -Wformat-insufficient-args
>>>    endif
>>>    endif
>>

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

[Linux regression tracking (Thorsten Leemhuis)]

On 02.05.23 11:48, Shreeya Patel wrote:
> On 28/04/23 13:11, Thorsten Leemhuis wrote:
>> On 27.04.23 13:53, Shreeya Patel wrote:
>>> On 19/09/22 22:38, Nick Desaulniers wrote:
>>>> cc-ifversion is GCC specific. Replace it with compiler specific
>>>> variants. Update the users of cc-ifversion to use these new macros.
>>>>
>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
>>>> Link:
>>>> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
>>>> Suggested-by: Bill Wendling <morbo@google.com>
>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>>> KernelCI found this patch causes a regression in the
>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
>>> see the bisection report for more details [2].
>>>
>>> Let me know if you have any questions.
>>>
>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
>>> [2] https://groups.io/g/kernelci-results/message/40804> [...]
>>> #regzbot introduced: 88b61e3bff93
>> How much of this text is auto generated? I ask for two reasons:
> 
> None of this text is auto generated yet but we plan to do it soon once
> we think the format of the reporting email is good enough for people to
> understand
> and look into it. Which is why your comments are really helpful here.

Thx, glad to hear that. FWIW and YMMV, but I'm not sure if fully
automating things is a good idea, as a bad report or two might be enough
to make some developers start ignoring kernelci reports; a quick human
review with small adjustments might help prevent that, as it's hard to
get that ship turned around later (that's why regzbot up to this day
doesn't send any mails automatically).

>> * You made regzbot track this, which is great, but didn't specify a
>> title. That is fine in general, if the subject round about says what the
>> regression is about. But in this case it doesn't; hence it would be
>> great if you in the future could specify one through "#regzbot title:"
>> or adjust the mail's subject (I guess the former is what developers will
>> prefer).
> 
> Noted. If I think the title is not very explanatory then I'll change it
> to reflect the problem in future.

Many thx!

> [...]
> 
> I understand that it might be more helpful to atleast put little more
> information about what is causing the regression here.
> I'll provide some more details in future for it to be easy for
> developers to look into it.

Yeah, especially a obvious "what's the actual problem (in 10 words or
less)" afaics would have been really good here.

>> To check if those were old or new problems, I tired to go back in the
>> history and on page 9 found an entry for the last succeeding test. But
>> clicking on the logs got me a 404 :-/
>>
>> Then I looked at the logs on [1] again and in the html view "Boot
>> result: FAIL". Is that the actual problem?
> 
> Unfortunately, we do have some broken links in the current KernelCI
> dashboard [...]

Happens :-D

> What I can do from my side is to attach logs of the working kernel if I
> can get them through LAVA.
> But one thing to note is that even LAVA stores limited logs and we won't
> be able to provide them always like in this case
> since the regression has been happening from a long time.

Before going down that route I'd work out with Nick why things work for
him, maybe all that isn't needed.

> Thanks for your input though, we will work on it and get a better format
> ready for reporting the regressions.

Thx for your work!

Ciao, Thorsten

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

[Shreeya Patel]

Hi Nick,

On 28/04/23 22:57, Nick Desaulniers wrote:
> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
> <shreeya.patel@collabora.com> wrote:
>> Hi Nick,
>>
>> On 19/09/22 22:38, Nick Desaulniers wrote:
>>> cc-ifversion is GCC specific. Replace it with compiler specific
>>> variants. Update the users of cc-ifversion to use these new macros.
>>>
>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
>>> Suggested-by: Bill Wendling <morbo@google.com>
>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>> KernelCI found this patch causes a regression in the
>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
>> see the bisection report for more details [2].
>>
>> Let me know if you have any questions.
>>
>>
>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
> Hi Shreeya,
> Thanks for the report.
>
> When I click the above link, then click `multi_v7_defconfig+debug` to
> get the config necessary to reproduce, I get an HTTP 404.
> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config
>
> Same for zImage
> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage

Apologies for the broken links. We will try to fix the important ones if 
we can but in the meantime,
following is the correct link that you can refer.

config :-
https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config

zImage :-
https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage

If you notice, they are present under the kernel directory and same way 
you can find links for other kernel
builds if you'd like to check them out.

> If I click on the log
> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt
> It looks like the machine powered up, then powered off. Is the test
> actually failing?

I recommend checking the html logs from the kernelci dashboard.
Also, FYI baseline.login test failure means that the device failed to 
boot which I think is causing by the issues that you pointed out.

<3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
../arch/arm/mach-sunxi/mc_smp.c:811:29

And potentially another issue with ftrace

<4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4


Let me know if you need more information from my side to reproduce this 
on your end.


Thanks,
Shreeya Patel
> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc
> (Debian 10.2.1-6) in QEMU just fine.  So I'm going to need some more
> information to help reproduce what specifically is failing.
>
> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023
>
> ---
>
> It does look like UBSAN is flagging an array OOB:
>
> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
> ../arch/arm/mach-sunxi/mc_smp.c:811:29
>
> And potentially another issue with ftrace
>
> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
>
>
>
>> [2] https://groups.io/g/kernelci-results/message/40804
>>
>>
>> Thanks,
>> Shreeya Patel
>>
>> #regzbot introduced: 88b61e3bff93
>>
>>> ---
>>> Changes v3 -> v4:
>>> * Split into its own patch again from series, as per Masahiro.
>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update
>>>     clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro.
>>>
>>>    Documentation/kbuild/makefiles.rst          | 29 ++++++++++++---------
>>>    Makefile                                    |  6 ++---
>>>    drivers/gpu/drm/amd/display/dc/dml/Makefile |  2 +-
>>>    scripts/Makefile.compiler                   | 10 ++++---
>>>    scripts/Makefile.extrawarn                  |  4 +--
>>>    5 files changed, 29 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
>>> index 11a296e52d68..ee7e3ea1fbe1 100644
>>> --- a/Documentation/kbuild/makefiles.rst
>>> +++ b/Documentation/kbuild/makefiles.rst
>>> @@ -682,22 +682,27 @@ more details, with real examples.
>>>        In the above example, -Wno-unused-but-set-variable will be added to
>>>        KBUILD_CFLAGS only if gcc really accepts it.
>>>
>>> -    cc-ifversion
>>> -     cc-ifversion tests the version of $(CC) and equals the fourth parameter
>>> -     if version expression is true, or the fifth (if given) if the version
>>> -     expression is false.
>>> +    gcc-min-version
>>> +     gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than
>>> +     or equal to the provided value and evaluates to y if so.
>>>
>>>        Example::
>>>
>>> -             #fs/reiserfs/Makefile
>>> -             ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
>>> +             cflags-$(call gcc-min-version, 70100) := -foo
>>>
>>> -     In this example, ccflags-y will be assigned the value -O1 if the
>>> -     $(CC) version is less than 4.2.
>>> -     cc-ifversion takes all the shell operators:
>>> -     -eq, -ne, -lt, -le, -gt, and -ge
>>> -     The third parameter may be a text as in this example, but it may also
>>> -     be an expanded variable or a macro.
>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and
>>> +     $(CONFIG_GCC_VERSION) is >= 7.1.
>>> +
>>> +    clang-min-version
>>> +     clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater
>>> +     than or equal to the provided value and evaluates to y if so.
>>> +
>>> +     Example::
>>> +
>>> +             cflags-$(call clang-min-version, 110000) := -foo
>>> +
>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is clang
>>> +     and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
>>>
>>>        cc-cross-prefix
>>>        cc-cross-prefix is used to check if there exists a $(CC) in path with
>>> diff --git a/Makefile b/Makefile
>>> index 298f69060f10..411c8480b37e 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y)
>>>
>>>    KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
>>>    KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>
>>>    ifdef CONFIG_CC_IS_CLANG
>>>    KBUILD_CPPFLAGS += -Qunused-arguments
>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC
>>>    KBUILD_CFLAGS += -Wno-maybe-uninitialized
>>>    endif
>>>
>>> -ifdef CONFIG_CC_IS_GCC
>>>    # The allocators already balk at large sizes, so silence the compiler
>>>    # warnings for bounds checks involving those possible values. While
>>>    # -Wno-alloc-size-larger-than would normally be used here, earlier versions
>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC
>>>    # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
>>>    # choice, we must perform a versioned check to disable this warning.
>>>    # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
>>> -endif
>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than
>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>
>>>    # disable invalid "can't wrap" optimizations for signed / pointers
>>>    KBUILD_CFLAGS       += -fno-strict-overflow
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> index cb81ed2fbd53..d70838edba80 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
>>>    endif
>>>
>>>    ifdef CONFIG_CC_IS_GCC
>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y)
>>> +ifneq ($(call gcc-min-version, 70100),y)
>>>    IS_OLD_GCC = 1
>>>    endif
>>>    endif
>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
>>> index 94d0d40cddb3..9d18fb91890e 100644
>>> --- a/scripts/Makefile.compiler
>>> +++ b/scripts/Makefile.compiler
>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\
>>>    cc-disable-warning = $(call try-run,\
>>>        $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
>>>
>>> -# cc-ifversion
>>> -# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
>>> +# gcc-min-version
>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo
>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
>>> +
>>> +# clang-min-version
>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo
>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y)
>>>
>>>    # ld-option
>>>    # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
>>> index 6ae482158bc4..5769c1939d40 100644
>>> --- a/scripts/Makefile.extrawarn
>>> +++ b/scripts/Makefile.extrawarn
>>> @@ -48,7 +48,7 @@ else
>>>    ifdef CONFIG_CC_IS_CLANG
>>>    KBUILD_CFLAGS += -Wno-initializer-overrides
>>>    # Clang before clang-16 would warn on default argument promotions.
>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
>>> +ifneq ($(call clang-min-version, 160000),y)
>>>    # Disable -Wformat
>>>    KBUILD_CFLAGS += -Wno-format
>>>    # Then re-enable flags that were part of the -Wformat group that aren't
>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format
>>>    KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
>>>    KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
>>>    # Requires clang-12+.
>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
>>> +ifeq ($(call clang-min-version, 120000),y)
>>>    KBUILD_CFLAGS += -Wformat-insufficient-args
>>>    endif
>>>    endif
>
>

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

[Nick Desaulniers]

On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
<shreeya.patel@collabora.com> wrote:
>
> Hi Nick,
>
> On 28/04/23 22:57, Nick Desaulniers wrote:
> > On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
> > <shreeya.patel@collabora.com> wrote:
> >> Hi Nick,
> >>
> >> On 19/09/22 22:38, Nick Desaulniers wrote:
> >>> cc-ifversion is GCC specific. Replace it with compiler specific
> >>> variants. Update the users of cc-ifversion to use these new macros.
> >>>
> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
> >>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
> >>> Suggested-by: Bill Wendling <morbo@google.com>
> >>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >> KernelCI found this patch causes a regression in the
> >> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
> >> see the bisection report for more details [2].
> >>
> >> Let me know if you have any questions.
> >>
> >>
> >> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
> > Hi Shreeya,
> > Thanks for the report.
> >
> > When I click the above link, then click `multi_v7_defconfig+debug` to
> > get the config necessary to reproduce, I get an HTTP 404.
> > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config
> >
> > Same for zImage
> > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage
>
> Apologies for the broken links. We will try to fix the important ones if
> we can but in the meantime,
> following is the correct link that you can refer.
>
> config :-
> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
>
> zImage :-
> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage
>
> If you notice, they are present under the kernel directory and same way
> you can find links for other kernel
> builds if you'd like to check them out.
>
> > If I click on the log
> > https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt
> > It looks like the machine powered up, then powered off. Is the test
> > actually failing?
>
> I recommend checking the html logs from the kernelci dashboard.
> Also, FYI baseline.login test failure means that the device failed to
> boot which I think is causing by the issues that you pointed out.
>
> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
> ../arch/arm/mach-sunxi/mc_smp.c:811:29
>
> And potentially another issue with ftrace
>
> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
>
>
> Let me know if you need more information from my side to reproduce this
> on your end.
>
>
> Thanks,
> Shreeya Patel

Hi Shreeya,
I may need your help to reproduce the failure.

$ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
-O .config
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s
<launch qemu>
/ # mount -t proc /proc
/ # cat /proc/version
Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
(Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
2.35.2) #2 SMP Wed May  3 21:10:27 UTC 2023

I was able to boot the resulting kernel to a command line.  Perhaps
there's something about the userspace image that tickles this? Can you
supply the rootfs that's used in testing?

> > I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc
> > (Debian 10.2.1-6) in QEMU just fine.  So I'm going to need some more
> > information to help reproduce what specifically is failing.
> >
> > Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
> > (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
> > 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023
> >
> > ---
> >
> > It does look like UBSAN is flagging an array OOB:
> >
> > <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
> > ../arch/arm/mach-sunxi/mc_smp.c:811:29
> >
> > And potentially another issue with ftrace
> >
> > <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
> > kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
> >
> >
> >
> >> [2] https://groups.io/g/kernelci-results/message/40804
> >>
> >>
> >> Thanks,
> >> Shreeya Patel
> >>
> >> #regzbot introduced: 88b61e3bff93
> >>
> >>> ---
> >>> Changes v3 -> v4:
> >>> * Split into its own patch again from series, as per Masahiro.
> >>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update
> >>>     clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro.
> >>>
> >>>    Documentation/kbuild/makefiles.rst          | 29 ++++++++++++---------
> >>>    Makefile                                    |  6 ++---
> >>>    drivers/gpu/drm/amd/display/dc/dml/Makefile |  2 +-
> >>>    scripts/Makefile.compiler                   | 10 ++++---
> >>>    scripts/Makefile.extrawarn                  |  4 +--
> >>>    5 files changed, 29 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
> >>> index 11a296e52d68..ee7e3ea1fbe1 100644
> >>> --- a/Documentation/kbuild/makefiles.rst
> >>> +++ b/Documentation/kbuild/makefiles.rst
> >>> @@ -682,22 +682,27 @@ more details, with real examples.
> >>>        In the above example, -Wno-unused-but-set-variable will be added to
> >>>        KBUILD_CFLAGS only if gcc really accepts it.
> >>>
> >>> -    cc-ifversion
> >>> -     cc-ifversion tests the version of $(CC) and equals the fourth parameter
> >>> -     if version expression is true, or the fifth (if given) if the version
> >>> -     expression is false.
> >>> +    gcc-min-version
> >>> +     gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than
> >>> +     or equal to the provided value and evaluates to y if so.
> >>>
> >>>        Example::
> >>>
> >>> -             #fs/reiserfs/Makefile
> >>> -             ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
> >>> +             cflags-$(call gcc-min-version, 70100) := -foo
> >>>
> >>> -     In this example, ccflags-y will be assigned the value -O1 if the
> >>> -     $(CC) version is less than 4.2.
> >>> -     cc-ifversion takes all the shell operators:
> >>> -     -eq, -ne, -lt, -le, -gt, and -ge
> >>> -     The third parameter may be a text as in this example, but it may also
> >>> -     be an expanded variable or a macro.
> >>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and
> >>> +     $(CONFIG_GCC_VERSION) is >= 7.1.
> >>> +
> >>> +    clang-min-version
> >>> +     clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater
> >>> +     than or equal to the provided value and evaluates to y if so.
> >>> +
> >>> +     Example::
> >>> +
> >>> +             cflags-$(call clang-min-version, 110000) := -foo
> >>> +
> >>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is clang
> >>> +     and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
> >>>
> >>>        cc-cross-prefix
> >>>        cc-cross-prefix is used to check if there exists a $(CC) in path with
> >>> diff --git a/Makefile b/Makefile
> >>> index 298f69060f10..411c8480b37e 100644
> >>> --- a/Makefile
> >>> +++ b/Makefile
> >>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y)
> >>>
> >>>    KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
> >>>    KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
> >>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> >>>
> >>>    ifdef CONFIG_CC_IS_CLANG
> >>>    KBUILD_CPPFLAGS += -Qunused-arguments
> >>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC
> >>>    KBUILD_CFLAGS += -Wno-maybe-uninitialized
> >>>    endif
> >>>
> >>> -ifdef CONFIG_CC_IS_GCC
> >>>    # The allocators already balk at large sizes, so silence the compiler
> >>>    # warnings for bounds checks involving those possible values. While
> >>>    # -Wno-alloc-size-larger-than would normally be used here, earlier versions
> >>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC
> >>>    # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
> >>>    # choice, we must perform a versioned check to disable this warning.
> >>>    # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
> >>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
> >>> -endif
> >>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than
> >>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> >>>
> >>>    # disable invalid "can't wrap" optimizations for signed / pointers
> >>>    KBUILD_CFLAGS       += -fno-strict-overflow
> >>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> >>> index cb81ed2fbd53..d70838edba80 100644
> >>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> >>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> >>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
> >>>    endif
> >>>
> >>>    ifdef CONFIG_CC_IS_GCC
> >>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y)
> >>> +ifneq ($(call gcc-min-version, 70100),y)
> >>>    IS_OLD_GCC = 1
> >>>    endif
> >>>    endif
> >>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> >>> index 94d0d40cddb3..9d18fb91890e 100644
> >>> --- a/scripts/Makefile.compiler
> >>> +++ b/scripts/Makefile.compiler
> >>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\
> >>>    cc-disable-warning = $(call try-run,\
> >>>        $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
> >>>
> >>> -# cc-ifversion
> >>> -# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> >>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
> >>> +# gcc-min-version
> >>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo
> >>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
> >>> +
> >>> +# clang-min-version
> >>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo
> >>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y)
> >>>
> >>>    # ld-option
> >>>    # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
> >>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> >>> index 6ae482158bc4..5769c1939d40 100644
> >>> --- a/scripts/Makefile.extrawarn
> >>> +++ b/scripts/Makefile.extrawarn
> >>> @@ -48,7 +48,7 @@ else
> >>>    ifdef CONFIG_CC_IS_CLANG
> >>>    KBUILD_CFLAGS += -Wno-initializer-overrides
> >>>    # Clang before clang-16 would warn on default argument promotions.
> >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
> >>> +ifneq ($(call clang-min-version, 160000),y)
> >>>    # Disable -Wformat
> >>>    KBUILD_CFLAGS += -Wno-format
> >>>    # Then re-enable flags that were part of the -Wformat group that aren't
> >>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format
> >>>    KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
> >>>    KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
> >>>    # Requires clang-12+.
> >>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
> >>> +ifeq ($(call clang-min-version, 120000),y)
> >>>    KBUILD_CFLAGS += -Wformat-insufficient-args
> >>>    endif
> >>>    endif
> >
> >



-- 
Thanks,
~Nick Desaulniers

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

[Shreeya Patel]

On 04/05/23 02:45, Nick Desaulniers wrote:
> On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
> <shreeya.patel@collabora.com> wrote:
>> Hi Nick,
>>
>> On 28/04/23 22:57, Nick Desaulniers wrote:
>>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
>>> <shreeya.patel@collabora.com> wrote:
>>>> Hi Nick,
>>>>
>>>> On 19/09/22 22:38, Nick Desaulniers wrote:
>>>>> cc-ifversion is GCC specific. Replace it with compiler specific
>>>>> variants. Update the users of cc-ifversion to use these new macros.
>>>>>
>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
>>>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
>>>>> Suggested-by: Bill Wendling <morbo@google.com>
>>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>>>> KernelCI found this patch causes a regression in the
>>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
>>>> see the bisection report for more details [2].
>>>>
>>>> Let me know if you have any questions.
>>>>
>>>>
>>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
>>> Hi Shreeya,
>>> Thanks for the report.
>>>
>>> When I click the above link, then click `multi_v7_defconfig+debug` to
>>> get the config necessary to reproduce, I get an HTTP 404.
>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config
>>>
>>> Same for zImage
>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage
>> Apologies for the broken links. We will try to fix the important ones if
>> we can but in the meantime,
>> following is the correct link that you can refer.
>>
>> config :-
>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
>>
>> zImage :-
>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage
>>
>> If you notice, they are present under the kernel directory and same way
>> you can find links for other kernel
>> builds if you'd like to check them out.
>>
>>> If I click on the log
>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt
>>> It looks like the machine powered up, then powered off. Is the test
>>> actually failing?
>> I recommend checking the html logs from the kernelci dashboard.
>> Also, FYI baseline.login test failure means that the device failed to
>> boot which I think is causing by the issues that you pointed out.
>>
>> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
>> ../arch/arm/mach-sunxi/mc_smp.c:811:29
>>
>> And potentially another issue with ftrace
>>
>> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
>>
>>
>> Let me know if you need more information from my side to reproduce this
>> on your end.
>>
>>
>> Thanks,
>> Shreeya Patel
> Hi Shreeya,
> I may need your help to reproduce the failure.
>
> $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
> -O .config
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s
> <launch qemu>
> / # mount -t proc /proc
> / # cat /proc/version
> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
> 2.35.2) #2 SMP Wed May  3 21:10:27 UTC 2023
>
> I was able to boot the resulting kernel to a command line.  Perhaps
> there's something about the userspace image that tickles this? Can you
> supply the rootfs that's used in testing?


Following is the link to the rootfs for a different kernel build which 
has the same test case failing.
http://storage.kernelci.org/images/rootfs/buildroot/buildroot-baseline/20230414.0/armel/rootfs.cpio.gz 


Lava job :-
https://lava.collabora.dev/scheduler/job/10135631

You can usually get the rootfs link and many more information through 
the "definition" present on Lava job dashboard.

Kernelci test id for the above job :-
https://linux.kernelci.org/test/case/id/64497865a4bab57def2e85e8/


Thanks,
Shreeya Patel

>>> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc
>>> (Debian 10.2.1-6) in QEMU just fine.  So I'm going to need some more
>>> information to help reproduce what specifically is failing.
>>>
>>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
>>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
>>> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023
>>>
>>> ---
>>>
>>> It does look like UBSAN is flagging an array OOB:
>>>
>>> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
>>> ../arch/arm/mach-sunxi/mc_smp.c:811:29
>>>
>>> And potentially another issue with ftrace
>>>
>>> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
>>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
>>>
>>>
>>>
>>>> [2] https://groups.io/g/kernelci-results/message/40804
>>>>
>>>>
>>>> Thanks,
>>>> Shreeya Patel
>>>>
>>>> #regzbot introduced: 88b61e3bff93
>>>>
>>>>> ---
>>>>> Changes v3 -> v4:
>>>>> * Split into its own patch again from series, as per Masahiro.
>>>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update
>>>>>      clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro.
>>>>>
>>>>>     Documentation/kbuild/makefiles.rst          | 29 ++++++++++++---------
>>>>>     Makefile                                    |  6 ++---
>>>>>     drivers/gpu/drm/amd/display/dc/dml/Makefile |  2 +-
>>>>>     scripts/Makefile.compiler                   | 10 ++++---
>>>>>     scripts/Makefile.extrawarn                  |  4 +--
>>>>>     5 files changed, 29 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
>>>>> index 11a296e52d68..ee7e3ea1fbe1 100644
>>>>> --- a/Documentation/kbuild/makefiles.rst
>>>>> +++ b/Documentation/kbuild/makefiles.rst
>>>>> @@ -682,22 +682,27 @@ more details, with real examples.
>>>>>         In the above example, -Wno-unused-but-set-variable will be added to
>>>>>         KBUILD_CFLAGS only if gcc really accepts it.
>>>>>
>>>>> -    cc-ifversion
>>>>> -     cc-ifversion tests the version of $(CC) and equals the fourth parameter
>>>>> -     if version expression is true, or the fifth (if given) if the version
>>>>> -     expression is false.
>>>>> +    gcc-min-version
>>>>> +     gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than
>>>>> +     or equal to the provided value and evaluates to y if so.
>>>>>
>>>>>         Example::
>>>>>
>>>>> -             #fs/reiserfs/Makefile
>>>>> -             ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
>>>>> +             cflags-$(call gcc-min-version, 70100) := -foo
>>>>>
>>>>> -     In this example, ccflags-y will be assigned the value -O1 if the
>>>>> -     $(CC) version is less than 4.2.
>>>>> -     cc-ifversion takes all the shell operators:
>>>>> -     -eq, -ne, -lt, -le, -gt, and -ge
>>>>> -     The third parameter may be a text as in this example, but it may also
>>>>> -     be an expanded variable or a macro.
>>>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and
>>>>> +     $(CONFIG_GCC_VERSION) is >= 7.1.
>>>>> +
>>>>> +    clang-min-version
>>>>> +     clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater
>>>>> +     than or equal to the provided value and evaluates to y if so.
>>>>> +
>>>>> +     Example::
>>>>> +
>>>>> +             cflags-$(call clang-min-version, 110000) := -foo
>>>>> +
>>>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is clang
>>>>> +     and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
>>>>>
>>>>>         cc-cross-prefix
>>>>>         cc-cross-prefix is used to check if there exists a $(CC) in path with
>>>>> diff --git a/Makefile b/Makefile
>>>>> index 298f69060f10..411c8480b37e 100644
>>>>> --- a/Makefile
>>>>> +++ b/Makefile
>>>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y)
>>>>>
>>>>>     KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
>>>>>     KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>>>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>>>
>>>>>     ifdef CONFIG_CC_IS_CLANG
>>>>>     KBUILD_CPPFLAGS += -Qunused-arguments
>>>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC
>>>>>     KBUILD_CFLAGS += -Wno-maybe-uninitialized
>>>>>     endif
>>>>>
>>>>> -ifdef CONFIG_CC_IS_GCC
>>>>>     # The allocators already balk at large sizes, so silence the compiler
>>>>>     # warnings for bounds checks involving those possible values. While
>>>>>     # -Wno-alloc-size-larger-than would normally be used here, earlier versions
>>>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC
>>>>>     # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
>>>>>     # choice, we must perform a versioned check to disable this warning.
>>>>>     # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
>>>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
>>>>> -endif
>>>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than
>>>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>>>
>>>>>     # disable invalid "can't wrap" optimizations for signed / pointers
>>>>>     KBUILD_CFLAGS       += -fno-strict-overflow
>>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>>>> index cb81ed2fbd53..d70838edba80 100644
>>>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
>>>>>     endif
>>>>>
>>>>>     ifdef CONFIG_CC_IS_GCC
>>>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y)
>>>>> +ifneq ($(call gcc-min-version, 70100),y)
>>>>>     IS_OLD_GCC = 1
>>>>>     endif
>>>>>     endif
>>>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
>>>>> index 94d0d40cddb3..9d18fb91890e 100644
>>>>> --- a/scripts/Makefile.compiler
>>>>> +++ b/scripts/Makefile.compiler
>>>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\
>>>>>     cc-disable-warning = $(call try-run,\
>>>>>         $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
>>>>>
>>>>> -# cc-ifversion
>>>>> -# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
>>>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
>>>>> +# gcc-min-version
>>>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo
>>>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
>>>>> +
>>>>> +# clang-min-version
>>>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo
>>>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y)
>>>>>
>>>>>     # ld-option
>>>>>     # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
>>>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
>>>>> index 6ae482158bc4..5769c1939d40 100644
>>>>> --- a/scripts/Makefile.extrawarn
>>>>> +++ b/scripts/Makefile.extrawarn
>>>>> @@ -48,7 +48,7 @@ else
>>>>>     ifdef CONFIG_CC_IS_CLANG
>>>>>     KBUILD_CFLAGS += -Wno-initializer-overrides
>>>>>     # Clang before clang-16 would warn on default argument promotions.
>>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
>>>>> +ifneq ($(call clang-min-version, 160000),y)
>>>>>     # Disable -Wformat
>>>>>     KBUILD_CFLAGS += -Wno-format
>>>>>     # Then re-enable flags that were part of the -Wformat group that aren't
>>>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format
>>>>>     KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
>>>>>     KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
>>>>>     # Requires clang-12+.
>>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
>>>>> +ifeq ($(call clang-min-version, 120000),y)
>>>>>     KBUILD_CFLAGS += -Wformat-insufficient-args
>>>>>     endif
>>>>>     endif
>>>
>
>

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

[Nick Desaulniers]

On Wed, May 3, 2023 at 3:33 PM Shreeya Patel
<shreeya.patel@collabora.com> wrote:
>
>
> On 04/05/23 02:45, Nick Desaulniers wrote:
> > On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
> > <shreeya.patel@collabora.com> wrote:
> >> Hi Nick,
> >>
> >> On 28/04/23 22:57, Nick Desaulniers wrote:
> >>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
> >>> <shreeya.patel@collabora.com> wrote:
> >>>> Hi Nick,
> >>>>
> >>>> On 19/09/22 22:38, Nick Desaulniers wrote:
> >>>>> cc-ifversion is GCC specific. Replace it with compiler specific
> >>>>> variants. Update the users of cc-ifversion to use these new macros.
> >>>>>
> >>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
> >>>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
> >>>>> Suggested-by: Bill Wendling <morbo@google.com>
> >>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> >>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >>>> KernelCI found this patch causes a regression in the
> >>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
> >>>> see the bisection report for more details [2].
> >>>>
> >>>> Let me know if you have any questions.
> >>>>
> >>>>
> >>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
> >>> Hi Shreeya,
> >>> Thanks for the report.
> >>>
> >>> When I click the above link, then click `multi_v7_defconfig+debug` to
> >>> get the config necessary to reproduce, I get an HTTP 404.
> >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config
> >>>
> >>> Same for zImage
> >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage
> >> Apologies for the broken links. We will try to fix the important ones if
> >> we can but in the meantime,
> >> following is the correct link that you can refer.
> >>
> >> config :-
> >> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
> >>
> >> zImage :-
> >> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage
> >>
> >> If you notice, they are present under the kernel directory and same way
> >> you can find links for other kernel
> >> builds if you'd like to check them out.
> >>
> >>> If I click on the log
> >>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt
> >>> It looks like the machine powered up, then powered off. Is the test
> >>> actually failing?
> >> I recommend checking the html logs from the kernelci dashboard.
> >> Also, FYI baseline.login test failure means that the device failed to
> >> boot which I think is causing by the issues that you pointed out.
> >>
> >> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
> >> ../arch/arm/mach-sunxi/mc_smp.c:811:29
> >>
> >> And potentially another issue with ftrace
> >>
> >> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
> >> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
> >>
> >>
> >> Let me know if you need more information from my side to reproduce this
> >> on your end.
> >>
> >>
> >> Thanks,
> >> Shreeya Patel
> > Hi Shreeya,
> > I may need your help to reproduce the failure.

Hi Shreeya,
Sorry for the delay, I was out traveling last week.  I still need help
reproducing. Trying this again from scratch, my VM is now failing to
boot regardless of whether I revert the patch in question or not.

Can you please help verify this failure by hand, and see if applying
https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch
helps?  If you can repro; mind sharing your precise steps to reproduce?

> >
> > $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
> > -O .config
> > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s
> > <launch qemu>
> > / # mount -t proc /proc
> > / # cat /proc/version
> > Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
> > (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
> > 2.35.2) #2 SMP Wed May  3 21:10:27 UTC 2023
> >
> > I was able to boot the resulting kernel to a command line.  Perhaps
> > there's something about the userspace image that tickles this? Can you
> > supply the rootfs that's used in testing?
>
>
> Following is the link to the rootfs for a different kernel build which
> has the same test case failing.
> http://storage.kernelci.org/images/rootfs/buildroot/buildroot-baseline/20230414.0/armel/rootfs.cpio.gz
>
>
> Lava job :-
> https://lava.collabora.dev/scheduler/job/10135631
>
> You can usually get the rootfs link and many more information through
> the "definition" present on Lava job dashboard.
>
> Kernelci test id for the above job :-
> https://linux.kernelci.org/test/case/id/64497865a4bab57def2e85e8/
>
>
> Thanks,
> Shreeya Patel
>
> >>> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc
> >>> (Debian 10.2.1-6) in QEMU just fine.  So I'm going to need some more
> >>> information to help reproduce what specifically is failing.
> >>>
> >>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
> >>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
> >>> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023
> >>>
> >>> ---
> >>>
> >>> It does look like UBSAN is flagging an array OOB:
> >>>
> >>> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
> >>> ../arch/arm/mach-sunxi/mc_smp.c:811:29
> >>>
> >>> And potentially another issue with ftrace
> >>>
> >>> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
> >>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
> >>>
> >>>
> >>>
> >>>> [2] https://groups.io/g/kernelci-results/message/40804
> >>>>
> >>>>
> >>>> Thanks,
> >>>> Shreeya Patel
> >>>>
> >>>> #regzbot introduced: 88b61e3bff93
> >>>>
> >>>>> ---
> >>>>> Changes v3 -> v4:
> >>>>> * Split into its own patch again from series, as per Masahiro.
> >>>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update
> >>>>>      clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro.
> >>>>>
> >>>>>     Documentation/kbuild/makefiles.rst          | 29 ++++++++++++---------
> >>>>>     Makefile                                    |  6 ++---
> >>>>>     drivers/gpu/drm/amd/display/dc/dml/Makefile |  2 +-
> >>>>>     scripts/Makefile.compiler                   | 10 ++++---
> >>>>>     scripts/Makefile.extrawarn                  |  4 +--
> >>>>>     5 files changed, 29 insertions(+), 22 deletions(-)
> >>>>>
> >>>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
> >>>>> index 11a296e52d68..ee7e3ea1fbe1 100644
> >>>>> --- a/Documentation/kbuild/makefiles.rst
> >>>>> +++ b/Documentation/kbuild/makefiles.rst
> >>>>> @@ -682,22 +682,27 @@ more details, with real examples.
> >>>>>         In the above example, -Wno-unused-but-set-variable will be added to
> >>>>>         KBUILD_CFLAGS only if gcc really accepts it.
> >>>>>
> >>>>> -    cc-ifversion
> >>>>> -     cc-ifversion tests the version of $(CC) and equals the fourth parameter
> >>>>> -     if version expression is true, or the fifth (if given) if the version
> >>>>> -     expression is false.
> >>>>> +    gcc-min-version
> >>>>> +     gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than
> >>>>> +     or equal to the provided value and evaluates to y if so.
> >>>>>
> >>>>>         Example::
> >>>>>
> >>>>> -             #fs/reiserfs/Makefile
> >>>>> -             ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
> >>>>> +             cflags-$(call gcc-min-version, 70100) := -foo
> >>>>>
> >>>>> -     In this example, ccflags-y will be assigned the value -O1 if the
> >>>>> -     $(CC) version is less than 4.2.
> >>>>> -     cc-ifversion takes all the shell operators:
> >>>>> -     -eq, -ne, -lt, -le, -gt, and -ge
> >>>>> -     The third parameter may be a text as in this example, but it may also
> >>>>> -     be an expanded variable or a macro.
> >>>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and
> >>>>> +     $(CONFIG_GCC_VERSION) is >= 7.1.
> >>>>> +
> >>>>> +    clang-min-version
> >>>>> +     clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater
> >>>>> +     than or equal to the provided value and evaluates to y if so.
> >>>>> +
> >>>>> +     Example::
> >>>>> +
> >>>>> +             cflags-$(call clang-min-version, 110000) := -foo
> >>>>> +
> >>>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is clang
> >>>>> +     and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
> >>>>>
> >>>>>         cc-cross-prefix
> >>>>>         cc-cross-prefix is used to check if there exists a $(CC) in path with
> >>>>> diff --git a/Makefile b/Makefile
> >>>>> index 298f69060f10..411c8480b37e 100644
> >>>>> --- a/Makefile
> >>>>> +++ b/Makefile
> >>>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y)
> >>>>>
> >>>>>     KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
> >>>>>     KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
> >>>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> >>>>>
> >>>>>     ifdef CONFIG_CC_IS_CLANG
> >>>>>     KBUILD_CPPFLAGS += -Qunused-arguments
> >>>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC
> >>>>>     KBUILD_CFLAGS += -Wno-maybe-uninitialized
> >>>>>     endif
> >>>>>
> >>>>> -ifdef CONFIG_CC_IS_GCC
> >>>>>     # The allocators already balk at large sizes, so silence the compiler
> >>>>>     # warnings for bounds checks involving those possible values. While
> >>>>>     # -Wno-alloc-size-larger-than would normally be used here, earlier versions
> >>>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC
> >>>>>     # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
> >>>>>     # choice, we must perform a versioned check to disable this warning.
> >>>>>     # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
> >>>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
> >>>>> -endif
> >>>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than
> >>>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> >>>>>
> >>>>>     # disable invalid "can't wrap" optimizations for signed / pointers
> >>>>>     KBUILD_CFLAGS       += -fno-strict-overflow
> >>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> >>>>> index cb81ed2fbd53..d70838edba80 100644
> >>>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> >>>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> >>>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
> >>>>>     endif
> >>>>>
> >>>>>     ifdef CONFIG_CC_IS_GCC
> >>>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y)
> >>>>> +ifneq ($(call gcc-min-version, 70100),y)
> >>>>>     IS_OLD_GCC = 1
> >>>>>     endif
> >>>>>     endif
> >>>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> >>>>> index 94d0d40cddb3..9d18fb91890e 100644
> >>>>> --- a/scripts/Makefile.compiler
> >>>>> +++ b/scripts/Makefile.compiler
> >>>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\
> >>>>>     cc-disable-warning = $(call try-run,\
> >>>>>         $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
> >>>>>
> >>>>> -# cc-ifversion
> >>>>> -# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> >>>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
> >>>>> +# gcc-min-version
> >>>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo
> >>>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
> >>>>> +
> >>>>> +# clang-min-version
> >>>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo
> >>>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y)
> >>>>>
> >>>>>     # ld-option
> >>>>>     # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
> >>>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> >>>>> index 6ae482158bc4..5769c1939d40 100644
> >>>>> --- a/scripts/Makefile.extrawarn
> >>>>> +++ b/scripts/Makefile.extrawarn
> >>>>> @@ -48,7 +48,7 @@ else
> >>>>>     ifdef CONFIG_CC_IS_CLANG
> >>>>>     KBUILD_CFLAGS += -Wno-initializer-overrides
> >>>>>     # Clang before clang-16 would warn on default argument promotions.
> >>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
> >>>>> +ifneq ($(call clang-min-version, 160000),y)
> >>>>>     # Disable -Wformat
> >>>>>     KBUILD_CFLAGS += -Wno-format
> >>>>>     # Then re-enable flags that were part of the -Wformat group that aren't
> >>>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format
> >>>>>     KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
> >>>>>     KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
> >>>>>     # Requires clang-12+.
> >>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
> >>>>> +ifeq ($(call clang-min-version, 120000),y)
> >>>>>     KBUILD_CFLAGS += -Wformat-insufficient-args
> >>>>>     endif
> >>>>>     endif
> >>>
> >
> >



-- 
Thanks,
~Nick Desaulniers

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

[Shreeya Patel]

Hi Nick,

On 16/05/23 04:31, Nick Desaulniers wrote:
> On Wed, May 3, 2023 at 3:33 PM Shreeya Patel
> <shreeya.patel@collabora.com> wrote:
>>
>> On 04/05/23 02:45, Nick Desaulniers wrote:
>>> On Wed, May 3, 2023 at 2:02 PM Shreeya Patel
>>> <shreeya.patel@collabora.com> wrote:
>>>> Hi Nick,
>>>>
>>>> On 28/04/23 22:57, Nick Desaulniers wrote:
>>>>> On Thu, Apr 27, 2023 at 4:54 AM Shreeya Patel
>>>>> <shreeya.patel@collabora.com> wrote:
>>>>>> Hi Nick,
>>>>>>
>>>>>> On 19/09/22 22:38, Nick Desaulniers wrote:
>>>>>>> cc-ifversion is GCC specific. Replace it with compiler specific
>>>>>>> variants. Update the users of cc-ifversion to use these new macros.
>>>>>>>
>>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/350
>>>>>>> Link: https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=BQ@mail.gmail.com/
>>>>>>> Suggested-by: Bill Wendling <morbo@google.com>
>>>>>>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>>>>>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>>>>>> KernelCI found this patch causes a regression in the
>>>>>> baseline.logintest on qemu_arm-virt-gicv3-uefi [1],
>>>>>> see the bisection report for more details [2].
>>>>>>
>>>>>> Let me know if you have any questions.
>>>>>>
>>>>>>
>>>>>> [1] https://linux.kernelci.org/test/case/id/644596a0beca2ead032e8669/
>>>>> Hi Shreeya,
>>>>> Thanks for the report.
>>>>>
>>>>> When I click the above link, then click `multi_v7_defconfig+debug` to
>>>>> get the config necessary to reproduce, I get an HTTP 404.
>>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel.config
>>>>>
>>>>> Same for zImage
>>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/zImage
>>>> Apologies for the broken links. We will try to fix the important ones if
>>>> we can but in the meantime,
>>>> following is the correct link that you can refer.
>>>>
>>>> config :-
>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
>>>>
>>>> zImage :-
>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/kernel/zImage
>>>>
>>>> If you notice, they are present under the kernel directory and same way
>>>> you can find links for other kernel
>>>> builds if you'd like to check them out.
>>>>
>>>>> If I click on the log
>>>>> https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/lab-collabora/baseline-qemu_arm-virt-gicv3-uefi.txt
>>>>> It looks like the machine powered up, then powered off. Is the test
>>>>> actually failing?
>>>> I recommend checking the html logs from the kernelci dashboard.
>>>> Also, FYI baseline.login test failure means that the device failed to
>>>> boot which I think is causing by the issues that you pointed out.
>>>>
>>>> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
>>>> ../arch/arm/mach-sunxi/mc_smp.c:811:29
>>>>
>>>> And potentially another issue with ftrace
>>>>
>>>> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
>>>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
>>>>
>>>>
>>>> Let me know if you need more information from my side to reproduce this
>>>> on your end.
>>>>
>>>>
>>>> Thanks,
>>>> Shreeya Patel
>>> Hi Shreeya,
>>> I may need your help to reproduce the failure.
> Hi Shreeya,
> Sorry for the delay, I was out traveling last week.  I still need help
> reproducing. Trying this again from scratch, my VM is now failing to
> boot regardless of whether I revert the patch in question or not.
>
> Can you please help verify this failure by hand, and see if applying
> https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch
> helps?  If you can repro; mind sharing your precise steps to reproduce?



No worries, even I was out traveling last week so wouldn't have been 
able to help you either.
Ricardo from my team is currently testing the patch that you have sent. 
We will let you know
the results soon.

I haven't really tried to manually reproduce it myself yet but I'll 
check and let you know what I can
do on my end.

Thanks,
Shreeya Patel

>>> $ wget https://storage.kernelci.org/mainline/master/v6.3/arm/multi_v7_defconfig+debug/gcc-10/config/kernel.config
>>> -O .config
>>> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j128 olddefconfig all -s
>>> <launch qemu>
>>> / # mount -t proc /proc
>>> / # cat /proc/version
>>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
>>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
>>> 2.35.2) #2 SMP Wed May  3 21:10:27 UTC 2023
>>>
>>> I was able to boot the resulting kernel to a command line.  Perhaps
>>> there's something about the userspace image that tickles this? Can you
>>> supply the rootfs that's used in testing?
>>
>> Following is the link to the rootfs for a different kernel build which
>> has the same test case failing.
>> http://storage.kernelci.org/images/rootfs/buildroot/buildroot-baseline/20230414.0/armel/rootfs.cpio.gz
>>
>>
>> Lava job :-
>> https://lava.collabora.dev/scheduler/job/10135631
>>
>> You can usually get the rootfs link and many more information through
>> the "definition" present on Lava job dashboard.
>>
>> Kernelci test id for the above job :-
>> https://linux.kernelci.org/test/case/id/64497865a4bab57def2e85e8/
>>
>>
>> Thanks,
>> Shreeya Patel
>>
>>>>> I was able to boot ARCH=arm defconfig with CC=arm-linux-gnueabihf-gcc
>>>>> (Debian 10.2.1-6) in QEMU just fine.  So I'm going to need some more
>>>>> information to help reproduce what specifically is failing.
>>>>>
>>>>> Linux version 6.3.0 (root@61385772abae) (arm-linux-gnueabihf-gcc
>>>>> (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian)
>>>>> 2.35.2) #1 SMP Fri Apr 28 17:19:59 UTC 2023
>>>>>
>>>>> ---
>>>>>
>>>>> It does look like UBSAN is flagging an array OOB:
>>>>>
>>>>> <3>[    0.417001][    T1] UBSAN: array-index-out-of-bounds in
>>>>> ../arch/arm/mach-sunxi/mc_smp.c:811:29
>>>>>
>>>>> And potentially another issue with ftrace
>>>>>
>>>>> <4>[    0.000000][    T0] WARNING: CPU: 0 PID: 0 at
>>>>> kernel/trace/ftrace.c:2176 ftrace_bug+0x340/0x3b4
>>>>>
>>>>>
>>>>>
>>>>>> [2] https://groups.io/g/kernelci-results/message/40804
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Shreeya Patel
>>>>>>
>>>>>> #regzbot introduced: 88b61e3bff93
>>>>>>
>>>>>>> ---
>>>>>>> Changes v3 -> v4:
>>>>>>> * Split into its own patch again from series, as per Masahiro.
>>>>>>> * Rebase on top of b0839b281c427e844143dba3893e25c83cdd6c17 and update
>>>>>>>       clang -Wformat logic in scripts/Makefile.extrawarn, as per Masahiro.
>>>>>>>
>>>>>>>      Documentation/kbuild/makefiles.rst          | 29 ++++++++++++---------
>>>>>>>      Makefile                                    |  6 ++---
>>>>>>>      drivers/gpu/drm/amd/display/dc/dml/Makefile |  2 +-
>>>>>>>      scripts/Makefile.compiler                   | 10 ++++---
>>>>>>>      scripts/Makefile.extrawarn                  |  4 +--
>>>>>>>      5 files changed, 29 insertions(+), 22 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
>>>>>>> index 11a296e52d68..ee7e3ea1fbe1 100644
>>>>>>> --- a/Documentation/kbuild/makefiles.rst
>>>>>>> +++ b/Documentation/kbuild/makefiles.rst
>>>>>>> @@ -682,22 +682,27 @@ more details, with real examples.
>>>>>>>          In the above example, -Wno-unused-but-set-variable will be added to
>>>>>>>          KBUILD_CFLAGS only if gcc really accepts it.
>>>>>>>
>>>>>>> -    cc-ifversion
>>>>>>> -     cc-ifversion tests the version of $(CC) and equals the fourth parameter
>>>>>>> -     if version expression is true, or the fifth (if given) if the version
>>>>>>> -     expression is false.
>>>>>>> +    gcc-min-version
>>>>>>> +     gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater than
>>>>>>> +     or equal to the provided value and evaluates to y if so.
>>>>>>>
>>>>>>>          Example::
>>>>>>>
>>>>>>> -             #fs/reiserfs/Makefile
>>>>>>> -             ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
>>>>>>> +             cflags-$(call gcc-min-version, 70100) := -foo
>>>>>>>
>>>>>>> -     In this example, ccflags-y will be assigned the value -O1 if the
>>>>>>> -     $(CC) version is less than 4.2.
>>>>>>> -     cc-ifversion takes all the shell operators:
>>>>>>> -     -eq, -ne, -lt, -le, -gt, and -ge
>>>>>>> -     The third parameter may be a text as in this example, but it may also
>>>>>>> -     be an expanded variable or a macro.
>>>>>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is gcc and
>>>>>>> +     $(CONFIG_GCC_VERSION) is >= 7.1.
>>>>>>> +
>>>>>>> +    clang-min-version
>>>>>>> +     clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is greater
>>>>>>> +     than or equal to the provided value and evaluates to y if so.
>>>>>>> +
>>>>>>> +     Example::
>>>>>>> +
>>>>>>> +             cflags-$(call clang-min-version, 110000) := -foo
>>>>>>> +
>>>>>>> +     In this example, cflags-y will be assigned the value -foo if $(CC) is clang
>>>>>>> +     and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
>>>>>>>
>>>>>>>          cc-cross-prefix
>>>>>>>          cc-cross-prefix is used to check if there exists a $(CC) in path with
>>>>>>> diff --git a/Makefile b/Makefile
>>>>>>> index 298f69060f10..411c8480b37e 100644
>>>>>>> --- a/Makefile
>>>>>>> +++ b/Makefile
>>>>>>> @@ -790,7 +790,6 @@ KBUILD_CFLAGS += $(stackp-flags-y)
>>>>>>>
>>>>>>>      KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
>>>>>>>      KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>>>>>>> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>>>>>
>>>>>>>      ifdef CONFIG_CC_IS_CLANG
>>>>>>>      KBUILD_CPPFLAGS += -Qunused-arguments
>>>>>>> @@ -972,7 +971,6 @@ ifdef CONFIG_CC_IS_GCC
>>>>>>>      KBUILD_CFLAGS += -Wno-maybe-uninitialized
>>>>>>>      endif
>>>>>>>
>>>>>>> -ifdef CONFIG_CC_IS_GCC
>>>>>>>      # The allocators already balk at large sizes, so silence the compiler
>>>>>>>      # warnings for bounds checks involving those possible values. While
>>>>>>>      # -Wno-alloc-size-larger-than would normally be used here, earlier versions
>>>>>>> @@ -984,8 +982,8 @@ ifdef CONFIG_CC_IS_GCC
>>>>>>>      # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
>>>>>>>      # choice, we must perform a versioned check to disable this warning.
>>>>>>>      # https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
>>>>>>> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
>>>>>>> -endif
>>>>>>> +KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than
>>>>>>> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>>>>>>
>>>>>>>      # disable invalid "can't wrap" optimizations for signed / pointers
>>>>>>>      KBUILD_CFLAGS       += -fno-strict-overflow
>>>>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>>>>>> index cb81ed2fbd53..d70838edba80 100644
>>>>>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>>>>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>>>>>> @@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
>>>>>>>      endif
>>>>>>>
>>>>>>>      ifdef CONFIG_CC_IS_GCC
>>>>>>> -ifeq ($(call cc-ifversion, -lt, 0701, y), y)
>>>>>>> +ifneq ($(call gcc-min-version, 70100),y)
>>>>>>>      IS_OLD_GCC = 1
>>>>>>>      endif
>>>>>>>      endif
>>>>>>> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
>>>>>>> index 94d0d40cddb3..9d18fb91890e 100644
>>>>>>> --- a/scripts/Makefile.compiler
>>>>>>> +++ b/scripts/Makefile.compiler
>>>>>>> @@ -61,9 +61,13 @@ cc-option-yn = $(call try-run,\
>>>>>>>      cc-disable-warning = $(call try-run,\
>>>>>>>          $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
>>>>>>>
>>>>>>> -# cc-ifversion
>>>>>>> -# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
>>>>>>> -cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
>>>>>>> +# gcc-min-version
>>>>>>> +# Usage: cflags-$(call gcc-min-version, 70100) += -foo
>>>>>>> +gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
>>>>>>> +
>>>>>>> +# clang-min-version
>>>>>>> +# Usage: cflags-$(call clang-min-version, 110000) += -foo
>>>>>>> +clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y)
>>>>>>>
>>>>>>>      # ld-option
>>>>>>>      # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
>>>>>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
>>>>>>> index 6ae482158bc4..5769c1939d40 100644
>>>>>>> --- a/scripts/Makefile.extrawarn
>>>>>>> +++ b/scripts/Makefile.extrawarn
>>>>>>> @@ -48,7 +48,7 @@ else
>>>>>>>      ifdef CONFIG_CC_IS_CLANG
>>>>>>>      KBUILD_CFLAGS += -Wno-initializer-overrides
>>>>>>>      # Clang before clang-16 would warn on default argument promotions.
>>>>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
>>>>>>> +ifneq ($(call clang-min-version, 160000),y)
>>>>>>>      # Disable -Wformat
>>>>>>>      KBUILD_CFLAGS += -Wno-format
>>>>>>>      # Then re-enable flags that were part of the -Wformat group that aren't
>>>>>>> @@ -56,7 +56,7 @@ KBUILD_CFLAGS += -Wno-format
>>>>>>>      KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
>>>>>>>      KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
>>>>>>>      # Requires clang-12+.
>>>>>>> -ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
>>>>>>> +ifeq ($(call clang-min-version, 120000),y)
>>>>>>>      KBUILD_CFLAGS += -Wformat-insufficient-args
>>>>>>>      endif
>>>>>>>      endif
>>>
>
>

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

[Ricardo Cañuelo]

Hi Nick,

On 16/5/23 1:01, Nick Desaulniers wrote:
> Can you please help verify this failure by hand, and see if applying
> https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch
> helps?  If you can repro; mind sharing your precise steps to reproduce?

I ran a few tests but the commit that introduced your changes
passes every time. There's a chance that the bisector got misled
due to the test runs failing for whatever reason unrelated to the
patch. There's definitely something introducing a bug somewhere,
as current mainline/master makes this test fail on this target
when kernel/configs/debug.config is applied, but it must be
somewhere else. I'll investigate this some more to see what I can
find.

About the steps to reproduce it, we're using the current KernelCI
tools (kci_build) to generate the kernel. To actually launch the
tests I'm submitting jobs to Collabora's LAVA lab, which is
something that isn't available to external users, so it might be
a bit hard for you to reproduce the exact environment from the
original test. If you need to test something, I can do it for
you.

Thanks,
Ricardo

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

[Nick Desaulniers]

On Wed, May 17, 2023 at 8:39 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Hi Nick,
>
> On 16/5/23 1:01, Nick Desaulniers wrote:
> > Can you please help verify this failure by hand, and see if applying
> > https://github.com/ClangBuiltLinux/linux/commit/45c4fb6095d872785e077942da896d65d87ab56b.patch
> > helps?  If you can repro; mind sharing your precise steps to reproduce?
>
> I ran a few tests but the commit that introduced your changes
> passes every time. There's a chance that the bisector got misled
> due to the test runs failing for whatever reason unrelated to the
> patch. There's definitely something introducing a bug somewhere,
> as current mainline/master makes this test fail on this target
> when kernel/configs/debug.config is applied, but it must be
> somewhere else. I'll investigate this some more to see what I can
> find.

Thanks for verifying/reporting.


>
> About the steps to reproduce it, we're using the current KernelCI
> tools (kci_build) to generate the kernel. To actually launch the
> tests I'm submitting jobs to Collabora's LAVA lab, which is
> something that isn't available to external users, so it might be
> a bit hard for you to reproduce the exact environment from the
> original test. If you need to test something, I can do it for
> you.

Shreeya mentioned upthread that `qemu_arm-virt-gicv3-uefi` was
failing, so I assume others should be able to repro in qemu. I'd guess
that LAVA lets you have VMs adjacent to physical hardware.  Having the
qemu command line, kernel config, and perhaps the initramfs are going
to be the three most useful things for any similar bug report.
-- 
Thanks,
~Nick Desaulniers

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

[Ricardo Cañuelo]

Hi Nick,

On mié, may 17 2023 at 09:27:41, Nick Desaulniers <ndesaulniers@google.com> wrote:
> Shreeya mentioned upthread that `qemu_arm-virt-gicv3-uefi` was
> failing, so I assume others should be able to repro in qemu. I'd guess
> that LAVA lets you have VMs adjacent to physical hardware.  Having the
> qemu command line, kernel config, and perhaps the initramfs are going
> to be the three most useful things for any similar bug report.

Sure, I'm using LAVA because that's the safest way to match exactly the
same setup from the original test, but anyone can try to reproduce it on
their own. You can get the job info and setup from any of the related
LAVA jobs. This one for example:
https://lava.collabora.dev/scheduler/job/10373216

Trying to reproduce this type of setups is not as straightforward as
we'd like, specially for people not familiar with KernelCI, but we're
putting in some effort to improve that so that anyone can pick up a
regression report and work on it right away.

By the way, I found a breaking point in the commit right after the one
that the bisector reported:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5750121ae7382ebac8d47ce6d68012d6cd1d7926
but I can't find anything there either that makes the boot hang,
specifically for configs including kernel/configs/debug.config. I
wouldn't rule out a problem with the qemu configuration.

Anyway, this is not a critical problem in any way, although it'd be
interesting to find the cause in case we can use the findings to improve
the test setups.

Cheers,
Ricardo

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

[Nick Desaulniers]

On Thu, May 18, 2023 at 7:23 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
>
> Hi Nick,
>
> On mié, may 17 2023 at 09:27:41, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > Shreeya mentioned upthread that `qemu_arm-virt-gicv3-uefi` was
> > failing, so I assume others should be able to repro in qemu. I'd guess
> > that LAVA lets you have VMs adjacent to physical hardware.  Having the
> > qemu command line, kernel config, and perhaps the initramfs are going
> > to be the three most useful things for any similar bug report.
>
> Sure, I'm using LAVA because that's the safest way to match exactly the
> same setup from the original test, but anyone can try to reproduce it on
> their own. You can get the job info and setup from any of the related
> LAVA jobs. This one for example:
> https://lava.collabora.dev/scheduler/job/10373216
>
> Trying to reproduce this type of setups is not as straightforward as
> we'd like, specially for people not familiar with KernelCI, but we're
> putting in some effort to improve that so that anyone can pick up a
> regression report and work on it right away.

Yeah makes sense.  Having reports surface the most relevant info for
developers to understand what went wrong where and how to reproduce is
something I'm passionate about.  We've been through this with Intel
0day bot folks; it takes a few iterations until everyone is happy.

>
> By the way, I found a breaking point in the commit right after the one
> that the bisector reported:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5750121ae7382ebac8d47ce6d68012d6cd1d7926

That's a higher risk change (and has my name on the tested-by tag, yikes).

So is that the culprit of the boot failure you're observing?

> but I can't find anything there either that makes the boot hang,
> specifically for configs including kernel/configs/debug.config. I
> wouldn't rule out a problem with the qemu configuration.
>
> Anyway, this is not a critical problem in any way, although it'd be

But you still have a boot failure/regression to track down for that
config, right?

> interesting to find the cause in case we can use the findings to improve
> the test setups.
>
> Cheers,
> Ricardo



-- 
Thanks,
~Nick Desaulniers

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

[Ricardo Cañuelo]

On jue, may 18 2023 at 14:12:30, Nick Desaulniers <ndesaulniers@google.com> wrote:
> That's a higher risk change (and has my name on the tested-by tag, yikes).
>
> So is that the culprit of the boot failure you're observing?

Right now it is.

Here's a test run using that commit
(5750121ae7382ebac8d47ce6d68012d6cd1d7926):
https://lava.collabora.dev/scheduler/job/10373216

Here's one with the commit right after that one
(26ef40de5cbb24728a34a319e8d42cdec99f186c):
https://lava.collabora.dev/scheduler/job/10371513

Then one with 26ef40de5cbb24728a34a319e8d42cdec99f186c with a revert
commit for 5750121ae7382ebac8d47ce6d68012d6cd1d7926 on top:
https://lava.collabora.dev/scheduler/job/10371882

But I'm not confident enough to jump ahead and call this a kernel
regression, specially after the bisector confidently said that about
your commit and then it turned out none of us could reproduce it.

There have been some cases where a commit made a test fail (kernel
failing to load, for instance) and the real problem was that the kernel
got bigger than the target was capable of handling. So not a problem
with the commit at all, it was just that the memory mappings needed to
be redefined for that target. What I'm saying is that sometimes a
regression report is really uncovering a problem in the test setup
rather than introducing a bug. Maybe this is one of those cases.

Cheers,
Ricardo

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

[Nick Desaulniers]

On Fri, May 19, 2023 at 1:35 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> On jue, may 18 2023 at 14:12:30, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > That's a higher risk change (and has my name on the tested-by tag, yikes).
> >
> > So is that the culprit of the boot failure you're observing?
>
> Right now it is.
>
> Here's a test run using that commit
> (5750121ae7382ebac8d47ce6d68012d6cd1d7926):
> https://lava.collabora.dev/scheduler/job/10373216
>
> Here's one with the commit right after that one
> (26ef40de5cbb24728a34a319e8d42cdec99f186c):
> https://lava.collabora.dev/scheduler/job/10371513
>
> Then one with 26ef40de5cbb24728a34a319e8d42cdec99f186c with a revert
> commit for 5750121ae7382ebac8d47ce6d68012d6cd1d7926 on top:
> https://lava.collabora.dev/scheduler/job/10371882
>
> But I'm not confident enough to jump ahead and call this a kernel
> regression, specially after the bisector confidently said that about
> your commit and then it turned out none of us could reproduce it.

It could be; if the link order was changed, it's possible that this
target may be hitting something along the lines of:
https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
initialization order fiasco"

I'm struggling to think of how this appears in C codebases, but I
swear years ago I had a discussion with GKH (maybe?) about this. I
think I was playing with converting Kbuild to use Ninja rather than
Make; the resulting kernel image wouldn't boot because I had modified
the order the object files were linked in.  If you were to randomly
shuffle the object files in the kernel, I recall some hazard that may
prevent boot.

>
> There have been some cases where a commit made a test fail (kernel
> failing to load, for instance) and the real problem was that the kernel
> got bigger than the target was capable of handling. So not a problem
> with the commit at all, it was just that the memory mappings needed to
> be redefined for that target. What I'm saying is that sometimes a
> regression report is really uncovering a problem in the test setup
> rather than introducing a bug. Maybe this is one of those cases.
>
> Cheers,
> Ricardo



-- 
Thanks,
~Nick Desaulniers

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

[Ricardo Cañuelo]

On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> It could be; if the link order was changed, it's possible that this
> target may be hitting something along the lines of:
> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> initialization order fiasco"
>
> I'm struggling to think of how this appears in C codebases, but I
> swear years ago I had a discussion with GKH (maybe?) about this. I
> think I was playing with converting Kbuild to use Ninja rather than
> Make; the resulting kernel image wouldn't boot because I had modified
> the order the object files were linked in.  If you were to randomly
> shuffle the object files in the kernel, I recall some hazard that may
> prevent boot.

I thought that was specifically a C++ problem? But then again, the
kernel docs explicitly say that the ordering of obj-y goals in kbuild is
significant in some instances [1]:

    --- 3.2 Built-in object goals - obj-y

    [...]

    Link order is significant, because certain functions (module_init()
    / __initcall) will be called during boot in the order they
    appear. So keep in mind that changing the link order may e.g. change
    the order in which your SCSI controllers are detected, and thus your
    disks are renumbered.

We'll dig deeper into this. Thanks for your insight.

Cheers,
Ricardo

[1]: https://www.kernel.org/doc/Documentation/kbuild/makefiles.txt

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

[Greg KH]

On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > It could be; if the link order was changed, it's possible that this
> > target may be hitting something along the lines of:
> > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> > initialization order fiasco"
> >
> > I'm struggling to think of how this appears in C codebases, but I
> > swear years ago I had a discussion with GKH (maybe?) about this. I
> > think I was playing with converting Kbuild to use Ninja rather than
> > Make; the resulting kernel image wouldn't boot because I had modified
> > the order the object files were linked in.  If you were to randomly
> > shuffle the object files in the kernel, I recall some hazard that may
> > prevent boot.
> 
> I thought that was specifically a C++ problem? But then again, the
> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
> significant in some instances [1]:

Yes, it matters, you can not change it.  If you do, systems will break.
It is the only way we have of properly ordering our init calls within
the same "level".

thanks,

greg k-h

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

[Nick Desaulniers]

On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
> > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > It could be; if the link order was changed, it's possible that this
> > > target may be hitting something along the lines of:
> > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> > > initialization order fiasco"
> > >
> > > I'm struggling to think of how this appears in C codebases, but I
> > > swear years ago I had a discussion with GKH (maybe?) about this. I
> > > think I was playing with converting Kbuild to use Ninja rather than
> > > Make; the resulting kernel image wouldn't boot because I had modified
> > > the order the object files were linked in.  If you were to randomly
> > > shuffle the object files in the kernel, I recall some hazard that may
> > > prevent boot.
> >
> > I thought that was specifically a C++ problem? But then again, the
> > kernel docs explicitly say that the ordering of obj-y goals in kbuild is
> > significant in some instances [1]:
>
> Yes, it matters, you can not change it.  If you do, systems will break.
> It is the only way we have of properly ordering our init calls within
> the same "level".

Ah, right it was the initcall ordering. Thanks for the reminder.

(There's a joke in there similar to the use of regexes to solve a
problem resulting in two new problems; initcalls have levels for
ordering, but we still have (unexpressed) dependencies between calls
of the same level; brittle!).

+Maksim, since that might be relevant info for the BOLT+Kernel work.

Ricardo,
https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
mentions that there's a kernel command line param `initcall_debug`.
Perhaps that can be used to see if
5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
ordering, resulting in a config that cannot boot?
-- 
Thanks,
~Nick Desaulniers

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

[Greg KH]

On Mon, May 22, 2023 at 12:52:13PM -0700, Nick Desaulniers wrote:
> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
> > > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > It could be; if the link order was changed, it's possible that this
> > > > target may be hitting something along the lines of:
> > > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> > > > initialization order fiasco"
> > > >
> > > > I'm struggling to think of how this appears in C codebases, but I
> > > > swear years ago I had a discussion with GKH (maybe?) about this. I
> > > > think I was playing with converting Kbuild to use Ninja rather than
> > > > Make; the resulting kernel image wouldn't boot because I had modified
> > > > the order the object files were linked in.  If you were to randomly
> > > > shuffle the object files in the kernel, I recall some hazard that may
> > > > prevent boot.
> > >
> > > I thought that was specifically a C++ problem? But then again, the
> > > kernel docs explicitly say that the ordering of obj-y goals in kbuild is
> > > significant in some instances [1]:
> >
> > Yes, it matters, you can not change it.  If you do, systems will break.
> > It is the only way we have of properly ordering our init calls within
> > the same "level".
> 
> Ah, right it was the initcall ordering. Thanks for the reminder.
> 
> (There's a joke in there similar to the use of regexes to solve a
> problem resulting in two new problems; initcalls have levels for
> ordering, but we still have (unexpressed) dependencies between calls
> of the same level; brittle!).

No, the dependencies are explicitly expressed with the linker order.  So
it's not brittle, but rather very deterministic.

When linker order didn't work for all sorts of things, we added
different levels, but due to the huge number of init calls, of course
can not give each one their own level.

It's always been this way with Linux, nothing new here at all :)

thanks,

greg k-h

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

[Nick Desaulniers]

On Mon, May 22, 2023 at 1:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, May 22, 2023 at 12:52:13PM -0700, Nick Desaulniers wrote:
> > On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
> > > > On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > > It could be; if the link order was changed, it's possible that this
> > > > > target may be hitting something along the lines of:
> > > > > https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> > > > > initialization order fiasco"
> > > > >
> > > > > I'm struggling to think of how this appears in C codebases, but I
> > > > > swear years ago I had a discussion with GKH (maybe?) about this. I
> > > > > think I was playing with converting Kbuild to use Ninja rather than
> > > > > Make; the resulting kernel image wouldn't boot because I had modified
> > > > > the order the object files were linked in.  If you were to randomly
> > > > > shuffle the object files in the kernel, I recall some hazard that may
> > > > > prevent boot.
> > > >
> > > > I thought that was specifically a C++ problem? But then again, the
> > > > kernel docs explicitly say that the ordering of obj-y goals in kbuild is
> > > > significant in some instances [1]:
> > >
> > > Yes, it matters, you can not change it.  If you do, systems will break.
> > > It is the only way we have of properly ordering our init calls within
> > > the same "level".
> >
> > Ah, right it was the initcall ordering. Thanks for the reminder.
> >
> > (There's a joke in there similar to the use of regexes to solve a
> > problem resulting in two new problems; initcalls have levels for
> > ordering, but we still have (unexpressed) dependencies between calls
> > of the same level; brittle!).
>
> No, the dependencies are explicitly expressed with the linker order.  So

I don't consider that "explicit."

The link order of object files does not express what symbols (if any)
are initcalls which are dependent on other symbols/initcalls from
which object file.

> it's not brittle, but rather very deterministic.

Brittle != non-deterministic.

We now have implicit dependencies between some init calls, but not all.

Given two initcalls, are you confident that you could tell which must
run before the other, if there is even such a dependency?

It prevents us from reordering symbol layout for performance (or
security via FGKASLR), safely.  If such dependencies were *explicit*,
we could do so safely since we'd have information about which
initcalls are dependencies or not.

The implicit nature of such dependencies is thus what I would consider brittle.

Hopefully initcall ordering related changes isn't the root cause of
the boot failure reported here, lest that lend more evidence to my
claim.

>
> When linker order didn't work for all sorts of things, we added
> different levels, but due to the huge number of init calls, of course
> can not give each one their own level.
>
> It's always been this way with Linux, nothing new here at all :)

:^)

>
> thanks,
>
> greg k-h



-- 
Thanks,
~Nick Desaulniers

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

[Shreeya Patel]

Hi Nick and Masahiro,

On 23/05/23 01:22, Nick Desaulniers wrote:
> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>>> It could be; if the link order was changed, it's possible that this
>>>> target may be hitting something along the lines of:
>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
>>>> initialization order fiasco"
>>>>
>>>> I'm struggling to think of how this appears in C codebases, but I
>>>> swear years ago I had a discussion with GKH (maybe?) about this. I
>>>> think I was playing with converting Kbuild to use Ninja rather than
>>>> Make; the resulting kernel image wouldn't boot because I had modified
>>>> the order the object files were linked in.  If you were to randomly
>>>> shuffle the object files in the kernel, I recall some hazard that may
>>>> prevent boot.
>>> I thought that was specifically a C++ problem? But then again, the
>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
>>> significant in some instances [1]:
>> Yes, it matters, you can not change it.  If you do, systems will break.
>> It is the only way we have of properly ordering our init calls within
>> the same "level".
> Ah, right it was the initcall ordering. Thanks for the reminder.
>
> (There's a joke in there similar to the use of regexes to solve a
> problem resulting in two new problems; initcalls have levels for
> ordering, but we still have (unexpressed) dependencies between calls
> of the same level; brittle!).
>
> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>
> Ricardo,
> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
> mentions that there's a kernel command line param `initcall_debug`.
> Perhaps that can be used to see if
> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
> ordering, resulting in a config that cannot boot?


Here are the links to Lava jobs ran with initcall_debug added to the 
kernel command line.

1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
https://lava.collabora.dev/scheduler/job/10417706
<https://lava.collabora.dev/scheduler/job/10417706>

2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926
https://lava.collabora.dev/scheduler/job/10418012 
<https://lava.collabora.dev/scheduler/job/10418012>


Thanks,
Shreeya Patel

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

[Nick Desaulniers]

On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
<shreeya.patel@collabora.com> wrote:
>
> Hi Nick and Masahiro,
>
> On 23/05/23 01:22, Nick Desaulniers wrote:
> > On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
> >>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >>>> It could be; if the link order was changed, it's possible that this
> >>>> target may be hitting something along the lines of:
> >>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> >>>> initialization order fiasco"
> >>>>
> >>>> I'm struggling to think of how this appears in C codebases, but I
> >>>> swear years ago I had a discussion with GKH (maybe?) about this. I
> >>>> think I was playing with converting Kbuild to use Ninja rather than
> >>>> Make; the resulting kernel image wouldn't boot because I had modified
> >>>> the order the object files were linked in.  If you were to randomly
> >>>> shuffle the object files in the kernel, I recall some hazard that may
> >>>> prevent boot.
> >>> I thought that was specifically a C++ problem? But then again, the
> >>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
> >>> significant in some instances [1]:
> >> Yes, it matters, you can not change it.  If you do, systems will break.
> >> It is the only way we have of properly ordering our init calls within
> >> the same "level".
> > Ah, right it was the initcall ordering. Thanks for the reminder.
> >
> > (There's a joke in there similar to the use of regexes to solve a
> > problem resulting in two new problems; initcalls have levels for
> > ordering, but we still have (unexpressed) dependencies between calls
> > of the same level; brittle!).
> >
> > +Maksim, since that might be relevant info for the BOLT+Kernel work.
> >
> > Ricardo,
> > https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
> > mentions that there's a kernel command line param `initcall_debug`.
> > Perhaps that can be used to see if
> > 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
> > ordering, resulting in a config that cannot boot?
>
>
> Here are the links to Lava jobs ran with initcall_debug added to the
> kernel command line.
>
> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
> https://lava.collabora.dev/scheduler/job/10417706
> <https://lava.collabora.dev/scheduler/job/10417706>
>
> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926
> https://lava.collabora.dev/scheduler/job/10418012
> <https://lava.collabora.dev/scheduler/job/10418012>

Thanks!

Yeah, I can see a diff in the initcall ordering as a result of
commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")

https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4

Not just different orderings, but some initcalls seem unique to the
before vs. after, which is troubling. (example init_events and
init_fs_sysctls respectively)

That isn't conclusive evidence that changes to initcall ordering are
to blame, but I suspect confirming that precisely to be very very time
consuming.

Masahiro, what are your thoughts on reverting 5750121ae738? There are
conflicts in Kbuild and Makefile when reverting 5750121ae738 on
mainline.

>
>
> Thanks,
> Shreeya Patel
>


-- 
Thanks,
~Nick Desaulniers

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

[Shreeya Patel]

Hi Masahiro,


On 24/05/23 02:57, Nick Desaulniers wrote:
> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
> <shreeya.patel@collabora.com> wrote:
>> Hi Nick and Masahiro,
>>
>> On 23/05/23 01:22, Nick Desaulniers wrote:
>>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>>>>> It could be; if the link order was changed, it's possible that this
>>>>>> target may be hitting something along the lines of:
>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
>>>>>> initialization order fiasco"
>>>>>>
>>>>>> I'm struggling to think of how this appears in C codebases, but I
>>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I
>>>>>> think I was playing with converting Kbuild to use Ninja rather than
>>>>>> Make; the resulting kernel image wouldn't boot because I had modified
>>>>>> the order the object files were linked in.  If you were to randomly
>>>>>> shuffle the object files in the kernel, I recall some hazard that may
>>>>>> prevent boot.
>>>>> I thought that was specifically a C++ problem? But then again, the
>>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
>>>>> significant in some instances [1]:
>>>> Yes, it matters, you can not change it.  If you do, systems will break.
>>>> It is the only way we have of properly ordering our init calls within
>>>> the same "level".
>>> Ah, right it was the initcall ordering. Thanks for the reminder.
>>>
>>> (There's a joke in there similar to the use of regexes to solve a
>>> problem resulting in two new problems; initcalls have levels for
>>> ordering, but we still have (unexpressed) dependencies between calls
>>> of the same level; brittle!).
>>>
>>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>>>
>>> Ricardo,
>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
>>> mentions that there's a kernel command line param `initcall_debug`.
>>> Perhaps that can be used to see if
>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
>>> ordering, resulting in a config that cannot boot?
>>
>> Here are the links to Lava jobs ran with initcall_debug added to the
>> kernel command line.
>>
>> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
>> https://lava.collabora.dev/scheduler/job/10417706
>> <https://lava.collabora.dev/scheduler/job/10417706>
>>
>> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926
>> https://lava.collabora.dev/scheduler/job/10418012
>> <https://lava.collabora.dev/scheduler/job/10418012>
> Thanks!
>
> Yeah, I can see a diff in the initcall ordering as a result of
> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
>
> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
>
> Not just different orderings, but some initcalls seem unique to the
> before vs. after, which is troubling. (example init_events and
> init_fs_sysctls respectively)
>
> That isn't conclusive evidence that changes to initcall ordering are
> to blame, but I suspect confirming that precisely to be very very time
> consuming.
>
> Masahiro, what are your thoughts on reverting 5750121ae738? There are
> conflicts in Kbuild and Makefile when reverting 5750121ae738 on
> mainline.

I'm not sure if you followed the conversation but we are still seeing 
this regression with the latest kernel builds and would like to know if 
you plan to revert 5750121ae738?


Thanks,
Shreeya Patel

>>
>> Thanks,
>> Shreeya Patel
>>
>

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

[Masahiro Yamada]

On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
<shreeya.patel@collabora.com> wrote:
>
> Hi Masahiro,
>
>
> On 24/05/23 02:57, Nick Desaulniers wrote:
> > On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
> > <shreeya.patel@collabora.com> wrote:
> >> Hi Nick and Masahiro,
> >>
> >> On 23/05/23 01:22, Nick Desaulniers wrote:
> >>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
> >>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >>>>>> It could be; if the link order was changed, it's possible that this
> >>>>>> target may be hitting something along the lines of:
> >>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
> >>>>>> initialization order fiasco"
> >>>>>>
> >>>>>> I'm struggling to think of how this appears in C codebases, but I
> >>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I
> >>>>>> think I was playing with converting Kbuild to use Ninja rather than
> >>>>>> Make; the resulting kernel image wouldn't boot because I had modified
> >>>>>> the order the object files were linked in.  If you were to randomly
> >>>>>> shuffle the object files in the kernel, I recall some hazard that may
> >>>>>> prevent boot.
> >>>>> I thought that was specifically a C++ problem? But then again, the
> >>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
> >>>>> significant in some instances [1]:
> >>>> Yes, it matters, you can not change it.  If you do, systems will break.
> >>>> It is the only way we have of properly ordering our init calls within
> >>>> the same "level".
> >>> Ah, right it was the initcall ordering. Thanks for the reminder.
> >>>
> >>> (There's a joke in there similar to the use of regexes to solve a
> >>> problem resulting in two new problems; initcalls have levels for
> >>> ordering, but we still have (unexpressed) dependencies between calls
> >>> of the same level; brittle!).
> >>>
> >>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
> >>>
> >>> Ricardo,
> >>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
> >>> mentions that there's a kernel command line param `initcall_debug`.
> >>> Perhaps that can be used to see if
> >>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
> >>> ordering, resulting in a config that cannot boot?
> >>
> >> Here are the links to Lava jobs ran with initcall_debug added to the
> >> kernel command line.
> >>
> >> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
> >> https://lava.collabora.dev/scheduler/job/10417706
> >> <https://lava.collabora.dev/scheduler/job/10417706>
> >>
> >> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926
> >> https://lava.collabora.dev/scheduler/job/10418012
> >> <https://lava.collabora.dev/scheduler/job/10418012>
> > Thanks!
> >
> > Yeah, I can see a diff in the initcall ordering as a result of
> > commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
> >
> > https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
> >
> > Not just different orderings, but some initcalls seem unique to the
> > before vs. after, which is troubling. (example init_events and
> > init_fs_sysctls respectively)
> >
> > That isn't conclusive evidence that changes to initcall ordering are
> > to blame, but I suspect confirming that precisely to be very very time
> > consuming.
> >
> > Masahiro, what are your thoughts on reverting 5750121ae738? There are
> > conflicts in Kbuild and Makefile when reverting 5750121ae738 on
> > mainline.
>
> I'm not sure if you followed the conversation but we are still seeing
> this regression with the latest kernel builds and would like to know if
> you plan to revert 5750121ae738?


Reverting 5750121ae738 does not solve the issue
because the issue happens even before 5750121ae738.
multi_v7_defconfig + debug.config + CONFIG_MODULES=n
fails to boot in the same way.

The revert would hide the issue on a particular build setup.


I submitted a patch to more pin-point the issue.
Let's see how it goes.
https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t


(BTW, the initcall order is unrelated)





>
>
> Thanks,
> Shreeya Patel
>
> >>
> >> Thanks,
> >> Shreeya Patel
> >>
> >

--
Best Regards
Masahiro Yamada

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

[Linux regression tracking (Thorsten Leemhuis)]

Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.

Shreeya Patel, Masahiro Yamada: what's the status of this? Was any
progress made to address this? Or is this maybe (accidentally?) fixed
with 6.5-rc1?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

On 20.06.23 06:19, Masahiro Yamada wrote:
> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
> <shreeya.patel@collabora.com> wrote:
>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>> <shreeya.patel@collabora.com> wrote:
>>>> Hi Nick and Masahiro,
>>>>
>>>> On 23/05/23 01:22, Nick Desaulniers wrote:
>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>>>>>>> It could be; if the link order was changed, it's possible that this
>>>>>>>> target may be hitting something along the lines of:
>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
>>>>>>>> initialization order fiasco"
>>>>>>>>
>>>>>>>> I'm struggling to think of how this appears in C codebases, but I
>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I
>>>>>>>> think I was playing with converting Kbuild to use Ninja rather than
>>>>>>>> Make; the resulting kernel image wouldn't boot because I had modified
>>>>>>>> the order the object files were linked in.  If you were to randomly
>>>>>>>> shuffle the object files in the kernel, I recall some hazard that may
>>>>>>>> prevent boot.
>>>>>>> I thought that was specifically a C++ problem? But then again, the
>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
>>>>>>> significant in some instances [1]:
>>>>>> Yes, it matters, you can not change it.  If you do, systems will break.
>>>>>> It is the only way we have of properly ordering our init calls within
>>>>>> the same "level".
>>>>> Ah, right it was the initcall ordering. Thanks for the reminder.
>>>>>
>>>>> (There's a joke in there similar to the use of regexes to solve a
>>>>> problem resulting in two new problems; initcalls have levels for
>>>>> ordering, but we still have (unexpressed) dependencies between calls
>>>>> of the same level; brittle!).
>>>>>
>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>>>>>
>>>>> Ricardo,
>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
>>>>> mentions that there's a kernel command line param `initcall_debug`.
>>>>> Perhaps that can be used to see if
>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
>>>>> ordering, resulting in a config that cannot boot?
>>>>
>>>> Here are the links to Lava jobs ran with initcall_debug added to the
>>>> kernel command line.
>>>>
>>>> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
>>>> https://lava.collabora.dev/scheduler/job/10417706
>>>> <https://lava.collabora.dev/scheduler/job/10417706>
>>>>
>>>> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926
>>>> https://lava.collabora.dev/scheduler/job/10418012
>>>> <https://lava.collabora.dev/scheduler/job/10418012>
>>> Thanks!
>>>
>>> Yeah, I can see a diff in the initcall ordering as a result of
>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
>>>
>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
>>>
>>> Not just different orderings, but some initcalls seem unique to the
>>> before vs. after, which is troubling. (example init_events and
>>> init_fs_sysctls respectively)
>>>
>>> That isn't conclusive evidence that changes to initcall ordering are
>>> to blame, but I suspect confirming that precisely to be very very time
>>> consuming.
>>>
>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are
>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on
>>> mainline.
>>
>> I'm not sure if you followed the conversation but we are still seeing
>> this regression with the latest kernel builds and would like to know if
>> you plan to revert 5750121ae738?
> 
> 
> Reverting 5750121ae738 does not solve the issue
> because the issue happens even before 5750121ae738.
> multi_v7_defconfig + debug.config + CONFIG_MODULES=n
> fails to boot in the same way.
> 
> The revert would hide the issue on a particular build setup.
> 
> 
> I submitted a patch to more pin-point the issue.
> Let's see how it goes.
> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
> 
> 
> (BTW, the initcall order is unrelated)
> 
> 
> 
> 
> 
>>
>>
>> Thanks,
>> Shreeya Patel
>>
>>>>
>>>> Thanks,
>>>> Shreeya Patel
>>>>
>>>
> 
> --
> Best Regards
> Masahiro Yamada
> 
> 

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

[Shreeya Patel]

On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> for once, to make this easily accessible to everyone.
>
> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any
> progress made to address this? Or is this maybe (accidentally?) fixed
> with 6.5-rc1?

Hi Thorsten,

I still see the regression happening so it doesn't seem to be fixed.
https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/

Masahiro had submitted a fix for this issue here.

https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t

But I don't see any movement there. Masahiro, are you planning to send a 
v2 for it?


Thanks,
Shreeya Patel

> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
> On 20.06.23 06:19, Masahiro Yamada wrote:
>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
>> <shreeya.patel@collabora.com> wrote:
>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>> <shreeya.patel@collabora.com> wrote:
>>>>> Hi Nick and Masahiro,
>>>>>
>>>>> On 23/05/23 01:22, Nick Desaulniers wrote:
>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>>>>>>>> It could be; if the link order was changed, it's possible that this
>>>>>>>>> target may be hitting something along the lines of:
>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the "static
>>>>>>>>> initialization order fiasco"
>>>>>>>>>
>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I
>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about this. I
>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather than
>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had modified
>>>>>>>>> the order the object files were linked in.  If you were to randomly
>>>>>>>>> shuffle the object files in the kernel, I recall some hazard that may
>>>>>>>>> prevent boot.
>>>>>>>> I thought that was specifically a C++ problem? But then again, the
>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in kbuild is
>>>>>>>> significant in some instances [1]:
>>>>>>> Yes, it matters, you can not change it.  If you do, systems will break.
>>>>>>> It is the only way we have of properly ordering our init calls within
>>>>>>> the same "level".
>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder.
>>>>>>
>>>>>> (There's a joke in there similar to the use of regexes to solve a
>>>>>> problem resulting in two new problems; initcalls have levels for
>>>>>> ordering, but we still have (unexpressed) dependencies between calls
>>>>>> of the same level; brittle!).
>>>>>>
>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>>>>>>
>>>>>> Ricardo,
>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
>>>>>> mentions that there's a kernel command line param `initcall_debug`.
>>>>>> Perhaps that can be used to see if
>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
>>>>>> ordering, resulting in a config that cannot boot?
>>>>> Here are the links to Lava jobs ran with initcall_debug added to the
>>>>> kernel command line.
>>>>>
>>>>> 1. Where regression happens (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
>>>>> https://lava.collabora.dev/scheduler/job/10417706
>>>>> <https://lava.collabora.dev/scheduler/job/10417706>
>>>>>
>>>>> 2. With a revert of the commit 5750121ae7382ebac8d47ce6d68012d6cd1d7926
>>>>> https://lava.collabora.dev/scheduler/job/10418012
>>>>> <https://lava.collabora.dev/scheduler/job/10418012>
>>>> Thanks!
>>>>
>>>> Yeah, I can see a diff in the initcall ordering as a result of
>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
>>>>
>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
>>>>
>>>> Not just different orderings, but some initcalls seem unique to the
>>>> before vs. after, which is troubling. (example init_events and
>>>> init_fs_sysctls respectively)
>>>>
>>>> That isn't conclusive evidence that changes to initcall ordering are
>>>> to blame, but I suspect confirming that precisely to be very very time
>>>> consuming.
>>>>
>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are
>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on
>>>> mainline.
>>> I'm not sure if you followed the conversation but we are still seeing
>>> this regression with the latest kernel builds and would like to know if
>>> you plan to revert 5750121ae738?
>>
>> Reverting 5750121ae738 does not solve the issue
>> because the issue happens even before 5750121ae738.
>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n
>> fails to boot in the same way.
>>
>> The revert would hide the issue on a particular build setup.
>>
>>
>> I submitted a patch to more pin-point the issue.
>> Let's see how it goes.
>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
>>
>>
>> (BTW, the initcall order is unrelated)
>>
>>
>>
>>
>>
>>>
>>> Thanks,
>>> Shreeya Patel
>>>
>>>>> Thanks,
>>>>> Shreeya Patel
>>>>>
>> --
>> Best Regards
>> Masahiro Yamada
>>
>>

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

[Linux regression tracking (Thorsten Leemhuis)]

On 11.07.23 13:16, Shreeya Patel wrote:
> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote:
>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
>> for once, to make this easily accessible to everyone.
>>
>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any
>> progress made to address this? Or is this maybe (accidentally?) fixed
>> with 6.5-rc1?
> 
> I still see the regression happening so it doesn't seem to be fixed.
> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/
> 
> Masahiro had submitted a fix for this issue here.
> 
> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
> 
> But I don't see any movement there. Masahiro, are you planning to send a
> v2 for it?

That was weeks ago and we didn't get a answer. :-/ Was this fixed in
between? Doesn't look like it from here, but I might be missing something.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

>> On 20.06.23 06:19, Masahiro Yamada wrote:
>>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
>>> <shreeya.patel@collabora.com> wrote:
>>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>>> <shreeya.patel@collabora.com> wrote:
>>>>>> Hi Nick and Masahiro,
>>>>>>
>>>>>> On 23/05/23 01:22, Nick Desaulniers wrote:
>>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH
>>>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers
>>>>>>>>> <ndesaulniers@google.com> wrote:
>>>>>>>>>> It could be; if the link order was changed, it's possible that
>>>>>>>>>> this
>>>>>>>>>> target may be hitting something along the lines of:
>>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the
>>>>>>>>>> "static
>>>>>>>>>> initialization order fiasco"
>>>>>>>>>>
>>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I
>>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about
>>>>>>>>>> this. I
>>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather
>>>>>>>>>> than
>>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had
>>>>>>>>>> modified
>>>>>>>>>> the order the object files were linked in.  If you were to
>>>>>>>>>> randomly
>>>>>>>>>> shuffle the object files in the kernel, I recall some hazard
>>>>>>>>>> that may
>>>>>>>>>> prevent boot.
>>>>>>>>> I thought that was specifically a C++ problem? But then again, the
>>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in
>>>>>>>>> kbuild is
>>>>>>>>> significant in some instances [1]:
>>>>>>>> Yes, it matters, you can not change it.  If you do, systems will
>>>>>>>> break.
>>>>>>>> It is the only way we have of properly ordering our init calls
>>>>>>>> within
>>>>>>>> the same "level".
>>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder.
>>>>>>>
>>>>>>> (There's a joke in there similar to the use of regexes to solve a
>>>>>>> problem resulting in two new problems; initcalls have levels for
>>>>>>> ordering, but we still have (unexpressed) dependencies between calls
>>>>>>> of the same level; brittle!).
>>>>>>>
>>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>>>>>>>
>>>>>>> Ricardo,
>>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
>>>>>>> mentions that there's a kernel command line param `initcall_debug`.
>>>>>>> Perhaps that can be used to see if
>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
>>>>>>> ordering, resulting in a config that cannot boot?
>>>>>> Here are the links to Lava jobs ran with initcall_debug added to the
>>>>>> kernel command line.
>>>>>>
>>>>>> 1. Where regression happens
>>>>>> (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
>>>>>> https://lava.collabora.dev/scheduler/job/10417706
>>>>>> <https://lava.collabora.dev/scheduler/job/10417706>
>>>>>>
>>>>>> 2. With a revert of the commit
>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926
>>>>>> https://lava.collabora.dev/scheduler/job/10418012
>>>>>> <https://lava.collabora.dev/scheduler/job/10418012>
>>>>> Thanks!
>>>>>
>>>>> Yeah, I can see a diff in the initcall ordering as a result of
>>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
>>>>>
>>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
>>>>>
>>>>> Not just different orderings, but some initcalls seem unique to the
>>>>> before vs. after, which is troubling. (example init_events and
>>>>> init_fs_sysctls respectively)
>>>>>
>>>>> That isn't conclusive evidence that changes to initcall ordering are
>>>>> to blame, but I suspect confirming that precisely to be very very time
>>>>> consuming.
>>>>>
>>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are
>>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on
>>>>> mainline.
>>>> I'm not sure if you followed the conversation but we are still seeing
>>>> this regression with the latest kernel builds and would like to know if
>>>> you plan to revert 5750121ae738?
>>>
>>> Reverting 5750121ae738 does not solve the issue
>>> because the issue happens even before 5750121ae738.
>>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n
>>> fails to boot in the same way.
>>>
>>> The revert would hide the issue on a particular build setup.
>>>
>>>
>>> I submitted a patch to more pin-point the issue.
>>> Let's see how it goes.
>>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
>>>
>>>
>>> (BTW, the initcall order is unrelated)
>>>
>>>
>>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> Shreeya Patel
>>>>
>>>>>> Thanks,
>>>>>> Shreeya Patel
>>>>>>
>>> -- 
>>> Best Regards
>>> Masahiro Yamada
>>>
>>>
> 
> 

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

[Thorsten Leemhuis]

On 29.08.23 13:28, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 11.07.23 13:16, Shreeya Patel wrote:
>> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
>>> for once, to make this easily accessible to everyone.
>>>
>>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any
>>> progress made to address this? Or is this maybe (accidentally?) fixed
>>> with 6.5-rc1?
>>
>> I still see the regression happening so it doesn't seem to be fixed.
>> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/
>>
>> Masahiro had submitted a fix for this issue here.
>>
>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
>>
>> But I don't see any movement there. Masahiro, are you planning to send a
>> v2 for it?
> 
> That was weeks ago and we didn't get a answer. :-/ Was this fixed in
> between? Doesn't look like it from here, but I might be missing something.

Still no reply. :-/

Shreeya Patel, does the problem still happen with 6.6-rc1 and do you
still want to see it fixed? In that case our only option to get things
rolling again might be to involve Linus, unless someone in the CC list
has a idea to resolve this. Might also be good to know if reverting the
culprit fixes the problem.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

>>> On 20.06.23 06:19, Masahiro Yamada wrote:
>>>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
>>>> <shreeya.patel@collabora.com> wrote:
>>>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>>>> <shreeya.patel@collabora.com> wrote:
>>>>>>> Hi Nick and Masahiro,
>>>>>>>
>>>>>>> On 23/05/23 01:22, Nick Desaulniers wrote:
>>>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH
>>>>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers
>>>>>>>>>> <ndesaulniers@google.com> wrote:
>>>>>>>>>>> It could be; if the link order was changed, it's possible that
>>>>>>>>>>> this
>>>>>>>>>>> target may be hitting something along the lines of:
>>>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the
>>>>>>>>>>> "static
>>>>>>>>>>> initialization order fiasco"
>>>>>>>>>>>
>>>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I
>>>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about
>>>>>>>>>>> this. I
>>>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather
>>>>>>>>>>> than
>>>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had
>>>>>>>>>>> modified
>>>>>>>>>>> the order the object files were linked in.  If you were to
>>>>>>>>>>> randomly
>>>>>>>>>>> shuffle the object files in the kernel, I recall some hazard
>>>>>>>>>>> that may
>>>>>>>>>>> prevent boot.
>>>>>>>>>> I thought that was specifically a C++ problem? But then again, the
>>>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in
>>>>>>>>>> kbuild is
>>>>>>>>>> significant in some instances [1]:
>>>>>>>>> Yes, it matters, you can not change it.  If you do, systems will
>>>>>>>>> break.
>>>>>>>>> It is the only way we have of properly ordering our init calls
>>>>>>>>> within
>>>>>>>>> the same "level".
>>>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder.
>>>>>>>>
>>>>>>>> (There's a joke in there similar to the use of regexes to solve a
>>>>>>>> problem resulting in two new problems; initcalls have levels for
>>>>>>>> ordering, but we still have (unexpressed) dependencies between calls
>>>>>>>> of the same level; brittle!).
>>>>>>>>
>>>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>>>>>>>>
>>>>>>>> Ricardo,
>>>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
>>>>>>>> mentions that there's a kernel command line param `initcall_debug`.
>>>>>>>> Perhaps that can be used to see if
>>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
>>>>>>>> ordering, resulting in a config that cannot boot?
>>>>>>> Here are the links to Lava jobs ran with initcall_debug added to the
>>>>>>> kernel command line.
>>>>>>>
>>>>>>> 1. Where regression happens
>>>>>>> (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
>>>>>>> https://lava.collabora.dev/scheduler/job/10417706
>>>>>>> <https://lava.collabora.dev/scheduler/job/10417706>
>>>>>>>
>>>>>>> 2. With a revert of the commit
>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926
>>>>>>> https://lava.collabora.dev/scheduler/job/10418012
>>>>>>> <https://lava.collabora.dev/scheduler/job/10418012>
>>>>>> Thanks!
>>>>>>
>>>>>> Yeah, I can see a diff in the initcall ordering as a result of
>>>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
>>>>>>
>>>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
>>>>>>
>>>>>> Not just different orderings, but some initcalls seem unique to the
>>>>>> before vs. after, which is troubling. (example init_events and
>>>>>> init_fs_sysctls respectively)
>>>>>>
>>>>>> That isn't conclusive evidence that changes to initcall ordering are
>>>>>> to blame, but I suspect confirming that precisely to be very very time
>>>>>> consuming.
>>>>>>
>>>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are
>>>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on
>>>>>> mainline.
>>>>> I'm not sure if you followed the conversation but we are still seeing
>>>>> this regression with the latest kernel builds and would like to know if
>>>>> you plan to revert 5750121ae738?
>>>>
>>>> Reverting 5750121ae738 does not solve the issue
>>>> because the issue happens even before 5750121ae738.
>>>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n
>>>> fails to boot in the same way.
>>>>
>>>> The revert would hide the issue on a particular build setup.
>>>>
>>>>
>>>> I submitted a patch to more pin-point the issue.
>>>> Let's see how it goes.
>>>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
>>>>
>>>>
>>>> (BTW, the initcall order is unrelated)
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> Shreeya Patel
>>>>>
>>>>>>> Thanks,
>>>>>>> Shreeya Patel
>>>>>>>
>>>> -- 
>>>> Best Regards
>>>> Masahiro Yamada
>>>>
>>>>
>>
>>

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

[Shreeya Patel]

On 11/09/23 15:35, Thorsten Leemhuis wrote:

Hi Thorsten,

> On 29.08.23 13:28, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 11.07.23 13:16, Shreeya Patel wrote:
>>> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
>>>> for once, to make this easily accessible to everyone.
>>>>
>>>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any
>>>> progress made to address this? Or is this maybe (accidentally?) fixed
>>>> with 6.5-rc1?
>>> I still see the regression happening so it doesn't seem to be fixed.
>>> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/
>>>
>>> Masahiro had submitted a fix for this issue here.
>>>
>>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
>>>
>>> But I don't see any movement there. Masahiro, are you planning to send a
>>> v2 for it?
>> That was weeks ago and we didn't get a answer. :-/ Was this fixed in
>> between? Doesn't look like it from here, but I might be missing something.
> Still no reply. :-/
>
> Shreeya Patel, does the problem still happen with 6.6-rc1 and do you
> still want to see it fixed? In that case our only option to get things
> rolling again might be to involve Linus, unless someone in the CC list
> has a idea to resolve this. Might also be good to know if reverting the
> culprit fixes the problem.


I don't see this issue happening on 6.6-rc1 kernel and it was only last 
seen in 6.5 kernel.
But there was no fix added to Kbuild in the meantime so not sure which 
commit really fixed this issue.

For now we can mark this as resolved and I'll keep an eye on the future 
test results to see if this pops up again.


Thanks,
Shreeya Patel

#regzbot resolve: Fixed in 6.6-rc1 kernel, fix commit is unknown.
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
>>>> On 20.06.23 06:19, Masahiro Yamada wrote:
>>>>> On Mon, Jun 12, 2023 at 7:10 PM Shreeya Patel
>>>>> <shreeya.patel@collabora.com> wrote:
>>>>>> On 24/05/23 02:57, Nick Desaulniers wrote:
>>>>>>> On Tue, May 23, 2023 at 3:27 AM Shreeya Patel
>>>>>>> <shreeya.patel@collabora.com> wrote:
>>>>>>>> Hi Nick and Masahiro,
>>>>>>>>
>>>>>>>> On 23/05/23 01:22, Nick Desaulniers wrote:
>>>>>>>>> On Mon, May 22, 2023 at 9:52 AM Greg KH
>>>>>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>>>>>> On Mon, May 22, 2023 at 12:09:34PM +0200, Ricardo Cañuelo wrote:
>>>>>>>>>>> On vie, may 19 2023 at 08:57:24, Nick Desaulniers
>>>>>>>>>>> <ndesaulniers@google.com> wrote:
>>>>>>>>>>>> It could be; if the link order was changed, it's possible that
>>>>>>>>>>>> this
>>>>>>>>>>>> target may be hitting something along the lines of:
>>>>>>>>>>>> https://isocpp.org/wiki/faq/ctors#static-init-order i.e. the
>>>>>>>>>>>> "static
>>>>>>>>>>>> initialization order fiasco"
>>>>>>>>>>>>
>>>>>>>>>>>> I'm struggling to think of how this appears in C codebases, but I
>>>>>>>>>>>> swear years ago I had a discussion with GKH (maybe?) about
>>>>>>>>>>>> this. I
>>>>>>>>>>>> think I was playing with converting Kbuild to use Ninja rather
>>>>>>>>>>>> than
>>>>>>>>>>>> Make; the resulting kernel image wouldn't boot because I had
>>>>>>>>>>>> modified
>>>>>>>>>>>> the order the object files were linked in.  If you were to
>>>>>>>>>>>> randomly
>>>>>>>>>>>> shuffle the object files in the kernel, I recall some hazard
>>>>>>>>>>>> that may
>>>>>>>>>>>> prevent boot.
>>>>>>>>>>> I thought that was specifically a C++ problem? But then again, the
>>>>>>>>>>> kernel docs explicitly say that the ordering of obj-y goals in
>>>>>>>>>>> kbuild is
>>>>>>>>>>> significant in some instances [1]:
>>>>>>>>>> Yes, it matters, you can not change it.  If you do, systems will
>>>>>>>>>> break.
>>>>>>>>>> It is the only way we have of properly ordering our init calls
>>>>>>>>>> within
>>>>>>>>>> the same "level".
>>>>>>>>> Ah, right it was the initcall ordering. Thanks for the reminder.
>>>>>>>>>
>>>>>>>>> (There's a joke in there similar to the use of regexes to solve a
>>>>>>>>> problem resulting in two new problems; initcalls have levels for
>>>>>>>>> ordering, but we still have (unexpressed) dependencies between calls
>>>>>>>>> of the same level; brittle!).
>>>>>>>>>
>>>>>>>>> +Maksim, since that might be relevant info for the BOLT+Kernel work.
>>>>>>>>>
>>>>>>>>> Ricardo,
>>>>>>>>> https://elinux.org/images/e/e8/2020_ELCE_initcalls_myjosserand.pdf
>>>>>>>>> mentions that there's a kernel command line param `initcall_debug`.
>>>>>>>>> Perhaps that can be used to see if
>>>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926 somehow changed initcall
>>>>>>>>> ordering, resulting in a config that cannot boot?
>>>>>>>> Here are the links to Lava jobs ran with initcall_debug added to the
>>>>>>>> kernel command line.
>>>>>>>>
>>>>>>>> 1. Where regression happens
>>>>>>>> (5750121ae7382ebac8d47ce6d68012d6cd1d7926)
>>>>>>>> https://lava.collabora.dev/scheduler/job/10417706
>>>>>>>> <https://lava.collabora.dev/scheduler/job/10417706>
>>>>>>>>
>>>>>>>> 2. With a revert of the commit
>>>>>>>> 5750121ae7382ebac8d47ce6d68012d6cd1d7926
>>>>>>>> https://lava.collabora.dev/scheduler/job/10418012
>>>>>>>> <https://lava.collabora.dev/scheduler/job/10418012>
>>>>>>> Thanks!
>>>>>>>
>>>>>>> Yeah, I can see a diff in the initcall ordering as a result of
>>>>>>> commit 5750121ae738 ("kbuild: list sub-directories in ./Kbuild")
>>>>>>>
>>>>>>> https://gist.github.com/nickdesaulniers/c09db256e42ad06b90842a4bb85cc0f4
>>>>>>>
>>>>>>> Not just different orderings, but some initcalls seem unique to the
>>>>>>> before vs. after, which is troubling. (example init_events and
>>>>>>> init_fs_sysctls respectively)
>>>>>>>
>>>>>>> That isn't conclusive evidence that changes to initcall ordering are
>>>>>>> to blame, but I suspect confirming that precisely to be very very time
>>>>>>> consuming.
>>>>>>>
>>>>>>> Masahiro, what are your thoughts on reverting 5750121ae738? There are
>>>>>>> conflicts in Kbuild and Makefile when reverting 5750121ae738 on
>>>>>>> mainline.
>>>>>> I'm not sure if you followed the conversation but we are still seeing
>>>>>> this regression with the latest kernel builds and would like to know if
>>>>>> you plan to revert 5750121ae738?
>>>>> Reverting 5750121ae738 does not solve the issue
>>>>> because the issue happens even before 5750121ae738.
>>>>> multi_v7_defconfig + debug.config + CONFIG_MODULES=n
>>>>> fails to boot in the same way.
>>>>>
>>>>> The revert would hide the issue on a particular build setup.
>>>>>
>>>>>
>>>>> I submitted a patch to more pin-point the issue.
>>>>> Let's see how it goes.
>>>>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
>>>>>
>>>>>
>>>>> (BTW, the initcall order is unrelated)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Shreeya Patel
>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Shreeya Patel
>>>>>>>>
>>>>> -- 
>>>>> Best Regards
>>>>> Masahiro Yamada
>>>>>
>>>>>
>>>

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

[Masahiro Yamada]

On Fri, Sep 15, 2023 at 6:33 PM Shreeya Patel
<shreeya.patel@collabora.com> wrote:
>
> On 11/09/23 15:35, Thorsten Leemhuis wrote:
>
> Hi Thorsten,
>
> > On 29.08.23 13:28, Linux regression tracking (Thorsten Leemhuis) wrote:
> >> On 11.07.23 13:16, Shreeya Patel wrote:
> >>> On 10/07/23 17:39, Linux regression tracking (Thorsten Leemhuis) wrote:
> >>>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> >>>> for once, to make this easily accessible to everyone.
> >>>>
> >>>> Shreeya Patel, Masahiro Yamada: what's the status of this? Was any
> >>>> progress made to address this? Or is this maybe (accidentally?) fixed
> >>>> with 6.5-rc1?
> >>> I still see the regression happening so it doesn't seem to be fixed.
> >>> https://linux.kernelci.org/test/case/id/64ac675a8aebf63753bb2a8c/
> >>>
> >>> Masahiro had submitted a fix for this issue here.
> >>>
> >>> https://lore.kernel.org/lkml/ZJEni98knMMkU%2Fcl@buildd.core.avm.de/T/#t
> >>>
> >>> But I don't see any movement there. Masahiro, are you planning to send a
> >>> v2 for it?
> >> That was weeks ago and we didn't get a answer. :-/ Was this fixed in
> >> between? Doesn't look like it from here, but I might be missing something.
> > Still no reply. :-/
> >
> > Shreeya Patel, does the problem still happen with 6.6-rc1 and do you
> > still want to see it fixed? In that case our only option to get things
> > rolling again might be to involve Linus, unless someone in the CC list
> > has a idea to resolve this. Might also be good to know if reverting the
> > culprit fixes the problem.
>
>
> I don't see this issue happening on 6.6-rc1 kernel and it was only last
> seen in 6.5 kernel.
> But there was no fix added to Kbuild in the meantime so not sure which
> commit really fixed this issue.
>
> For now we can mark this as resolved and I'll keep an eye on the future
> test results to see if this pops up again.
>
>
> Thanks,
> Shreeya Patel
>
> #regzbot resolve: Fixed in 6.6-rc1 kernel, fix commit is unknown.



Not fixed in 6.6-rc1.

I submitted a patch to shoot the root cause.

https://lore.kernel.org/all/20230925110023.1796789-1-masahiroy@kernel.org/


Now it is in Russell's patch tracker.




-- 
Best Regards
Masahiro Yamada