[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKwvOdksu_L+e52awkd=ffkaasCZeBjKcFU4nvU7q7reEzF2WA@mail.gmail.com>
Date: Fri, 30 Aug 2019 14:13:44 -0700
From: Nick Desaulniers <ndesaulniers@...gle.com>
To: Nathan Chancellor <natechancellor@...il.com>
Cc: Stefan Agner <stefan@...er.ch>,
Russell King <linux@...linux.org.uk>,
Arnd Bergmann <arnd@...db.de>,
Matthias Kaehlcke <mka@...omium.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
LKML <linux-kernel@...r.kernel.org>,
clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer
On Fri, Aug 30, 2019 at 10:28 AM Nathan Chancellor
<natechancellor@...il.com> 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 <stefan@...er.ch> 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 >= 100000)
> 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 <ndesaulniers@...gle.com>
> +
> # Accept old syntax despite ".syntax unified"
> AFLAGS_NOWARN :=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
>
--
Thanks,
~Nick Desaulniers
Powered by blists - more mailing lists