Date:   Thu, 28 May 2020 12:06:46 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Catalin Marinas <catalin.marinas@....com>,
        Peter Smith <Peter.Smith@....com>,
        Will Deacon <will@...nel.org>
Cc:     Robin Murphy <Robin.Murphy@....com>,
        Naohiro Aota <naohiro.aota@....com>,
        Stephen Boyd <swboyd@...gle.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Manoj Gupta <manojgupta@...gle.com>,
        Luis Lozano <llozano@...gle.com>,
        Nathan Chancellor <natechancellor@...il.com>,
        Vincenzo Frascino <Vincenzo.Frascino@....com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Kristof Beyls <Kristof.Beyls@....com>,
        Victor Campos <Victor.Campos@....com>,
        "david.spickett@...aro.org" <david.spickett@...aro.org>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm

On Thu, May 28, 2020 at 12:20 AM Will Deacon <will@...nel.org> wrote:
> > Yes, I know that :)

Right, I forgot; you wrote the 64b one. :)

On Thu, May 28, 2020 at 2:41 AM Catalin Marinas <catalin.marinas@....com> wrote:
>
> On Thu, May 28, 2020 at 09:05:08AM +0100, Peter Smith wrote:
> > I suggest using Arm if you need a frame pointer, and disable the
> > frame pointer if you want/need to use Thumb. My understanding is that
> > runtime unwinding using the frame pointer in Thumb is already difficult
> > due to Arm and Thumb functions using different registers for the frame
> > pointer.
>
> IIRC from the Thumb-2 kernel porting days, even in the absence of
> ARM/Thumb interworking, the Thumb-2 frame pointer was pretty useless for
> unwinding since it points to the bottom of the current stack frame (the
> reason I think is that some LDR/STR instructions with negative indexing
> are not available). So finding the previous frame pointer was not
> possible and had to rely on the exception unwinding information.

Eureka!  That's it!  Implicit state of -fomit-frame-pointer.

Ok, forgetting ARCH=arm64 for a second, for ARCH=arm we have the
choice CONFIG_THUMB2_KERNEL.  Regardless of which is chosen, we
*always* explicitly set -mthumb or -marm, but we never rely on
implicit defaults.  Again for ARCH=arm, we have a choice of unwinders,
or at least we do when *not* targeting thumb.  If we select
CONFIG_THUMB2_KERNEL, then CONFIG_UNWINDER_FRAME_POINTER cannot be
selected.

arch/arm/Kconfig.debug:
  57 config UNWINDER_FRAME_POINTER
  58   bool "Frame pointer unwinder"
  59   depends on !THUMB2_KERNEL
...

CONFIG_UNWINDER_FRAME_POINTER selects CONFIG_FRAME_POINTER which sets
-fno-omit-frame-pointer.  Otherwise, it looks like the choice of
-f{no-}omit-frame-pointer is left unspecified, relying on compiler
defaults.

And that's just for ARCH=arm.  Returning to ARCH=arm64, and the compat
vdso in particular, it doesn't look like we ever set
-f{no-}omit-frame-pointer either, so again we're looking at the
compiler defaults.

And when we look at the default behavior for the implicit state of
-f{no-}omit-frame-pointer, we find differences.

Here's a test case you can play around with:
https://godbolt.org/z/0oY39t

For Clang, can you tell what the default state is if left off?
For GCC, can you tell what the default state is if left off?
Do they match?
Is this specific to -mthumb?
(Bonus: what happens when you remove `-O2`?)

Answers:
-fno-omit-frame-pointer
-fomit-frame-pointer
No.
No.
-fno-omit-frame-pointer in GCC (-fomit-frame-pointer is an optimization)

Deja vu, I fixed a very similar discrepancy reported by Linus not too long ago.
https://reviews.llvm.org/D74698
Looking at the relevant logic in Clang:
https://github.com/llvm/llvm-project/blob/58beb76b7bd2f7caa1df461b9db6629521c3b60b/clang/lib/Driver/ToolChains/Clang.cpp#L527-L591
Noticely absent are arm, thumb, aarch64, and big endian variants,
specifically here:
https://github.com/llvm/llvm-project/blob/58beb76b7bd2f7caa1df461b9db6629521c3b60b/clang/lib/Driver/ToolChains/Clang.cpp#L556-L571

I should fix that in Clang.

That should also speed up our 32b ARM kernels that select
CONFIG_THUMB2_KERNEL (ie. CrOS veyron-minnie platform, rk3288).
Shouldn't make a difference for 64b ARM kernels since those select
frame pointers.  Though I am curious about the userspaces now for CrOS
and Android...

On Thu, May 28, 2020 at 1:05 AM Peter Smith <Peter.Smith@....com> wrote:
> > Hope this helps

Always, m8, you're the expert.

So r11 will be the frame pointer for arm and thumb according to latest
aapcs, but the compilers haven't yet made any changes related to this,
and can still use r7 in a bunch of cases (-mthumb
--target=arm-linux-gnueabi being the most relevant one for our
discussion).

> On Thu, May 28, 2020 at 12:20 AM Will Deacon <will@...nel.org> wrote:
> your
> patch is papering over a deeper issue.

Ah, your right.  Sorry, I was wrong.  I owe you (another) beer?  I'm
going into debt over these and will have to take out a loan, soon!
-- 
Thanks,
~Nick Desaulniers

Powered by blists - more mailing lists