Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-30 Thread Nick Desaulniers
On Fri, Aug 30, 2019 at 2:13 PM Nick Desaulniers
 wrote:
>
> On Fri, Aug 30, 2019 at 10:28 AM Nathan Chancellor
>  wrote:
> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > index a43fc753aa53..23c2bf0fbd30 100644
> > --- a/arch/arm/Makefile
> > +++ b/arch/arm/Makefile
> > @@ -115,6 +115,10 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
> >  CFLAGS_ABI +=-funwind-tables
> >  endif
> >
> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> > +CFLAGS_ABI +=-meabi gnu

Needs a space.  `+=-`.
-- 
Thanks,
~Nick Desaulniers


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-30 Thread Nick Desaulniers
On Fri, Aug 30, 2019 at 10:28 AM Nathan Chancellor
 wrote:
>
> On Thu, Aug 29, 2019 at 01:57:35PM -0700, Nick Desaulniers wrote:
> > On Thu, Aug 29, 2019 at 1:21 PM Stefan Agner  wrote:
> > >
> > > On 2019-08-29 21:34, Nathan Chancellor wrote:
> > > > On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
> > > >> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
> > > > I will test with or without CONFIG_AEABI like Matthias asked and I will
> > > > implement your Kconfig suggestion if it passes all of my tests. The
> > > > reason that I did it this way is because I didn't want a user to end up
> > > > with a non-booting kernel since -meabi gnu works with older versions of
> > > > clang at build time, the issue happens at boot time but the Kconfig
> > > > suggestion + cc-option should fix that.
> Disabling CONFIG_AEABI does not work with clang, I get a ton of failures
> around undefined references to __aeabi_idivmod and __aeabi_uidivmod. I
> don't think this is worth looking into given that CONFIG_AEABI is not
> selectable when CONFIG_CPU_V6 or CONFIG_CPU_V7 is selected, which is
> what we primarily care about.. We currently build and boot
> multi_v5_defconfig but it has CONFIG_AEABI set in it. As a result, I
> don't think we need to care about it with this patch.

The plan of record is to never support !CONFIG_AEBI (ie OABI) w/
Clang. See also my commit currently in linux-next:
ARM: 8875/1: Kconfig: default to AEABI w/ Clang
https://github.com/ClangBuiltLinux/linux/issues/482
so !AEABI is a moot point.  If we ever changed our minds, then yes we
should additionally guard on !CONFIG_AEABI, but I feel like that's a
highly unlikely scenario at this point.

>
> This diff would also solve the issue without the need for the version
> check in the Makefile, as it is the combination of -meabi gnu and -pg
> that causes the boot issue and -pg gets added when
> CONFIG_FUNCTION_TRACER is enabled. Would that be preferred? I do not
> think adding cc-option is necessary given that we know GCC universally
> does not support this flag; there is no point in adding a small penalty
> with cc-option to either compiler.
>
> Cheers,
> Nathan
>
> 
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a98c7af50bf0..440ad41e77e4 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -83,7 +83,7 @@ config ARM
> select HAVE_FAST_GUP if ARM_LPAE
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
> -   select HAVE_FUNCTION_TRACER if !XIP_KERNEL
> +   select HAVE_FUNCTION_TRACER if !XIP_KERNEL && (CC_IS_GCC || 
> CLANG_VERSION >= 10)
> select HAVE_GCC_PLUGINS
> select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || 
> CPU_V7)
> select HAVE_IDE if PCI || ISA || PCMCIA
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index a43fc753aa53..23c2bf0fbd30 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -115,6 +115,10 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>  CFLAGS_ABI +=-funwind-tables
>  endif
>
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +CFLAGS_ABI +=-meabi gnu
> +endif

Reviewed-by: Nick Desaulniers 

