* [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error
@ 2020-03-26 15:26 Stefan Ghinea
2020-03-26 17:23 ` [OE-core] " Andre McCurdy
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Stefan Ghinea @ 2020-03-26 15:26 UTC (permalink / raw)
To: openembedded-core
From: Catalin Enache <catalin.enache@windriver.com>
When compiling for Thumb or Thumb2, frame pointers _must_ be disabled
since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline
asm to make syscalls (where r7 is used for the syscall NR).
In most cases, frame pointers will be disabled automatically due to
the optimisation level, but appending an explicit -fomit-frame-pointer
to CFLAGS handles cases where optimisation is set to -O0 or frame
pointers have been enabled by -fno-omit-frame-pointer earlier in
CFLAGS, etc.
References:
https://www.openwall.com/lists/musl/2017/10/09/2
Signed-off-by: Catalin Enache <catalin.enache@windriver.com>
Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
---
meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
index 4e32b27087..c7f3e67022 100644
--- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
+++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
@@ -63,6 +63,14 @@ DEPENDS += "speexdsp libxml-parser-perl-native libcap"
inherit autotools bash-completion pkgconfig useradd gettext perlnative systemd manpages gsettings
+# When compiling for Thumb or Thumb2, frame pointers _must_ be disabled since the
+# Thumb frame pointer in r7 clashes with pulseaudio's use of inline asm to make syscalls
+# (where r7 is used for the syscall NR). In most cases, frame pointers will be
+# disabled automatically due to the optimisation level, but append an explicit
+# -fomit-frame-pointer to handle cases where optimisation is set to -O0 or frame
+# pointers have been enabled by -fno-omit-frame-pointer earlier in CFLAGS, etc.
+CFLAGS_append_arm = " ${@bb.utils.contains('TUNE_CCARGS', '-mthumb', '-fomit-frame-pointer', '', d)}"
+
# *.desktop rules wont be generated during configure and build will fail
# if using --disable-nls
USE_NLS = "yes"
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 15:26 [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error Stefan Ghinea @ 2020-03-26 17:23 ` Andre McCurdy 2020-03-26 19:16 ` Adrian Bunk ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Andre McCurdy @ 2020-03-26 17:23 UTC (permalink / raw) To: Stefan Ghinea; +Cc: OE Core mailing list On Thu, Mar 26, 2020 at 8:26 AM Stefan Ghinea <stefan.ghinea@windriver.com> wrote: > > From: Catalin Enache <catalin.enache@windriver.com> > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline > asm to make syscalls (where r7 is used for the syscall NR). > > In most cases, frame pointers will be disabled automatically due to > the optimisation level, but appending an explicit -fomit-frame-pointer > to CFLAGS handles cases where optimisation is set to -O0 or frame > pointers have been enabled by -fno-omit-frame-pointer earlier in > CFLAGS, etc. > > References: > https://www.openwall.com/lists/musl/2017/10/09/2 > > Signed-off-by: Catalin Enache <catalin.enache@windriver.com> > Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com> > --- > meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc > index 4e32b27087..c7f3e67022 100644 > --- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc > +++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc > @@ -63,6 +63,14 @@ DEPENDS += "speexdsp libxml-parser-perl-native libcap" > > inherit autotools bash-completion pkgconfig useradd gettext perlnative systemd manpages gsettings > > +# When compiling for Thumb or Thumb2, frame pointers _must_ be disabled since the > +# Thumb frame pointer in r7 clashes with pulseaudio's use of inline asm to make syscalls > +# (where r7 is used for the syscall NR). In most cases, frame pointers will be > +# disabled automatically due to the optimisation level, but append an explicit > +# -fomit-frame-pointer to handle cases where optimisation is set to -O0 or frame > +# pointers have been enabled by -fno-omit-frame-pointer earlier in CFLAGS, etc. > +CFLAGS_append_arm = " ${@bb.utils.contains('TUNE_CCARGS', '-mthumb', '-fomit-frame-pointer', '', d)}" This is OK and safe to merge but note that it's now regarded as the "old style" fix for this problem. The newer approach is to tweak the asm code to save the frame pointer before making the syscall and restore it afterwards. e.g. https://github.com/strace/strace/commit/0c75ebaad09d6d3f2395dfe6160904af883dd0d9 If pulseaudio can be fixed in the same way then that's probably a better approach. > # *.desktop rules wont be generated during configure and build will fail > # if using --disable-nls > USE_NLS = "yes" > -- > 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 15:26 [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error Stefan Ghinea 2020-03-26 17:23 ` [OE-core] " Andre McCurdy @ 2020-03-26 19:16 ` Adrian Bunk 2020-03-26 19:53 ` Andre McCurdy 2020-03-30 16:29 ` Tanu Kaskinen [not found] ` <160121D94406B75D.27565@lists.openembedded.org> 3 siblings, 1 reply; 12+ messages in thread From: Adrian Bunk @ 2020-03-26 19:16 UTC (permalink / raw) To: Stefan Ghinea; +Cc: openembedded-core On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: >... > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > since the Thumb frame pointer in r7 >... How are you reproducing the problem in pulseaudio? This sounds like a workaround for a bug in musl that was fixed 2 years ago. cu Adrian ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 19:16 ` Adrian Bunk @ 2020-03-26 19:53 ` Andre McCurdy 2020-03-26 20:26 ` Adrian Bunk 0 siblings, 1 reply; 12+ messages in thread From: Andre McCurdy @ 2020-03-26 19:53 UTC (permalink / raw) To: Adrian Bunk; +Cc: Stefan Ghinea, OE Core mailing list On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote: > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: > >... > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > since the Thumb frame pointer in r7 > >... > > How are you reproducing the problem in pulseaudio? > > This sounds like a workaround for a bug in musl that was fixed 2 years ago. The problem can show up anywhere that inline asm is trying to use r7. In this case it looks like: https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50 Better fix there might be to use registers { r4,r5,r6,r12 } instead of { r4,r5,r6,r7 } ? > cu > Adrian > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 19:53 ` Andre McCurdy @ 2020-03-26 20:26 ` Adrian Bunk 2020-03-26 21:23 ` Andre McCurdy 0 siblings, 1 reply; 12+ messages in thread From: Adrian Bunk @ 2020-03-26 20:26 UTC (permalink / raw) To: Andre McCurdy; +Cc: Stefan Ghinea, OE Core mailing list On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote: > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: > > >... > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > > since the Thumb frame pointer in r7 > > >... > > > > How are you reproducing the problem in pulseaudio? > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago. > > The problem can show up anywhere that inline asm is trying to use r7. > In this case it looks like: > > https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50 >... After looking at the pulseaudio code I suspected the patch description claiming pulseaudio syscall code would be the problem was rubbish, and that this NEON code was the problem. But when I tried to reproduce the problem it built for me with both glibc and musl in master (the patch didn't mention that this was a musl-only problem). Then I saw that this was fixed in musl upstream 2 years ago: https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e cu Adrian ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 20:26 ` Adrian Bunk @ 2020-03-26 21:23 ` Andre McCurdy 2020-03-27 18:56 ` Adrian Bunk 2020-07-17 10:09 ` Tanu Kaskinen 0 siblings, 2 replies; 12+ messages in thread From: Andre McCurdy @ 2020-03-26 21:23 UTC (permalink / raw) To: Adrian Bunk; +Cc: Stefan Ghinea, OE Core mailing list On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote: > > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote: > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: > > > >... > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > > > since the Thumb frame pointer in r7 > > > >... > > > > > > How are you reproducing the problem in pulseaudio? > > > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago. > > > > The problem can show up anywhere that inline asm is trying to use r7. > > In this case it looks like: > > > > https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50 > >... > > After looking at the pulseaudio code I suspected the patch description > claiming pulseaudio syscall code would be the problem was rubbish, and > that this NEON code was the problem. Yes, the comment looks like it was copied and pasted and doesn't really apply in this case (since pulseaudio isn't making syscalls). That should be updated. > But when I tried to reproduce the problem it built for me with both > glibc and musl in master (the patch didn't mention that this was a > musl-only problem). > > Then I saw that this was fixed in musl upstream 2 years ago: > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e Right, it's not related to musl or glibc. I suspect it can be reproduced by building for an ARM target which supports NEON, ensuring that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to thumb and then compiling with frame pointers enabled (e.g. by adding -fno-omit-frame-pointer to CFLAGS). In terms of a fix, then changing the code to use r12 instead of r7 is probably the best solution (assuming it works), but would need careful testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines building for Thumb is safe and should fix the issue too. Presumably limiting the -fomit-frame-pointer workaround to ARM machines which support NEON building for Thumb would be an even more targeted solution. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 21:23 ` Andre McCurdy @ 2020-03-27 18:56 ` Adrian Bunk 2020-07-17 10:09 ` Tanu Kaskinen 1 sibling, 0 replies; 12+ messages in thread From: Adrian Bunk @ 2020-03-27 18:56 UTC (permalink / raw) To: Andre McCurdy; +Cc: Stefan Ghinea, OE Core mailing list On Thu, Mar 26, 2020 at 02:23:03PM -0700, Andre McCurdy wrote: >... > Right, it's not related to musl or glibc. I suspect it can be > reproduced by building for an ARM target which supports NEON, ensuring > that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be > armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to > thumb and then compiling with frame pointers enabled (e.g. by adding > -fno-omit-frame-pointer to CFLAGS). >... Thanks, this was helpful in figuring out what I missed when testing. I can now confirm that the problem still exists, also with glibc. cu Adrian ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 21:23 ` Andre McCurdy 2020-03-27 18:56 ` Adrian Bunk @ 2020-07-17 10:09 ` Tanu Kaskinen 2020-07-17 19:19 ` Andre McCurdy 1 sibling, 1 reply; 12+ messages in thread From: Tanu Kaskinen @ 2020-07-17 10:09 UTC (permalink / raw) To: Andre McCurdy, Adrian Bunk; +Cc: Stefan Ghinea, OE Core mailing list On Thu, 2020-03-26 at 14:23 -0700, Andre McCurdy wrote: > On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote: > > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote: > > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: > > > > > ... > > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > > > > since the Thumb frame pointer in r7 > > > > > ... > > > > > > > > How are you reproducing the problem in pulseaudio? > > > > > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago. > > > > > > The problem can show up anywhere that inline asm is trying to use r7. > > > In this case it looks like: > > > > > > https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50 > > > ... > > > > After looking at the pulseaudio code I suspected the patch description > > claiming pulseaudio syscall code would be the problem was rubbish, and > > that this NEON code was the problem. > > Yes, the comment looks like it was copied and pasted and doesn't > really apply in this case (since pulseaudio isn't making syscalls). > That should be updated. > > > But when I tried to reproduce the problem it built for me with both > > glibc and musl in master (the patch didn't mention that this was a > > musl-only problem). > > > > Then I saw that this was fixed in musl upstream 2 years ago: > > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e > > Right, it's not related to musl or glibc. I suspect it can be > reproduced by building for an ARM target which supports NEON, ensuring > that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be > armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to > thumb and then compiling with frame pointers enabled (e.g. by adding > -fno-omit-frame-pointer to CFLAGS). > > In terms of a fix, then changing the code to use r12 instead of r7 is > probably the best solution (assuming it works), but would need careful > testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines > building for Thumb is safe and should fix the issue too. Presumably > limiting the -fomit-frame-pointer workaround to ARM machines which > support NEON building for Thumb would be an even more targeted > solution. I finally found time to test fixing the assembly code to use r12 instead of r7. Seems to work fine (I was first baffled by incorrect behaviour, because I changed "{r4-r7}" to "{r4-r12}" without realizing that "r4-r12" meant a range of all registers from r4 to r12). Can you enlighten me: why did you choose r12 instead of r8? Why did the original author use registers r4-r7 instead of r0-r3? Is it somehow advisable to avoid registers r0-r3 and r8-r11? The code seems to work fine with any set of registers, except r7. -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-07-17 10:09 ` Tanu Kaskinen @ 2020-07-17 19:19 ` Andre McCurdy 2020-07-18 4:09 ` Tanu Kaskinen 0 siblings, 1 reply; 12+ messages in thread From: Andre McCurdy @ 2020-07-17 19:19 UTC (permalink / raw) To: Tanu Kaskinen; +Cc: Adrian Bunk, Stefan Ghinea, OE Core mailing list On Fri, Jul 17, 2020 at 3:09 AM Tanu Kaskinen <tanuk@iki.fi> wrote: > On Thu, 2020-03-26 at 14:23 -0700, Andre McCurdy wrote: > > On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote: > > > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote: > > > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: > > > > > > ... > > > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > > > > > since the Thumb frame pointer in r7 > > > > > > ... > > > > > > > > > > How are you reproducing the problem in pulseaudio? > > > > > > > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago. > > > > > > > > The problem can show up anywhere that inline asm is trying to use r7. > > > > In this case it looks like: > > > > > > > > https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50 > > > > ... > > > > > > After looking at the pulseaudio code I suspected the patch description > > > claiming pulseaudio syscall code would be the problem was rubbish, and > > > that this NEON code was the problem. > > > > Yes, the comment looks like it was copied and pasted and doesn't > > really apply in this case (since pulseaudio isn't making syscalls). > > That should be updated. > > > > > But when I tried to reproduce the problem it built for me with both > > > glibc and musl in master (the patch didn't mention that this was a > > > musl-only problem). > > > > > > Then I saw that this was fixed in musl upstream 2 years ago: > > > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e > > > > Right, it's not related to musl or glibc. I suspect it can be > > reproduced by building for an ARM target which supports NEON, ensuring > > that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be > > armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to > > thumb and then compiling with frame pointers enabled (e.g. by adding > > -fno-omit-frame-pointer to CFLAGS). > > > > In terms of a fix, then changing the code to use r12 instead of r7 is > > probably the best solution (assuming it works), but would need careful > > testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines > > building for Thumb is safe and should fix the issue too. Presumably > > limiting the -fomit-frame-pointer workaround to ARM machines which > > support NEON building for Thumb would be an even more targeted > > solution. > > I finally found time to test fixing the assembly code to use r12 > instead of r7. Seems to work fine (I was first baffled by incorrect > behaviour, because I changed "{r4-r7}" to "{r4-r12}" without realizing > that "r4-r12" meant a range of all registers from r4 to r12). > > Can you enlighten me: why did you choose r12 instead of r8? Why did the > original author use registers r4-r7 instead of r0-r3? Is it somehow > advisable to avoid registers r0-r3 and r8-r11? The code seems to work > fine with any set of registers, except r7. The compiler will work around whichever set of registers you want to use (apart from r7 in some cases) so it's expected that other combinations will work fine. Some combinations will be more efficient than others (ie the compiler will need to do less shuffling values between registers or saving register values to the stack in order to make registers you specify available to you). Using r12 instead of r8 is just an educated guess about the combination will allow the compiler to generate the most efficient code. Registers r0-r3 and r12 can be used within a function without needing to preserve their previous contents, so if a function needs registers it's more efficient to use these registers first. Other registers need to be preserved (ie saved to the stack) before use. Registers r0-r3 are also used to pass non-floating point arguments to a function, so if a function takes 4 or more non-floating point arguments, then r0-r3 will already contain values which the function will need to use. Note that in this particular function, the first argument (ie the pointer m) is never actually used, so it may be that using r0, r4, r5 and r12 will give the best result. The function is pretty trivial though so I guess with a recent compiler just writing the whole thing in C will give close to the optimal result too without all the maintenance issues. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-07-17 19:19 ` Andre McCurdy @ 2020-07-18 4:09 ` Tanu Kaskinen 0 siblings, 0 replies; 12+ messages in thread From: Tanu Kaskinen @ 2020-07-18 4:09 UTC (permalink / raw) To: Andre McCurdy; +Cc: Adrian Bunk, Stefan Ghinea, OE Core mailing list On Fri, 2020-07-17 at 12:19 -0700, Andre McCurdy wrote: > On Fri, Jul 17, 2020 at 3:09 AM Tanu Kaskinen <tanuk@iki.fi> wrote: > > On Thu, 2020-03-26 at 14:23 -0700, Andre McCurdy wrote: > > > On Thu, Mar 26, 2020 at 1:26 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > On Thu, Mar 26, 2020 at 12:53:08PM -0700, Andre McCurdy wrote: > > > > > On Thu, Mar 26, 2020 at 12:16 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > > > On Thu, Mar 26, 2020 at 05:26:29PM +0200, Stefan Ghinea wrote: > > > > > > > ... > > > > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > > > > > > since the Thumb frame pointer in r7 > > > > > > > ... > > > > > > > > > > > > How are you reproducing the problem in pulseaudio? > > > > > > > > > > > > This sounds like a workaround for a bug in musl that was fixed 2 years ago. > > > > > > > > > > The problem can show up anywhere that inline asm is trying to use r7. > > > > > In this case it looks like: > > > > > > > > > > https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/remap_neon.c#L50 > > > > > ... > > > > > > > > After looking at the pulseaudio code I suspected the patch description > > > > claiming pulseaudio syscall code would be the problem was rubbish, and > > > > that this NEON code was the problem. > > > > > > Yes, the comment looks like it was copied and pasted and doesn't > > > really apply in this case (since pulseaudio isn't making syscalls). > > > That should be updated. > > > > > > > But when I tried to reproduce the problem it built for me with both > > > > glibc and musl in master (the patch didn't mention that this was a > > > > musl-only problem). > > > > > > > > Then I saw that this was fixed in musl upstream 2 years ago: > > > > https://git.musl-libc.org/cgit/musl/commit/?id=e3c682ab5257aaa6739ef242a9676d897370e78e > > > > > > Right, it's not related to musl or glibc. I suspect it can be > > > reproduced by building for an ARM target which supports NEON, ensuring > > > that DEFAULTTUNE doesn't forcefully disable Thumb (e.g. it should be > > > armv7vethf-neon, not armv7vehf-neon), setting ARM_INSTRUCTION_SET to > > > thumb and then compiling with frame pointers enabled (e.g. by adding > > > -fno-omit-frame-pointer to CFLAGS). > > > > > > In terms of a fix, then changing the code to use r12 instead of r7 is > > > probably the best solution (assuming it works), but would need careful > > > testing. Appending -fomit-frame-pointer to CFLAGS for ARM machines > > > building for Thumb is safe and should fix the issue too. Presumably > > > limiting the -fomit-frame-pointer workaround to ARM machines which > > > support NEON building for Thumb would be an even more targeted > > > solution. > > > > I finally found time to test fixing the assembly code to use r12 > > instead of r7. Seems to work fine (I was first baffled by incorrect > > behaviour, because I changed "{r4-r7}" to "{r4-r12}" without realizing > > that "r4-r12" meant a range of all registers from r4 to r12). > > > > Can you enlighten me: why did you choose r12 instead of r8? Why did the > > original author use registers r4-r7 instead of r0-r3? Is it somehow > > advisable to avoid registers r0-r3 and r8-r11? The code seems to work > > fine with any set of registers, except r7. > > The compiler will work around whichever set of registers you want to > use (apart from r7 in some cases) so it's expected that other > combinations will work fine. Some combinations will be more efficient > than others (ie the compiler will need to do less shuffling values > between registers or saving register values to the stack in order to > make registers you specify available to you). Using r12 instead of r8 > is just an educated guess about the combination will allow the > compiler to generate the most efficient code. > > Registers r0-r3 and r12 can be used within a function without needing > to preserve their previous contents, so if a function needs registers > it's more efficient to use these registers first. Other registers need > to be preserved (ie saved to the stack) before use. > > Registers r0-r3 are also used to pass non-floating point arguments to > a function, so if a function takes 4 or more non-floating point > arguments, then r0-r3 will already contain values which the function > will need to use. > > Note that in this particular function, the first argument (ie the > pointer m) is never actually used, so it may be that using r0, r4, r5 > and r12 will give the best result. The function is pretty trivial > though so I guess with a recent compiler just writing the whole thing > in C will give close to the optimal result too without all the > maintenance issues. Thanks for explaining! -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error 2020-03-26 15:26 [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error Stefan Ghinea 2020-03-26 17:23 ` [OE-core] " Andre McCurdy 2020-03-26 19:16 ` Adrian Bunk @ 2020-03-30 16:29 ` Tanu Kaskinen [not found] ` <160121D94406B75D.27565@lists.openembedded.org> 3 siblings, 0 replies; 12+ messages in thread From: Tanu Kaskinen @ 2020-03-30 16:29 UTC (permalink / raw) To: Stefan Ghinea, openembedded-core On Thu, 2020-03-26 at 17:26 +0200, Stefan Ghinea wrote: > From: Catalin Enache <catalin.enache@windriver.com> > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline > asm to make syscalls (where r7 is used for the syscall NR). Do you have a pointer to the asm code that does syscalls? I'm an upstream maintainer of PulseAudio, and to me it sounds a bit strange that there would be inline asm that does syscalls. It would be great to fix this problem in upstream (either by applying this fix/workaround in the build system, or by modifying the asm code so that the problem goes away). -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <160121D94406B75D.27565@lists.openembedded.org>]
* Re: [OE-core] [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error [not found] ` <160121D94406B75D.27565@lists.openembedded.org> @ 2020-03-30 16:52 ` Tanu Kaskinen 0 siblings, 0 replies; 12+ messages in thread From: Tanu Kaskinen @ 2020-03-30 16:52 UTC (permalink / raw) To: Stefan Ghinea, openembedded-core On Mon, 2020-03-30 at 19:29 +0300, Tanu Kaskinen wrote: > On Thu, 2020-03-26 at 17:26 +0200, Stefan Ghinea wrote: > > From: Catalin Enache <catalin.enache@windriver.com> > > > > When compiling for Thumb or Thumb2, frame pointers _must_ be disabled > > since the Thumb frame pointer in r7 clashes with pulseaudio's use of inline > > asm to make syscalls (where r7 is used for the syscall NR). > > Do you have a pointer to the asm code that does syscalls? I'm an > upstream maintainer of PulseAudio, and to me it sounds a bit strange > that there would be inline asm that does syscalls. > > It would be great to fix this problem in upstream (either by applying > this fix/workaround in the build system, or by modifying the asm code > so that the problem goes away). I forgot to check if others had already commented on this... So the real problem is that src/pulsecore/remap_neon.c uses the r7 register in a few places. Andre McCurdy said: "In terms of a fix, then changing the code to use r12 instead of r7 is probably the best solution (assuming it works), but would need careful testing." I'll try this. -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-07-18 4:09 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-26 15:26 [PATCH] pulseaudio: fix for ARM thumb + frame pointers compilation error Stefan Ghinea
2020-03-26 17:23 ` [OE-core] " Andre McCurdy
2020-03-26 19:16 ` Adrian Bunk
2020-03-26 19:53 ` Andre McCurdy
2020-03-26 20:26 ` Adrian Bunk
2020-03-26 21:23 ` Andre McCurdy
2020-03-27 18:56 ` Adrian Bunk
2020-07-17 10:09 ` Tanu Kaskinen
2020-07-17 19:19 ` Andre McCurdy
2020-07-18 4:09 ` Tanu Kaskinen
2020-03-30 16:29 ` Tanu Kaskinen
[not found] ` <160121D94406B75D.27565@lists.openembedded.org>
2020-03-30 16:52 ` Tanu Kaskinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox