All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Paul Menzel <pmenzel@molgen.mpg.de>
Cc: Nathan Chancellor <nathan@kernel.org>,
	linuxppc-dev@lists.ozlabs.org,
	clang-built-linux@googlegroups.com
Subject: Re: [PATCH] powerpc/vdso: Don't use r30 to avoid breaking Go lang
Date: Thu, 29 Jul 2021 09:25:43 -0700	[thread overview]
Message-ID: <CAKwvOdmR9nB3T-PLNSGea3=9qbZuOoLs5D22UPRBbvw_+aOO7A@mail.gmail.com> (raw)
In-Reply-To: <392b3b47-1878-d27a-9656-4596fdc7c343@molgen.mpg.de>

On Thu, Jul 29, 2021 at 6:42 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Michael,
>
>
> Am 29.07.21 um 15:12 schrieb Michael Ellerman:
> > The Go runtime uses r30 for some special value called 'g'. It assumes
> > that value will remain unchanged even when calling VDSO functions.
> > Although r30 is non-volatile across function calls, the callee is free
> > to use it, as long as the callee saves the value and restores it before
> > returning.
> >
> > It used to be true by accident that the VDSO didn't use r30, because the
> > VDSO was hand-written asm. When we switched to building the VDSO from C
> > the compiler started using r30, at least in some builds, leading to
> > crashes in Go. eg:
> >
> >    ~/go/src$ ./all.bash
> >    Building Go cmd/dist using /usr/lib/go-1.16. (go1.16.2 linux/ppc64le)
> >    Building Go toolchain1 using /usr/lib/go-1.16.
> >    go build os/exec: /usr/lib/go-1.16/pkg/tool/linux_ppc64le/compile: signal: segmentation fault
> >    go build reflect: /usr/lib/go-1.16/pkg/tool/linux_ppc64le/compile: signal: segmentation fault
> >    go tool dist: FAILED: /usr/lib/go-1.16/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 1
> >
> > There are patches in flight to fix Go[1], but until they are released
> > and widely deployed we can workaround it in the VDSO by avoiding use of
>
> Nit: work around is spelled with a space.
>
> > r30.
> >
> > Note this only works with GCC, clang does not support -ffixed-rN.
>
> Maybe the clang/LLVM build support folks (in CC) have an idea.

Right, we've had issues with these in the past.  Generally, we need to
teach clang about which registers are valid for `N` so that it can
diagnose invalid values ASAP.  This has to be done on a per arch basis
in LLVM to steal the register from the register allocator.  For
example, this was used previously for aarch64 (but removed from use in
the kernel) and IIRC is used for m68k (which we're working to get
builds online for).

I've filed https://bugs.llvm.org/show_bug.cgi?id=51272. Thanks for the report.

>
> > 1: https://go-review.googlesource.com/c/go/+/328110
> >
> > Fixes: ab037dd87a2f ("powerpc/vdso: Switch VDSO to generic C implementation.")
> > Cc: stable@vger.kernel.org # v5.11+
> > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> > Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> > ---
> >   arch/powerpc/kernel/vdso64/Makefile | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> > index 2813e3f98db6..3c5baaa6f1e7 100644
> > --- a/arch/powerpc/kernel/vdso64/Makefile
> > +++ b/arch/powerpc/kernel/vdso64/Makefile
> > @@ -27,6 +27,13 @@ KASAN_SANITIZE := n
> >
> >   ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> >       -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
> > +
> > +# Go prior to 1.16.x assumes r30 is not clobbered by any VDSO code. That used to be true
> > +# by accident when the VDSO was hand-written asm code, but may not be now that the VDSO is
> > +# compiler generated. To avoid breaking Go tell GCC not to use r30. Impact on code
> > +# generation is minimal, it will just use r29 instead.
> > +ccflags-y += $(call cc-option, -ffixed-r30)
> > +
> >   asflags-y := -D__VDSO64__ -s
> >
> >   targets += vdso64.lds
> >
>
> The rest looks good.
>
>
> Kind regards,
>
> Paul



-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2021-07-29 16:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 13:12 Michael Ellerman
2021-07-29 13:42 ` Paul Menzel
2021-07-29 16:25   ` Nick Desaulniers [this message]
2021-07-30 23:55     ` Segher Boessenkool
2021-08-01 13:14 ` Michael Ellerman
2021-08-02 11:27 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKwvOdmR9nB3T-PLNSGea3=9qbZuOoLs5D22UPRBbvw_+aOO7A@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=nathan@kernel.org \
    --cc=pmenzel@molgen.mpg.de \
    --subject='Re: [PATCH] powerpc/vdso: Don'\''t use r30 to avoid breaking Go lang' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.