> +
>  # Accept old syntax despite ".syntax unified"
>  AFLAGS_NOWARN  :=$(call 
> as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
>


-- 
Thanks,
~Nick Desaulniers


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-30 Thread Nathan Chancellor
On Thu, Aug 29, 2019 at 01:57:35PM -0700, Nick Desaulniers wrote:
> On Thu, Aug 29, 2019 at 1:21 PM Stefan Agner  wrote:
> >
> > On 2019-08-29 21:34, Nathan Chancellor wrote:
> > > On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
> > >> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
> > >>  wrote:
> > >> >
> > >> > Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> > >> > with clang:
> > >> >
> > >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> > >> > softirq.c:(.text+0x504): undefined reference to `mcount'
> > >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function 
> > >> > `__local_bh_enable_ip':
> > >> > softirq.c:(.text+0x58c): undefined reference to `mcount'
> > >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> > >> > softirq.c:(.text+0x6c8): undefined reference to `mcount'
> > >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> > >> > softirq.c:(.text+0x75c): undefined reference to `mcount'
> > >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> > >> > softirq.c:(.text+0x840): undefined reference to `mcount'
> > >> > arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
> > >> > undefined references to `mcount' follow
> > >> >
> > >> > clang can emit a working mcount symbol, __gnu_mcount_nc, when
> > >> > '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> > >> > broken and caused the kernel not to boot because the calling
> > >> > convention was not correct. Now that it is fixed, add this to
> > >> > the command line when clang is 10.0.0 or newer so everything
> > >> > works properly.
> > >> >
> > >> > Link: https://github.com/ClangBuiltLinux/linux/issues/35
> > >> > Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> > >> > Link: 
> > >> > https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> > >> > Signed-off-by: Nathan Chancellor 
> > >> > ---
> > >> >  arch/arm/Makefile | 6 ++
> > >> >  1 file changed, 6 insertions(+)
> > >> >
> > >> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > >> > index c3624ca6c0bc..7b5a26a866fc 100644
> > >> > --- a/arch/arm/Makefile
> > >> > +++ b/arch/arm/Makefile
> > >> > @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
> > >> >  CFLAGS_ABI +=-funwind-tables
> > >> >  endif
> > >> >
> > >> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> > >> > +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> > >> > +CFLAGS_ABI +=-meabi gnu
> > >> > +endif
> > >> > +endif
> > >> > +
> > >>
> > >> Thanks for the patch!  I think this is one of the final issues w/ 32b
> > >> ARM configs when building w/ Clang.
> > >>
> > >> I'm not super enthused about the version check.  The flag is indeed
> > >> not recognized by GCC, but I think it would actually be more concise
> > >> with $(cc-option) and no compiler or version check.
> > >>
> > >> Further, I think that the working __gnu_mcount_nc in Clang would
> > >> better be represented as marking the arch/arm/KConfig option for
> > >> CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
> > >> than or equal to Clang 10, not conditionally adding this flag. (We
> > >> should always add the flag when supported, IMO.  __gnu_mcount_nc's
> > >> calling convention being broken is orthogonal to the choice of
> > >> __gnu_mcount_nc vs mcount, and it's the former's that should be
> > >> checked, not the latter as in this patch.
> > >
> > > I will test with or without CONFIG_AEABI like Matthias asked and I will
> > > implement your Kconfig suggestion if it passes all of my tests. The
> > > reason that I did it this way is because I didn't want a user to end up
> > > with a non-booting kernel since -meabi gnu works with older versions of
> > > clang at build time, the issue happens at boot time but the Kconfig
> > > suggestion + cc-option should fix that.
> >
> > I agree with Nathan here, I'd rather prefer the build system to fail
> > building rather than runtime error.
> >
> > If we decide we want to have it building despite it not building a
> > functional kernel, we should at least add a #warning...
> 
> Just to be clear...I was suggesting a build failure, but for
> __gnu_mcount_nc not having the correct calling convention in older
> clang releases, which is orthogonal to passing `-meabi gnu`.  This
> patch uses the __gnu_mcount_nc calling convention problem to justify a
> version check for a flag that while closely related, is not actually
> the problem, IMO.

I am not entirely sure what you mean. -meabi gnu has always produced a
buildable kernel (see Stefan's original report [1]), it just doesn't
boot and that all happens silently.

[1]: 
https://lore.kernel.org/linux-arm-kernel/35ae3d464c7b77a5fe86a732079e3...@agner.ch/

Okay, after doing some further testing...

Disabling CONFIG_AEABI does not work with clang, I get a ton of failures
around undefined references to __aeabi_idivmod and __aeabi_uidivmod. I
don't 

Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Nick Desaulniers
On Thu, Aug 29, 2019 at 1:21 PM Stefan Agner  wrote:
>
> On 2019-08-29 21:34, Nathan Chancellor wrote:
> > On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
> >> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
> >>  wrote:
> >> >
> >> > Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> >> > with clang:
> >> >
> >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> >> > softirq.c:(.text+0x504): undefined reference to `mcount'
> >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function 
> >> > `__local_bh_enable_ip':
> >> > softirq.c:(.text+0x58c): undefined reference to `mcount'
> >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> >> > softirq.c:(.text+0x6c8): undefined reference to `mcount'
> >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> >> > softirq.c:(.text+0x75c): undefined reference to `mcount'
> >> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> >> > softirq.c:(.text+0x840): undefined reference to `mcount'
> >> > arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
> >> > undefined references to `mcount' follow
> >> >
> >> > clang can emit a working mcount symbol, __gnu_mcount_nc, when
> >> > '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> >> > broken and caused the kernel not to boot because the calling
> >> > convention was not correct. Now that it is fixed, add this to
> >> > the command line when clang is 10.0.0 or newer so everything
> >> > works properly.
> >> >
> >> > Link: https://github.com/ClangBuiltLinux/linux/issues/35
> >> > Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> >> > Link: 
> >> > https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> >> > Signed-off-by: Nathan Chancellor 
> >> > ---
> >> >  arch/arm/Makefile | 6 ++
> >> >  1 file changed, 6 insertions(+)
> >> >
> >> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> >> > index c3624ca6c0bc..7b5a26a866fc 100644
> >> > --- a/arch/arm/Makefile
> >> > +++ b/arch/arm/Makefile
> >> > @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
> >> >  CFLAGS_ABI +=-funwind-tables
> >> >  endif
> >> >
> >> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> >> > +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> >> > +CFLAGS_ABI +=-meabi gnu
> >> > +endif
> >> > +endif
> >> > +
> >>
> >> Thanks for the patch!  I think this is one of the final issues w/ 32b
> >> ARM configs when building w/ Clang.
> >>
> >> I'm not super enthused about the version check.  The flag is indeed
> >> not recognized by GCC, but I think it would actually be more concise
> >> with $(cc-option) and no compiler or version check.
> >>
> >> Further, I think that the working __gnu_mcount_nc in Clang would
> >> better be represented as marking the arch/arm/KConfig option for
> >> CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
> >> than or equal to Clang 10, not conditionally adding this flag. (We
> >> should always add the flag when supported, IMO.  __gnu_mcount_nc's
> >> calling convention being broken is orthogonal to the choice of
> >> __gnu_mcount_nc vs mcount, and it's the former's that should be
> >> checked, not the latter as in this patch.
> >
> > I will test with or without CONFIG_AEABI like Matthias asked and I will
> > implement your Kconfig suggestion if it passes all of my tests. The
> > reason that I did it this way is because I didn't want a user to end up
> > with a non-booting kernel since -meabi gnu works with older versions of
> > clang at build time, the issue happens at boot time but the Kconfig
> > suggestion + cc-option should fix that.
>
> I agree with Nathan here, I'd rather prefer the build system to fail
> building rather than runtime error.
>
> If we decide we want to have it building despite it not building a
> functional kernel, we should at least add a #warning...

Just to be clear...I was suggesting a build failure, but for
__gnu_mcount_nc not having the correct calling convention in older
clang releases, which is orthogonal to passing `-meabi gnu`.  This
patch uses the __gnu_mcount_nc calling convention problem to justify a
version check for a flag that while closely related, is not actually
the problem, IMO.
-- 
Thanks,
~Nick Desaulniers


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Stefan Agner
On 2019-08-29 21:34, Nathan Chancellor wrote:
> On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
>> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
>>  wrote:
>> >
>> > Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
>> > with clang:
>> >
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
>> > softirq.c:(.text+0x504): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
>> > softirq.c:(.text+0x58c): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
>> > softirq.c:(.text+0x6c8): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
>> > softirq.c:(.text+0x75c): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
>> > softirq.c:(.text+0x840): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
>> > undefined references to `mcount' follow
>> >
>> > clang can emit a working mcount symbol, __gnu_mcount_nc, when
>> > '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
>> > broken and caused the kernel not to boot because the calling
>> > convention was not correct. Now that it is fixed, add this to
>> > the command line when clang is 10.0.0 or newer so everything
>> > works properly.
>> >
>> > Link: https://github.com/ClangBuiltLinux/linux/issues/35
>> > Link: https://bugs.llvm.org/show_bug.cgi?id=33845
>> > Link: 
>> > https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
>> > Signed-off-by: Nathan Chancellor 
>> > ---
>> >  arch/arm/Makefile | 6 ++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> > index c3624ca6c0bc..7b5a26a866fc 100644
>> > --- a/arch/arm/Makefile
>> > +++ b/arch/arm/Makefile
>> > @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>> >  CFLAGS_ABI +=-funwind-tables
>> >  endif
>> >
>> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
>> > +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
>> > +CFLAGS_ABI +=-meabi gnu
>> > +endif
>> > +endif
>> > +
>>
>> Thanks for the patch!  I think this is one of the final issues w/ 32b
>> ARM configs when building w/ Clang.
>>
>> I'm not super enthused about the version check.  The flag is indeed
>> not recognized by GCC, but I think it would actually be more concise
>> with $(cc-option) and no compiler or version check.
>>
>> Further, I think that the working __gnu_mcount_nc in Clang would
>> better be represented as marking the arch/arm/KConfig option for
>> CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
>> than or equal to Clang 10, not conditionally adding this flag. (We
>> should always add the flag when supported, IMO.  __gnu_mcount_nc's
>> calling convention being broken is orthogonal to the choice of
>> __gnu_mcount_nc vs mcount, and it's the former's that should be
>> checked, not the latter as in this patch.
> 
> I will test with or without CONFIG_AEABI like Matthias asked and I will
> implement your Kconfig suggestion if it passes all of my tests. The
> reason that I did it this way is because I didn't want a user to end up
> with a non-booting kernel since -meabi gnu works with older versions of
> clang at build time, the issue happens at boot time but the Kconfig
> suggestion + cc-option should fix that.

I agree with Nathan here, I'd rather prefer the build system to fail
building rather than runtime error.

If we decide we want to have it building despite it not building a
functional kernel, we should at least add a #warning...

--
Stefan

> 
> I should have a v2 out this evening.
> 
> Cheers,
> Nathan


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Nathan Chancellor
On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
>  wrote:
> >
> > Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> > with clang:
> >
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> > softirq.c:(.text+0x504): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> > softirq.c:(.text+0x58c): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> > softirq.c:(.text+0x6c8): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> > softirq.c:(.text+0x75c): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> > softirq.c:(.text+0x840): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
> > undefined references to `mcount' follow
> >
> > clang can emit a working mcount symbol, __gnu_mcount_nc, when
> > '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> > broken and caused the kernel not to boot because the calling
> > convention was not correct. Now that it is fixed, add this to
> > the command line when clang is 10.0.0 or newer so everything
> > works properly.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/35
> > Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> > Link: 
> > https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> > Signed-off-by: Nathan Chancellor 
> > ---
> >  arch/arm/Makefile | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > index c3624ca6c0bc..7b5a26a866fc 100644
> > --- a/arch/arm/Makefile
> > +++ b/arch/arm/Makefile
> > @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
> >  CFLAGS_ABI +=-funwind-tables
> >  endif
> >
> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> > +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> > +CFLAGS_ABI +=-meabi gnu
> > +endif
> > +endif
> > +
> 
> Thanks for the patch!  I think this is one of the final issues w/ 32b
> ARM configs when building w/ Clang.
> 
> I'm not super enthused about the version check.  The flag is indeed
> not recognized by GCC, but I think it would actually be more concise
> with $(cc-option) and no compiler or version check.
> 
> Further, I think that the working __gnu_mcount_nc in Clang would
> better be represented as marking the arch/arm/KConfig option for
> CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
> than or equal to Clang 10, not conditionally adding this flag. (We
> should always add the flag when supported, IMO.  __gnu_mcount_nc's
> calling convention being broken is orthogonal to the choice of
> __gnu_mcount_nc vs mcount, and it's the former's that should be
> checked, not the latter as in this patch.

I will test with or without CONFIG_AEABI like Matthias asked and I will
implement your Kconfig suggestion if it passes all of my tests. The
reason that I did it this way is because I didn't want a user to end up
with a non-booting kernel since -meabi gnu works with older versions of
clang at build time, the issue happens at boot time but the Kconfig
suggestion + cc-option should fix that.

I should have a v2 out this evening.

Cheers,
Nathan


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Nick Desaulniers
On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
 wrote:
>
> Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> with clang:
>
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> softirq.c:(.text+0x504): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> softirq.c:(.text+0x58c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> softirq.c:(.text+0x6c8): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> softirq.c:(.text+0x75c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> softirq.c:(.text+0x840): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
> undefined references to `mcount' follow
>
> clang can emit a working mcount symbol, __gnu_mcount_nc, when
> '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> broken and caused the kernel not to boot because the calling
> convention was not correct. Now that it is fixed, add this to
> the command line when clang is 10.0.0 or newer so everything
> works properly.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/35
> Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> Link: 
> https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> Signed-off-by: Nathan Chancellor 
> ---
>  arch/arm/Makefile | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index c3624ca6c0bc..7b5a26a866fc 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>  CFLAGS_ABI +=-funwind-tables
>  endif
>
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> +CFLAGS_ABI +=-meabi gnu
> +endif
> +endif
> +

Thanks for the patch!  I think this is one of the final issues w/ 32b
ARM configs when building w/ Clang.

I'm not super enthused about the version check.  The flag is indeed
not recognized by GCC, but I think it would actually be more concise
with $(cc-option) and no compiler or version check.

Further, I think that the working __gnu_mcount_nc in Clang would
better be represented as marking the arch/arm/KConfig option for
CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
than or equal to Clang 10, not conditionally adding this flag. (We
should always add the flag when supported, IMO.  __gnu_mcount_nc's
calling convention being broken is orthogonal to the choice of
__gnu_mcount_nc vs mcount, and it's the former's that should be
checked, not the latter as in this patch.

>  # Accept old syntax despite ".syntax unified"
>  AFLAGS_NOWARN  :=$(call 
> as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
>
> --
> 2.23.0
>


-- 
Thanks,
~Nick Desaulniers


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Matthias Kaehlcke
On Wed, Aug 28, 2019 at 11:26:35PM -0700, Nathan Chancellor wrote:
> Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> with clang:
> 
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> softirq.c:(.text+0x504): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> softirq.c:(.text+0x58c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> softirq.c:(.text+0x6c8): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> softirq.c:(.text+0x75c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> softirq.c:(.text+0x840): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
> undefined references to `mcount' follow
> 
> clang can emit a working mcount symbol, __gnu_mcount_nc, when
> '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> broken and caused the kernel not to boot because the calling
> convention was not correct. Now that it is fixed, add this to
> the command line when clang is 10.0.0 or newer so everything
> works properly.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/35
> Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> Link: 
> https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> Signed-off-by: Nathan Chancellor 
> ---
>  arch/arm/Makefile | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index c3624ca6c0bc..7b5a26a866fc 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>  CFLAGS_ABI   +=-funwind-tables
>  endif
>  
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> +CFLAGS_ABI   +=-meabi gnu
> +endif
> +endif

Is this also correct/needed when CONFIG_AEABI is not set?


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Stefan Agner
On 2019-08-29 08:26, Nathan Chancellor wrote:
> Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> with clang:
> 
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> softirq.c:(.text+0x504): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> softirq.c:(.text+0x58c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> softirq.c:(.text+0x6c8): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> softirq.c:(.text+0x75c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> softirq.c:(.text+0x840): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more
> undefined references to `mcount' follow
> 
> clang can emit a working mcount symbol, __gnu_mcount_nc, when
> '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> broken and caused the kernel not to boot because the calling
> convention was not correct. Now that it is fixed, add this to
> the command line when clang is 10.0.0 or newer so everything
> works properly.

Cool, finally function tracing with Clang :-)

> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/35
> Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> Link:
> https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> Signed-off-by: Nathan Chancellor 

Reviewed-by: Stefan Agner 

--
Stefan

> ---
>  arch/arm/Makefile | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index c3624ca6c0bc..7b5a26a866fc 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>  CFLAGS_ABI   +=-funwind-tables
>  endif
>  
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> +CFLAGS_ABI   +=-meabi gnu
> +endif
> +endif
> +
>  # Accept old syntax despite ".syntax unified"
>  AFLAGS_NOWARN:=$(call 
> as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)