* [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
@ 2020-02-05 0:50 Fangrui Song
2020-02-05 6:25 ` Christophe Leroy
2020-02-10 11:01 ` Michael Ellerman
0 siblings, 2 replies; 6+ messages in thread
From: Fangrui Song @ 2020-02-05 0:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Christophe Leroy
Cc: linuxppc-dev, linux-kernel, clang-built-linux, Fangrui Song
A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
preemptible symbol in a -shared link is not allowed. GNU ld's powerpc
port is permissive and allows it [1], but lld will report an error after
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
Make the symbol protected so that it is non-preemptible but still
exported.
[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=25500
Link: https://github.com/ClangBuiltLinux/linux/issues/851
Signed-off-by: Fangrui Song <maskray@google.com>
---
arch/powerpc/kernel/vdso32/datapage.S | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
index 217bb630f8f9..2831a8676365 100644
--- a/arch/powerpc/kernel/vdso32/datapage.S
+++ b/arch/powerpc/kernel/vdso32/datapage.S
@@ -13,7 +13,8 @@
#include <asm/vdso_datapage.h>
.text
- .global __kernel_datapage_offset;
+ .global __kernel_datapage_offset
+ .protected __kernel_datapage_offset
__kernel_datapage_offset:
.long 0
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
2020-02-05 0:50 [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED Fangrui Song
@ 2020-02-05 6:25 ` Christophe Leroy
2020-02-07 6:42 ` Nathan Chancellor
2020-02-10 11:01 ` Michael Ellerman
1 sibling, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2020-02-05 6:25 UTC (permalink / raw)
To: Fangrui Song, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel, clang-built-linux
Le 05/02/2020 à 01:50, Fangrui Song a écrit :
> A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> preemptible symbol in a -shared link is not allowed. GNU ld's powerpc
> port is permissive and allows it [1], but lld will report an error after
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
Note that there is a series whose first two patches aim at dropping
__kernel_datapage_offset . See
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=156045
and especially patches https://patchwork.ozlabs.org/patch/1231467/ and
https://patchwork.ozlabs.org/patch/1231461/
Those patches can be applied independentely of the rest.
Christophe
>
> Make the symbol protected so that it is non-preemptible but still
> exported.
>
> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=25500
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/851
> Signed-off-by: Fangrui Song <maskray@google.com>
> ---
> arch/powerpc/kernel/vdso32/datapage.S | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
> index 217bb630f8f9..2831a8676365 100644
> --- a/arch/powerpc/kernel/vdso32/datapage.S
> +++ b/arch/powerpc/kernel/vdso32/datapage.S
> @@ -13,7 +13,8 @@
> #include <asm/vdso_datapage.h>
>
> .text
> - .global __kernel_datapage_offset;
> + .global __kernel_datapage_offset
> + .protected __kernel_datapage_offset
> __kernel_datapage_offset:
> .long 0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
2020-02-05 6:25 ` Christophe Leroy
@ 2020-02-07 6:42 ` Nathan Chancellor
2020-06-03 23:50 ` Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2020-02-07 6:42 UTC (permalink / raw)
To: Christophe Leroy
Cc: Fangrui Song, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, linuxppc-dev, linux-kernel, clang-built-linux
On Wed, Feb 05, 2020 at 07:25:59AM +0100, Christophe Leroy wrote:
>
>
> Le 05/02/2020 à 01:50, Fangrui Song a écrit :
> > A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> > preemptible symbol in a -shared link is not allowed. GNU ld's powerpc
> > port is permissive and allows it [1], but lld will report an error after
> > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
>
> Note that there is a series whose first two patches aim at dropping
> __kernel_datapage_offset . See
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=156045 and
> especially patches https://patchwork.ozlabs.org/patch/1231467/ and
> https://patchwork.ozlabs.org/patch/1231461/
>
> Those patches can be applied independentely of the rest.
>
> Christophe
If that is the case, it would be nice if those could be fast tracked to
5.6 because as it stands now, all PowerPC builds that were working with
ld.lld are now broken. Either that or take this patch and rebase that
series on this one.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
2020-02-05 0:50 [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED Fangrui Song
2020-02-05 6:25 ` Christophe Leroy
@ 2020-02-10 11:01 ` Michael Ellerman
2020-02-10 18:41 ` Fāng-ruì Sòng
1 sibling, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2020-02-10 11:01 UTC (permalink / raw)
To: Fangrui Song, Benjamin Herrenschmidt, Paul Mackerras, Christophe Leroy
Cc: linuxppc-dev, linux-kernel, clang-built-linux, Fangrui Song
Fangrui Song <maskray@google.com> writes:
> A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> preemptible symbol in a -shared link is not allowed. GNU ld's powerpc
> port is permissive and allows it [1], but lld will report an error after
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
>
> Make the symbol protected so that it is non-preemptible but still
> exported.
"preemptible" means something different to me, and I assume we're not
using it to mean the same thing.
Can you explain it using small words that a kernel developer can
understand? :)
cheers
> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=25500
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/851
> Signed-off-by: Fangrui Song <maskray@google.com>
> ---
> arch/powerpc/kernel/vdso32/datapage.S | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
> index 217bb630f8f9..2831a8676365 100644
> --- a/arch/powerpc/kernel/vdso32/datapage.S
> +++ b/arch/powerpc/kernel/vdso32/datapage.S
> @@ -13,7 +13,8 @@
> #include <asm/vdso_datapage.h>
>
> .text
> - .global __kernel_datapage_offset;
> + .global __kernel_datapage_offset
> + .protected __kernel_datapage_offset
> __kernel_datapage_offset:
> .long 0
>
> --
> 2.25.0.341.g760bfbb309-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
2020-02-10 11:01 ` Michael Ellerman
@ 2020-02-10 18:41 ` Fāng-ruì Sòng
0 siblings, 0 replies; 6+ messages in thread
From: Fāng-ruì Sòng @ 2020-02-10 18:41 UTC (permalink / raw)
To: Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, Christophe Leroy,
linuxppc-dev, linux-kernel, clang-built-linux
On Mon, Feb 10, 2020 at 3:01 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Fangrui Song <maskray@google.com> writes:
> > A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> > preemptible symbol in a -shared link is not allowed. GNU ld's powerpc
> > port is permissive and allows it [1], but lld will report an error after
> > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
> >
> > Make the symbol protected so that it is non-preemptible but still
> > exported.
>
> "preemptible" means something different to me, and I assume we're not
> using it to mean the same thing.
>
> Can you explain it using small words that a kernel developer can
> understand? :)
>
> cheers
The term used in the ELF specification is "preemptable". I heard from
Roland McGrathr that "preemptable" was a typo. The correct term is
"preemptible".
On a random article I found, it mentions that "preemptible" is used
more than "preemptable". So now I stick with "preemptible".
The word is overloaded and has a different meaning in the kernel, but
here we refer to within the ELF binary format context.
From http://www.sco.com/developers/gabi/latest/ch4.symtab.html
"The visibility of symbols with the STV_DEFAULT attribute is as
specified by the symbol's binding type. That is, global and weak
symbols are visible outside of their defining component (executable
file or shared object). Local symbols are hidden, as described below.
Global and weak symbols are also preemptable, that is, they may by
preempted by definitions of the same name in another component."
__kernel_datapage_offset is a STB_GLOBAL STV_DEFAULT symbol. In a
-shared link, it is considered preemptible. There are some methods
that make such symbols non-preemptible but none is used in this
context.
* -Bsymbolic
* -Bsymbolic-functions if STT_FUNC
* --dynamic-list is specified but the dynamic list does not name this symbol
* A --version-script makes the symbol local
__kernel_datapage_offset is accessed via some mechanism similar to
dlsym, so it has to be exported.
Given all the above, I chose STV_PROTECTED, which is the simplest and
least intrusive approach.
> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=25500
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/851
> > Signed-off-by: Fangrui Song <maskray@google.com>
>
> > ---
> > arch/powerpc/kernel/vdso32/datapage.S | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
> > index 217bb630f8f9..2831a8676365 100644
> > --- a/arch/powerpc/kernel/vdso32/datapage.S
> > +++ b/arch/powerpc/kernel/vdso32/datapage.S
> > @@ -13,7 +13,8 @@
> > #include <asm/vdso_datapage.h>
> >
> > .text
> > - .global __kernel_datapage_offset;
> > + .global __kernel_datapage_offset
> > + .protected __kernel_datapage_offset
> > __kernel_datapage_offset:
> > .long 0
> >
> > --
> > 2.25.0.341.g760bfbb309-goog
--
宋方睿
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
2020-02-07 6:42 ` Nathan Chancellor
@ 2020-06-03 23:50 ` Nick Desaulniers
0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2020-06-03 23:50 UTC (permalink / raw)
To: Nathan Chancellor, Christophe Leroy, Fangrui Song, Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, LKML,
clang-built-linux
On Thu, Feb 6, 2020 at 10:42 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Wed, Feb 05, 2020 at 07:25:59AM +0100, Christophe Leroy wrote:
> >
> >
> > Le 05/02/2020 à 01:50, Fangrui Song a écrit :
> > > A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> > > preemptible symbol in a -shared link is not allowed. GNU ld's powerpc
> > > port is permissive and allows it [1], but lld will report an error after
> > > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
> >
> > Note that there is a series whose first two patches aim at dropping
> > __kernel_datapage_offset . See
> > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=156045 and
> > especially patches https://patchwork.ozlabs.org/patch/1231467/ and
> > https://patchwork.ozlabs.org/patch/1231461/
> >
> > Those patches can be applied independentely of the rest.
> >
> > Christophe
>
> If that is the case, it would be nice if those could be fast tracked to
> 5.6 because as it stands now, all PowerPC builds that were working with
> ld.lld are now broken. Either that or take this patch and rebase that
> series on this one.
So do we still need Fangrui's patch or is it moot? I'm doing a scrub
of our bug tracker and this issue is still open:
https://github.com/ClangBuiltLinux/linux/issues/851
but it looks like all of our ppc LE targets are linking with LLD just fine
https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/builds/169379039
though it sounds like
https://github.com/ClangBuiltLinux/linux/issues/774
may be a blocker?
Though I don't see Cristophe's
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/5f97f7c921ffc2113ada0f32924e409bccc8277a.1580399657.git.christophe.leroy@c-s.fr/
in mainline or -next. Was the series not accepted?
>
> Cheers,
> Nathan
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200207064210.GA13125%40ubuntu-x2-xlarge-x86.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-03 23:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-05 0:50 [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED Fangrui Song
2020-02-05 6:25 ` Christophe Leroy
2020-02-07 6:42 ` Nathan Chancellor
2020-06-03 23:50 ` Nick Desaulniers
2020-02-10 11:01 ` Michael Ellerman
2020-02-10 18:41 ` Fāng-ruì Sòng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).