Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Thu, May 28, 2020 at 12:20 AM Will Deacon wrote:
> > Yes, I know that :)
Right, I forgot; you wrote the 64b one. :)
On Thu, May 28, 2020 at 2:41 AM Catalin Marinas 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 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 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
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
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. -- Catalin
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
> From: Nick Desaulniers > Sent: 27 May 2020 21:31 > To: Robin Murphy > Cc: Catalin Marinas; Will Deacon; Naohiro Aota; Stephen Boyd; Masahiro > Yamada; LKML; Manoj Gupta; Luis Lozano; Nathan Chancellor; Vincenzo Frascino; > Linux ARM; Kristof Beyls; Victor Campos; david.spick...@linaro.org; Arnd > Bergmann; Peter Smith > Subject: Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm > > On Wed, May 27, 2020 at 1:14 PM Nick Desaulniers > wrote: > > > > On Wed, May 27, 2020 at 12:28 PM Robin Murphy wrote: > > > > > > On 2020-05-27 18:55, Nick Desaulniers wrote: > > > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy > > > > wrote: > > > >> > > > >> On 2020-05-26 18:31, Nick Desaulniers wrote: > > > >>> Custom toolchains that modify the default target to -mthumb cannot > > > >>> compile the arm64 compat vdso32, as > > > >>> arch/arm64/include/asm/vdso/compat_gettimeofday.h > > > >>> contains assembly that's invalid in -mthumb. Force the use of -marm, > > > >>> always. > > > >> > > > >> FWIW, this seems suspicious - the only assembly instructions I see > > > >> there > > > >> are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > > > >> -march=armv7a baseline that we set. > > > >> > > > >> On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > > > >> built a Thumb VDSO quite happily with Ubuntu 19.04's > > > >> gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > > > > > > > From the link in the commit message: `write to reserved register 'R7'` > > > > https://godbolt.org/z/zwr7iZ > > > > IIUC r7 is reserved for the frame pointer in THUMB? > > > > > > It can be, if you choose to build with frame pointers and the common > > > frame pointer ABI for Thumb code that uses r7. However it can also be > > > for other things like the syscall number in the Arm syscall ABI too. > > > > Ah, right, with -fomit-frame-pointer, this error also goes away. Not > > sure if we prefer either: > > - build the compat vdso as -marm always or > > - disable frame pointers for the vdso (does this have unwinding > > implications?) > > - other? > > > > > I > > > take it Clang has decided that writing syscall wrappers with minimal > > > inline asm is not a thing people deserve to do without arbitrary other > > > restrictions? > > > > Was the intent not obvious? We would have gotten away with it, too, if > > wasn't for you meddling kids and your stupid dog! /s > > https://www.youtube.com/watch?v=hXUqwuzcGeU > > Anyways, this seems to explain more the intentions: > > https://reviews.llvm.org/D76848#1945810 > > + Victor, Kristof (ARM) > > And maybe some other useful data points regarding warning on use of r7 > and frame pointers. > https://github.com/ClangBuiltLinux/linux/issues/701#issuecomment-591325758 > https://bugs.llvm.org/show_bug.cgi?id=45826 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94986 > > + Peter (ARM) > + David, Arnd (Linaro) > -- > Thanks, > ~Nick Desaulniers Hello Nick, The AAPCS has only recently (28th January 2020) been updated with a specification of the frame pointer and frame chain. My understanding is that neither gcc nor clang implement this part yet. Historically the frame pointer in Thumb has not been defined in the AAPCS with implementations falling back to historic definitions of fp = r7 in Thumb and fp = r11 in Arm. The use of different frame pointer registers in Arm and Thumb state makes it difficult to construct a frame chain with interworking. My understanding of the AAPCS is that it has been changed to make the frame pointer r11 on both Arm and Thumb to make unwinding possible, at the expense of r11 being harder to access from 16-bit Thumb instructions. There are 4 levels of conformance with respect to construction of frame records and frame chain as it is likely some platforms will want to persist with r7. An implementation of the latest version of the AAPCS would permit a frame pointer and use of r7 as a reserved register. Although as you'll need to support older compilers this may not be an option. 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. Hope this helps Peter
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 11:17:33AM -0700, Nick Desaulniers wrote: > On Wed, May 27, 2020 at 11:08 AM Will Deacon wrote: > > > > On Wed, May 27, 2020 at 10:55:24AM -0700, Nick Desaulniers wrote: > > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: > > > > > > > > On 2020-05-26 18:31, Nick Desaulniers wrote: > > > > > Custom toolchains that modify the default target to -mthumb cannot > > > > > compile the arm64 compat vdso32, as > > > > > arch/arm64/include/asm/vdso/compat_gettimeofday.h > > > > > contains assembly that's invalid in -mthumb. Force the use of -marm, > > > > > always. > > > > > > > > FWIW, this seems suspicious - the only assembly instructions I see there > > > > are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > > > > -march=armv7a baseline that we set. > > > > > > > > On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > > > > built a Thumb VDSO quite happily with Ubuntu 19.04's > > > > gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > > > > > From the link in the commit message: `write to reserved register 'R7'` > > > https://godbolt.org/z/zwr7iZ > > > IIUC r7 is reserved for the frame pointer in THUMB? > > > > > > What is the implicit default of your gcc-arm-linux-gnueabihf at -O2? > > > -mthumb, or -marm? > > > > Hmm, but this *is* weird because if I build a 32-bit kernel then I get > > either an ARM or a Thumb-2 VDSO depending on CONFIG_THUMB2_KERNEL. I'm > > not sure if that's deliberate, but both build and appear to work. > > That's because there's 3 VDSO's when it comes to ARM: > arm64's 64b vdso: arch/arm64/kernel/vdso > arm64's 32b vdso: arch/arm64/kernel/vdso32/ > arm's 32b vdso: arch/arm/kernel/vdso.c Yes, I know that :) > When you build a 32b kernel, you're only making use of the last of > those three; the arm64 vdso and vdso32 code is irrelevant. > This patch is specific to the second case, which is the 32b compat > vdso for a 64b kernel. Sure, but if you can build a Thumb-2 vDSO object for arch/arm/ using then we should be able to build a Thumb-2 compat vDSO for arch/arm64, and your patch is papering over a deeper issue. Generally, having the compat vDSO behave differently to the arch/arm/ vDSO is indicative of something being broken. In other words, if your patch was correct (not sure that it is) then I would expect a corresponding change to arch/arm/ to pass -marm when CONFIG_THUMB2_KERNEL=y. Make sense? Will
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 1:31 PM Nick Desaulniers wrote: > > On Wed, May 27, 2020 at 1:14 PM Nick Desaulniers > wrote: > > > > On Wed, May 27, 2020 at 12:28 PM Robin Murphy wrote: > > > > > > On 2020-05-27 18:55, Nick Desaulniers wrote: > > > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy > > > > wrote: > > > >> > > > >> On 2020-05-26 18:31, Nick Desaulniers wrote: > > > >>> Custom toolchains that modify the default target to -mthumb cannot > > > >>> compile the arm64 compat vdso32, as > > > >>> arch/arm64/include/asm/vdso/compat_gettimeofday.h > > > >>> contains assembly that's invalid in -mthumb. Force the use of -marm, > > > >>> always. > > > >> > > > >> FWIW, this seems suspicious - the only assembly instructions I see > > > >> there > > > >> are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > > > >> -march=armv7a baseline that we set. > > > >> > > > >> On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > > > >> built a Thumb VDSO quite happily with Ubuntu 19.04's > > > >> gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > > > > > > > From the link in the commit message: `write to reserved register 'R7'` > > > > https://godbolt.org/z/zwr7iZ > > > > IIUC r7 is reserved for the frame pointer in THUMB? > > > > > > It can be, if you choose to build with frame pointers and the common > > > frame pointer ABI for Thumb code that uses r7. However it can also be > > > for other things like the syscall number in the Arm syscall ABI too. > > > > Ah, right, with -fomit-frame-pointer, this error also goes away. Not > > sure if we prefer either: > > - build the compat vdso as -marm always or > > - disable frame pointers for the vdso (does this have unwinding > > implications?) > > - other? > > > > > I > > > take it Clang has decided that writing syscall wrappers with minimal > > > inline asm is not a thing people deserve to do without arbitrary other > > > restrictions? > > > > Was the intent not obvious? We would have gotten away with it, too, if > > wasn't for you meddling kids and your stupid dog! /s > > https://www.youtube.com/watch?v=hXUqwuzcGeU > > Anyways, this seems to explain more the intentions: > > https://reviews.llvm.org/D76848#1945810 > > + Victor, Kristof (ARM) > > And maybe some other useful data points regarding warning on use of r7 > and frame pointers. > https://github.com/ClangBuiltLinux/linux/issues/701#issuecomment-591325758 > https://bugs.llvm.org/show_bug.cgi?id=45826 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94986 > > + Peter (ARM) > + David, Arnd (Linaro) Also, when I looked into this briefly, I didn't happen to see anything in AAPCS that mentions r7 is used as the frame pointer for THUMB. Does AAPCS not cover THUMB? It also states the TPCS is obsolete. https://developer.arm.com/docs/ihi0042/latest https://static.docs.arm.com/ihi0042/i/aapcs32.pdf -- Thanks, ~Nick Desaulniers
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 1:14 PM Nick Desaulniers wrote: > > On Wed, May 27, 2020 at 12:28 PM Robin Murphy wrote: > > > > On 2020-05-27 18:55, Nick Desaulniers wrote: > > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: > > >> > > >> On 2020-05-26 18:31, Nick Desaulniers wrote: > > >>> Custom toolchains that modify the default target to -mthumb cannot > > >>> compile the arm64 compat vdso32, as > > >>> arch/arm64/include/asm/vdso/compat_gettimeofday.h > > >>> contains assembly that's invalid in -mthumb. Force the use of -marm, > > >>> always. > > >> > > >> FWIW, this seems suspicious - the only assembly instructions I see there > > >> are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > > >> -march=armv7a baseline that we set. > > >> > > >> On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > > >> built a Thumb VDSO quite happily with Ubuntu 19.04's > > >> gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > > > > > From the link in the commit message: `write to reserved register 'R7'` > > > https://godbolt.org/z/zwr7iZ > > > IIUC r7 is reserved for the frame pointer in THUMB? > > > > It can be, if you choose to build with frame pointers and the common > > frame pointer ABI for Thumb code that uses r7. However it can also be > > for other things like the syscall number in the Arm syscall ABI too. > > Ah, right, with -fomit-frame-pointer, this error also goes away. Not > sure if we prefer either: > - build the compat vdso as -marm always or > - disable frame pointers for the vdso (does this have unwinding implications?) > - other? > > > I > > take it Clang has decided that writing syscall wrappers with minimal > > inline asm is not a thing people deserve to do without arbitrary other > > restrictions? > > Was the intent not obvious? We would have gotten away with it, too, if > wasn't for you meddling kids and your stupid dog! /s > https://www.youtube.com/watch?v=hXUqwuzcGeU > Anyways, this seems to explain more the intentions: > https://reviews.llvm.org/D76848#1945810 > + Victor, Kristof (ARM) And maybe some other useful data points regarding warning on use of r7 and frame pointers. https://github.com/ClangBuiltLinux/linux/issues/701#issuecomment-591325758 https://bugs.llvm.org/show_bug.cgi?id=45826 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94986 + Peter (ARM) + David, Arnd (Linaro) -- Thanks, ~Nick Desaulniers
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 12:28 PM Robin Murphy wrote: > > On 2020-05-27 18:55, Nick Desaulniers wrote: > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: > >> > >> On 2020-05-26 18:31, Nick Desaulniers wrote: > >>> Custom toolchains that modify the default target to -mthumb cannot > >>> compile the arm64 compat vdso32, as > >>> arch/arm64/include/asm/vdso/compat_gettimeofday.h > >>> contains assembly that's invalid in -mthumb. Force the use of -marm, > >>> always. > >> > >> FWIW, this seems suspicious - the only assembly instructions I see there > >> are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > >> -march=armv7a baseline that we set. > >> > >> On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > >> built a Thumb VDSO quite happily with Ubuntu 19.04's > >> gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > > > From the link in the commit message: `write to reserved register 'R7'` > > https://godbolt.org/z/zwr7iZ > > IIUC r7 is reserved for the frame pointer in THUMB? > > It can be, if you choose to build with frame pointers and the common > frame pointer ABI for Thumb code that uses r7. However it can also be > for other things like the syscall number in the Arm syscall ABI too. Ah, right, with -fomit-frame-pointer, this error also goes away. Not sure if we prefer either: - build the compat vdso as -marm always or - disable frame pointers for the vdso (does this have unwinding implications?) - other? > I > take it Clang has decided that writing syscall wrappers with minimal > inline asm is not a thing people deserve to do without arbitrary other > restrictions? Was the intent not obvious? We would have gotten away with it, too, if wasn't for you meddling kids and your stupid dog! /s https://www.youtube.com/watch?v=hXUqwuzcGeU Anyways, this seems to explain more the intentions: https://reviews.llvm.org/D76848#1945810 + Victor, Kristof (ARM) -- Thanks, ~Nick Desaulniers
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On 2020-05-27 20:28, Robin Murphy wrote: On 2020-05-27 18:55, Nick Desaulniers wrote: On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: On 2020-05-26 18:31, Nick Desaulniers wrote: Custom toolchains that modify the default target to -mthumb cannot compile the arm64 compat vdso32, as arch/arm64/include/asm/vdso/compat_gettimeofday.h contains assembly that's invalid in -mthumb. Force the use of -marm, always. FWIW, this seems suspicious - the only assembly instructions I see there are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the -march=armv7a baseline that we set. On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and built a Thumb VDSO quite happily with Ubuntu 19.04's gcc-arm-linux-gnueabihf. What was the actual failure you saw? From the link in the commit message: `write to reserved register 'R7'` https://godbolt.org/z/zwr7iZ IIUC r7 is reserved for the frame pointer in THUMB? It can be, if you choose to build with frame pointers and the common frame pointer ABI for Thumb code that uses r7. However it can also be for other things like the syscall number in the Arm syscall ABI too. I Oh, and for the avoidance of ambiguity that's "Arm" as in the 32-bit Arm architecture port, not a specific instruction set. Robin. take it Clang has decided that writing syscall wrappers with minimal inline asm is not a thing people deserve to do without arbitrary other restrictions? What is the implicit default of your gcc-arm-linux-gnueabihf at -O2? -mthumb, or -marm? As Dave pointed out, like the probable majority of users it's Thumb: $ arm-linux-gnueabihf-gcc -v Using built-in specs. COLLECT_GCC=arm-linux-gnueabihf-gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/arm-linux-gnueabihf/8/lto-wrapper Target: arm-linux-gnueabihf Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 8.3.0-6ubuntu1' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libitm --disable-libquadmath --disable-libquadmath-support --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-multiarch --enable-multilib --disable-sjlj-exceptions --with-arch=armv7-a --with-fpu=vfpv3-d16 --with-float=hard --with-mode=thumb --disable-werror --enable-multilib --enable-checking=release --build=aarch64-linux-gnu --host=aarch64-linux-gnu --target=arm-linux-gnueabihf --program-prefix=arm-linux-gnueabihf- --includedir=/usr/arm-linux-gnueabihf/include Thread model: posix gcc version 8.3.0 (Ubuntu/Linaro 8.3.0-6ubuntu1) (yeah, I didn't actually need to hack my makefile at all) Robin. ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On 2020-05-27 18:55, Nick Desaulniers wrote: On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: On 2020-05-26 18:31, Nick Desaulniers wrote: Custom toolchains that modify the default target to -mthumb cannot compile the arm64 compat vdso32, as arch/arm64/include/asm/vdso/compat_gettimeofday.h contains assembly that's invalid in -mthumb. Force the use of -marm, always. FWIW, this seems suspicious - the only assembly instructions I see there are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the -march=armv7a baseline that we set. On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and built a Thumb VDSO quite happily with Ubuntu 19.04's gcc-arm-linux-gnueabihf. What was the actual failure you saw? From the link in the commit message: `write to reserved register 'R7'` https://godbolt.org/z/zwr7iZ IIUC r7 is reserved for the frame pointer in THUMB? It can be, if you choose to build with frame pointers and the common frame pointer ABI for Thumb code that uses r7. However it can also be for other things like the syscall number in the Arm syscall ABI too. I take it Clang has decided that writing syscall wrappers with minimal inline asm is not a thing people deserve to do without arbitrary other restrictions? What is the implicit default of your gcc-arm-linux-gnueabihf at -O2? -mthumb, or -marm? As Dave pointed out, like the probable majority of users it's Thumb: $ arm-linux-gnueabihf-gcc -v Using built-in specs. COLLECT_GCC=arm-linux-gnueabihf-gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/arm-linux-gnueabihf/8/lto-wrapper Target: arm-linux-gnueabihf Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 8.3.0-6ubuntu1' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libitm --disable-libquadmath --disable-libquadmath-support --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-multiarch --enable-multilib --disable-sjlj-exceptions --with-arch=armv7-a --with-fpu=vfpv3-d16 --with-float=hard --with-mode=thumb --disable-werror --enable-multilib --enable-checking=release --build=aarch64-linux-gnu --host=aarch64-linux-gnu --target=arm-linux-gnueabihf --program-prefix=arm-linux-gnueabihf- --includedir=/usr/arm-linux-gnueabihf/include Thread model: posix gcc version 8.3.0 (Ubuntu/Linaro 8.3.0-6ubuntu1) (yeah, I didn't actually need to hack my makefile at all) Robin.
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 11:08 AM Will Deacon wrote: > > On Wed, May 27, 2020 at 10:55:24AM -0700, Nick Desaulniers wrote: > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: > > > > > > On 2020-05-26 18:31, Nick Desaulniers wrote: > > > > Custom toolchains that modify the default target to -mthumb cannot > > > > compile the arm64 compat vdso32, as > > > > arch/arm64/include/asm/vdso/compat_gettimeofday.h > > > > contains assembly that's invalid in -mthumb. Force the use of -marm, > > > > always. > > > > > > FWIW, this seems suspicious - the only assembly instructions I see there > > > are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > > > -march=armv7a baseline that we set. > > > > > > On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > > > built a Thumb VDSO quite happily with Ubuntu 19.04's > > > gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > > > From the link in the commit message: `write to reserved register 'R7'` > > https://godbolt.org/z/zwr7iZ > > IIUC r7 is reserved for the frame pointer in THUMB? > > > > What is the implicit default of your gcc-arm-linux-gnueabihf at -O2? > > -mthumb, or -marm? > > Hmm, but this *is* weird because if I build a 32-bit kernel then I get > either an ARM or a Thumb-2 VDSO depending on CONFIG_THUMB2_KERNEL. I'm > not sure if that's deliberate, but both build and appear to work. That's because there's 3 VDSO's when it comes to ARM: arm64's 64b vdso: arch/arm64/kernel/vdso arm64's 32b vdso: arch/arm64/kernel/vdso32/ arm's 32b vdso: arch/arm/kernel/vdso.c When you build a 32b kernel, you're only making use of the last of those three; the arm64 vdso and vdso32 code is irrelevant. This patch is specific to the second case, which is the 32b compat vdso for a 64b kernel. > > I'll drop this patch for now, while we figure it out a bit more. > > Cheers, > > Will -- Thanks, ~Nick Desaulniers
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 10:55:24AM -0700, Nick Desaulniers wrote: > On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: > > > > On 2020-05-26 18:31, Nick Desaulniers wrote: > > > Custom toolchains that modify the default target to -mthumb cannot > > > compile the arm64 compat vdso32, as > > > arch/arm64/include/asm/vdso/compat_gettimeofday.h > > > contains assembly that's invalid in -mthumb. Force the use of -marm, > > > always. > > > > FWIW, this seems suspicious - the only assembly instructions I see there > > are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > > -march=armv7a baseline that we set. > > > > On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > > built a Thumb VDSO quite happily with Ubuntu 19.04's > > gcc-arm-linux-gnueabihf. What was the actual failure you saw? > > From the link in the commit message: `write to reserved register 'R7'` > https://godbolt.org/z/zwr7iZ > IIUC r7 is reserved for the frame pointer in THUMB? > > What is the implicit default of your gcc-arm-linux-gnueabihf at -O2? > -mthumb, or -marm? Hmm, but this *is* weird because if I build a 32-bit kernel then I get either an ARM or a Thumb-2 VDSO depending on CONFIG_THUMB2_KERNEL. I'm not sure if that's deliberate, but both build and appear to work. I'll drop this patch for now, while we figure it out a bit more. Cheers, Will
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 6:53 AM Dave Martin wrote: > > On Tue, May 26, 2020 at 09:45:05PM +0100, Will Deacon wrote: > > On Tue, 26 May 2020 10:31:14 -0700, Nick Desaulniers wrote: > > > Custom toolchains that modify the default target to -mthumb cannot > > It's probably too late to water this down, but it's unfortunate to have > this comment in the upstream commit history. > > It's not constructive to call the native compiler configuration of > major distros for many years a "custom" toolchain. Unmodified GCC has I don't think you know which toolchain or distro I'm referring to. ;) > had a clean configure option for this for a very long time; it's not > someone's dirty hack. (The wisdom of armhf's choice of -mthumb might > be debated, but it is well established.) > > Ignoring the triplet and passing random options to a compiler in the > hopes that it will do the right thing for an irregular usecase has never > been reliable. Usecases don't get much more irregular than building > vdso32. > > arch/arm has the proper options in its Makefiles. > > This patch is a kernel bugfix, plain and simple. Borrowing from the Zen of Python: Explicit is better than Implicit. Better not to rely on implicit defaults that may be changed at configure time. > Does this need to go to stable? Oh, probably. Need to wait until it hits mainline now. I don't think the compat vdso series was backported to 5.4, but IIUC stable maintains a branch for the latest release, which would have that series. -- Thanks, ~Nick Desaulniers
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Wed, May 27, 2020 at 6:45 AM Robin Murphy wrote: > > On 2020-05-26 18:31, Nick Desaulniers wrote: > > Custom toolchains that modify the default target to -mthumb cannot > > compile the arm64 compat vdso32, as > > arch/arm64/include/asm/vdso/compat_gettimeofday.h > > contains assembly that's invalid in -mthumb. Force the use of -marm, > > always. > > FWIW, this seems suspicious - the only assembly instructions I see there > are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the > -march=armv7a baseline that we set. > > On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and > built a Thumb VDSO quite happily with Ubuntu 19.04's > gcc-arm-linux-gnueabihf. What was the actual failure you saw? >From the link in the commit message: `write to reserved register 'R7'` https://godbolt.org/z/zwr7iZ IIUC r7 is reserved for the frame pointer in THUMB? What is the implicit default of your gcc-arm-linux-gnueabihf at -O2? -mthumb, or -marm? -- Thanks, ~Nick Desaulniers
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Tue, May 26, 2020 at 09:45:05PM +0100, Will Deacon wrote: > On Tue, 26 May 2020 10:31:14 -0700, Nick Desaulniers wrote: > > Custom toolchains that modify the default target to -mthumb cannot It's probably too late to water this down, but it's unfortunate to have this comment in the upstream commit history. It's not constructive to call the native compiler configuration of major distros for many years a "custom" toolchain. Unmodified GCC has had a clean configure option for this for a very long time; it's not someone's dirty hack. (The wisdom of armhf's choice of -mthumb might be debated, but it is well established.) Ignoring the triplet and passing random options to a compiler in the hopes that it will do the right thing for an irregular usecase has never been reliable. Usecases don't get much more irregular than building vdso32. arch/arm has the proper options in its Makefiles. This patch is a kernel bugfix, plain and simple. > > compile the arm64 compat vdso32, as > > arch/arm64/include/asm/vdso/compat_gettimeofday.h > > contains assembly that's invalid in -mthumb. Force the use of -marm, > > always. > > Applied to arm64 (for-next/vdso), thanks! > > [1/1] arm64: vdso32: force vdso32 to be compiled as -marm > https://git.kernel.org/arm64/c/20363b59ad4f Does this need to go to stable? Cheers ---Dave
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On 2020-05-26 18:31, Nick Desaulniers wrote: Custom toolchains that modify the default target to -mthumb cannot compile the arm64 compat vdso32, as arch/arm64/include/asm/vdso/compat_gettimeofday.h contains assembly that's invalid in -mthumb. Force the use of -marm, always. FWIW, this seems suspicious - the only assembly instructions I see there are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the -march=armv7a baseline that we set. On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and built a Thumb VDSO quite happily with Ubuntu 19.04's gcc-arm-linux-gnueabihf. What was the actual failure you saw? Robin. Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084372 Cc: Stephen Boyd Reported-by: Luis Lozano Tested-by: Manoj Gupta Signed-off-by: Nick Desaulniers --- Surgeon General's Warning: changing the compiler defaults is not recommended and can lead to spooky bugs that are hard to reproduce upstream. arch/arm64/kernel/vdso32/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 3964738ebbde..c449a293d81e 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -104,6 +104,8 @@ VDSO_CFLAGS += -D__uint128_t='void*' # (on GCC 4.8 or older, there is unfortunately no way to silence this warning) VDSO_CFLAGS += $(call cc32-disable-warning,shift-count-overflow) VDSO_CFLAGS += -Wno-int-to-pointer-cast +# Force vdso to be compiled in ARM mode, not THUMB. +VDSO_CFLAGS += -marm VDSO_AFLAGS := $(VDSO_CAFLAGS) VDSO_AFLAGS += -D__ASSEMBLY__
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
On Tue, 26 May 2020 10:31:14 -0700, Nick Desaulniers wrote: > Custom toolchains that modify the default target to -mthumb cannot > compile the arm64 compat vdso32, as > arch/arm64/include/asm/vdso/compat_gettimeofday.h > contains assembly that's invalid in -mthumb. Force the use of -marm, > always. Applied to arm64 (for-next/vdso), thanks! [1/1] arm64: vdso32: force vdso32 to be compiled as -marm https://git.kernel.org/arm64/c/20363b59ad4f Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev
Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
Quoting Nick Desaulniers (2020-05-26 10:31:14) > Custom toolchains that modify the default target to -mthumb cannot > compile the arm64 compat vdso32, as > arch/arm64/include/asm/vdso/compat_gettimeofday.h > contains assembly that's invalid in -mthumb. Force the use of -marm, > always. > > Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084372 > Cc: Stephen Boyd > Reported-by: Luis Lozano > Tested-by: Manoj Gupta > Signed-off-by: Nick Desaulniers > --- Reviewed-by: Stephen Boyd
[PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
Custom toolchains that modify the default target to -mthumb cannot compile the arm64 compat vdso32, as arch/arm64/include/asm/vdso/compat_gettimeofday.h contains assembly that's invalid in -mthumb. Force the use of -marm, always. Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084372 Cc: Stephen Boyd Reported-by: Luis Lozano Tested-by: Manoj Gupta Signed-off-by: Nick Desaulniers --- Surgeon General's Warning: changing the compiler defaults is not recommended and can lead to spooky bugs that are hard to reproduce upstream. arch/arm64/kernel/vdso32/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 3964738ebbde..c449a293d81e 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -104,6 +104,8 @@ VDSO_CFLAGS += -D__uint128_t='void*' # (on GCC 4.8 or older, there is unfortunately no way to silence this warning) VDSO_CFLAGS += $(call cc32-disable-warning,shift-count-overflow) VDSO_CFLAGS += -Wno-int-to-pointer-cast +# Force vdso to be compiled in ARM mode, not THUMB. +VDSO_CFLAGS += -marm VDSO_AFLAGS := $(VDSO_CAFLAGS) VDSO_AFLAGS += -D__ASSEMBLY__ -- 2.27.0.rc0.183.gde8f92d652-goog
