diff mbox series

[v3,2/5] Make libc symbols hidden in static PIE

Message ID 27498bbc768372541e4379794656ac2778d33035.1610471272.git.szabolcs.nagy@arm.com
State Superseded
Headers show
Series fix ifunc with static pie [BZ #27072] | expand

Commit Message

Szabolcs Nagy Jan. 12, 2021, 5:22 p.m. UTC
Hidden matters with static PIE: extern symbol access in position
independent code usually involves GOT indirections which needs
RELATIVE relocs in a static linked PIE. Using So hidden avoids
indirections and RELATIVE relocs on targets that can access symbols
pc-relative.

The check should use IS_IN_LIB instead of IS_IN(libc) since all
static libraries can use hidden visibility to avoid indirections,
however the test system links objects from libcrypt.a into dynamic
linked test binaries so hidden does not work there.  I think mixing
static and shared libc components in the same binary should not be
supported usage, but to be safe only use hidden in libc.a.

From -static-pie linked 'int main(){}' this shaves off 73 relative
relocs on aarch64 and reduces code size too.
---
 include/libc-symbols.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

H.J. Lu Jan. 12, 2021, 11:09 p.m. UTC | #1
On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Hidden matters with static PIE: extern symbol access in position
> independent code usually involves GOT indirections which needs
> RELATIVE relocs in a static linked PIE. Using So hidden avoids
> indirections and RELATIVE relocs on targets that can access symbols
> pc-relative.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> static libraries can use hidden visibility to avoid indirections,
> however the test system links objects from libcrypt.a into dynamic
> linked test binaries so hidden does not work there.  I think mixing
> static and shared libc components in the same binary should not be
> supported usage, but to be safe only use hidden in libc.a.
>
> From -static-pie linked 'int main(){}' this shaves off 73 relative
> relocs on aarch64 and reduces code size too.
> ---
>  include/libc-symbols.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index ea126ae70c..93e63ee889 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -434,13 +434,17 @@ for linking")
>    strong_alias(real, name)
>  #endif
>
> -#if defined SHARED || defined LIBC_NONSHARED \
> -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> +#if defined SHARED || defined LIBC_NONSHARED
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>  #else
>  # define attribute_hidden
>  #endif
>
> +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> +# pragma GCC visibility push(hidden)
> +#endif
> +
>  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
>
>  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> --
> 2.17.1
>

Unfortunately it doesn't work on i686 with --enable-static-pie:

/usr/local/bin/ld:
/export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
unsupported non-PIC call to IFUNC `strcmp'

Please make it opt-out.
H.J. Lu Jan. 13, 2021, 12:02 a.m. UTC | #2
On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > Hidden matters with static PIE: extern symbol access in position
> > independent code usually involves GOT indirections which needs
> > RELATIVE relocs in a static linked PIE. Using So hidden avoids
> > indirections and RELATIVE relocs on targets that can access symbols
> > pc-relative.
> >
> > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > static libraries can use hidden visibility to avoid indirections,
> > however the test system links objects from libcrypt.a into dynamic
> > linked test binaries so hidden does not work there.  I think mixing
> > static and shared libc components in the same binary should not be
> > supported usage, but to be safe only use hidden in libc.a.
> >
> > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > relocs on aarch64 and reduces code size too.
> > ---
> >  include/libc-symbols.h | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> > index ea126ae70c..93e63ee889 100644
> > --- a/include/libc-symbols.h
> > +++ b/include/libc-symbols.h
> > @@ -434,13 +434,17 @@ for linking")
> >    strong_alias(real, name)
> >  #endif
> >
> > -#if defined SHARED || defined LIBC_NONSHARED \
> > -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> > +#if defined SHARED || defined LIBC_NONSHARED
> >  # define attribute_hidden __attribute__ ((visibility ("hidden")))
> >  #else
> >  # define attribute_hidden
> >  #endif
> >
> > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > +# pragma GCC visibility push(hidden)
> > +#endif
> > +
> >  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
> >
> >  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> > --
> > 2.17.1
> >
>
> Unfortunately it doesn't work on i686 with --enable-static-pie:
>
> /usr/local/bin/ld:
> /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> unsupported non-PIC call to IFUNC `strcmp'
>
> Please make it opt-out.
>

See:

https://sourceware.org/bugzilla/show_bug.cgi?id=14961
H.J. Lu Jan. 13, 2021, 12:33 a.m. UTC | #3
On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > Hidden matters with static PIE: extern symbol access in position
> > > independent code usually involves GOT indirections which needs
> > > RELATIVE relocs in a static linked PIE. Using So hidden avoids
> > > indirections and RELATIVE relocs on targets that can access symbols
> > > pc-relative.
> > >
> > > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > > static libraries can use hidden visibility to avoid indirections,
> > > however the test system links objects from libcrypt.a into dynamic
> > > linked test binaries so hidden does not work there.  I think mixing
> > > static and shared libc components in the same binary should not be
> > > supported usage, but to be safe only use hidden in libc.a.
> > >
> > > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > > relocs on aarch64 and reduces code size too.
> > > ---
> > >  include/libc-symbols.h | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> > > index ea126ae70c..93e63ee889 100644
> > > --- a/include/libc-symbols.h
> > > +++ b/include/libc-symbols.h
> > > @@ -434,13 +434,17 @@ for linking")
> > >    strong_alias(real, name)
> > >  #endif
> > >
> > > -#if defined SHARED || defined LIBC_NONSHARED \
> > > -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> > > +#if defined SHARED || defined LIBC_NONSHARED
> > >  # define attribute_hidden __attribute__ ((visibility ("hidden")))
> > >  #else
> > >  # define attribute_hidden
> > >  #endif
> > >
> > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > +# pragma GCC visibility push(hidden)
> > > +#endif
> > > +
> > >  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
> > >
> > >  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> > > --
> > > 2.17.1
> > >
> >
> > Unfortunately it doesn't work on i686 with --enable-static-pie:
> >
> > /usr/local/bin/ld:
> > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> > unsupported non-PIC call to IFUNC `strcmp'
> >
> > Please make it opt-out.
> >
>
> See:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=14961
>
> --
> H.J.

Testing:

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index 93e63ee889..f4dd735555 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -441,7 +441,8 @@ for linking")
 #endif

 /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
-#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
+#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+    && IS_IN (libc) && !defined LIBC_NONSHARED
 # pragma GCC visibility push(hidden)
 #endif
H.J. Lu Jan. 13, 2021, 1:19 a.m. UTC | #4
On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
> > > <libc-alpha@sourceware.org> wrote:
> > > >
> > > > Hidden matters with static PIE: extern symbol access in position
> > > > independent code usually involves GOT indirections which needs
> > > > RELATIVE relocs in a static linked PIE. Using So hidden avoids
> > > > indirections and RELATIVE relocs on targets that can access symbols
> > > > pc-relative.
> > > >
> > > > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > > > static libraries can use hidden visibility to avoid indirections,
> > > > however the test system links objects from libcrypt.a into dynamic
> > > > linked test binaries so hidden does not work there.  I think mixing
> > > > static and shared libc components in the same binary should not be
> > > > supported usage, but to be safe only use hidden in libc.a.
> > > >
> > > > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > > > relocs on aarch64 and reduces code size too.
> > > > ---
> > > >  include/libc-symbols.h | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> > > > index ea126ae70c..93e63ee889 100644
> > > > --- a/include/libc-symbols.h
> > > > +++ b/include/libc-symbols.h
> > > > @@ -434,13 +434,17 @@ for linking")
> > > >    strong_alias(real, name)
> > > >  #endif
> > > >
> > > > -#if defined SHARED || defined LIBC_NONSHARED \
> > > > -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> > > > +#if defined SHARED || defined LIBC_NONSHARED
> > > >  # define attribute_hidden __attribute__ ((visibility ("hidden")))
> > > >  #else
> > > >  # define attribute_hidden
> > > >  #endif
> > > >
> > > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > +# pragma GCC visibility push(hidden)
> > > > +#endif
> > > > +
> > > >  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
> > > >
> > > >  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> > > > --
> > > > 2.17.1
> > > >
> > >
> > > Unfortunately it doesn't work on i686 with --enable-static-pie:
> > >
> > > /usr/local/bin/ld:
> > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> > > unsupported non-PIC call to IFUNC `strcmp'
> > >
> > > Please make it opt-out.
> > >
> >
> > See:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> >
> > --
> > H.J.
>
> Testing:
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index 93e63ee889..f4dd735555 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -441,7 +441,8 @@ for linking")
>  #endif
>
>  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> +    && IS_IN (libc) && !defined LIBC_NONSHARED
>  # pragma GCC visibility push(hidden)
>  #endif
>

This works on i686.
Szabolcs Nagy Jan. 13, 2021, 9:50 a.m. UTC | #5
The 01/12/2021 17:19, H.J. Lu wrote:
> On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > Unfortunately it doesn't work on i686 with --enable-static-pie:
> > > >
> > > > /usr/local/bin/ld:
> > > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> > > > unsupported non-PIC call to IFUNC `strcmp'
> > > >
> > > > Please make it opt-out.
> > > >
> > >
> > > See:
> > >
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > >
> >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> >  # pragma GCC visibility push(hidden)
> >  #endif
> >
> 
> This works on i686.

if it works then likely pie uses copy relocs on i686
so object symbols are always accessed locally instead
of via GOT. this sounds a bit fragile.

ideally there would be a way to mark object symbols
hidden but not functions for such targets.

i will add this change for now, thanks.
Szabolcs Nagy Jan. 14, 2021, 11:17 a.m. UTC | #6
The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> The 01/12/2021 17:19, H.J. Lu wrote:
> > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > See:
> > > >
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > >
> > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > >  # pragma GCC visibility push(hidden)
> > >  #endif
> > >
> > 
> > This works on i686.

The series i plan to commit today is in nsz/bug27072 now,

This is the v4 of this patch:

Hidden matters with static PIE: extern symbol access in position
independent code usually involves GOT indirections which needs
RELATIVE relocs in a static linked PIE. Hidden visibility avoids
indirections and RELATIVE relocs on targets that can access symbols
pc-relative.

The check should use IS_IN_LIB instead of IS_IN(libc) since all
static libraries can use hidden visibility to avoid indirections,
however the test system links objects from libcrypt.a into dynamic
linked test binaries so hidden does not work there.  I think mixing
static and shared libc components in the same binary should not be
supported usage, but to be safe only use hidden in libc.a.

There are targets (i686) where hidden visibility functions are
problematic in PIE code so hidden cannot be applied to all symbols.
Then static PIE requires extern object access without relocations
(e.g. by relying on copy relocations in shared libraries instead of
GOT access in PIE code). See bug 14961.

From -static-pie linked 'int main(){}' this shaves off 73 relative
relocs on aarch64 and reduces code size too.
---
 include/libc-symbols.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index ea126ae70c..f4dd735555 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,13 +434,18 @@ for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED \
-  || (BUILD_PIE_DEFAULT && IS_IN (libc))
+#if defined SHARED || defined LIBC_NONSHARED
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
 #endif
 
+/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
+#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+    && IS_IN (libc) && !defined LIBC_NONSHARED
+# pragma GCC visibility push(hidden)
+#endif
+
 #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
 
 #define attribute_relro __attribute__ ((section (".data.rel.ro")))
H.J. Lu Jan. 14, 2021, 3:39 p.m. UTC | #7
On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > The 01/12/2021 17:19, H.J. Lu wrote:
> > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > See:
> > > > >
> > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > > >
> > > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > > >  # pragma GCC visibility push(hidden)
> > > >  #endif
> > > >
> > >
> > > This works on i686.
>
> The series i plan to commit today is in nsz/bug27072 now,
>
> This is the v4 of this patch:
>
> Hidden matters with static PIE: extern symbol access in position
> independent code usually involves GOT indirections which needs
> RELATIVE relocs in a static linked PIE. Hidden visibility avoids
> indirections and RELATIVE relocs on targets that can access symbols
> pc-relative.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> static libraries can use hidden visibility to avoid indirections,
> however the test system links objects from libcrypt.a into dynamic
> linked test binaries so hidden does not work there.  I think mixing
> static and shared libc components in the same binary should not be
> supported usage, but to be safe only use hidden in libc.a.
>
> There are targets (i686) where hidden visibility functions are
> problematic in PIE code so hidden cannot be applied to all symbols.
> Then static PIE requires extern object access without relocations
> (e.g. by relying on copy relocations in shared libraries instead of
> GOT access in PIE code). See bug 14961.
>
> From -static-pie linked 'int main(){}' this shaves off 73 relative
> relocs on aarch64 and reduces code size too.
> ---
>  include/libc-symbols.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index ea126ae70c..f4dd735555 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -434,13 +434,18 @@ for linking")
>    strong_alias(real, name)
>  #endif
>
> -#if defined SHARED || defined LIBC_NONSHARED \
> -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> +#if defined SHARED || defined LIBC_NONSHARED
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>  #else
>  # define attribute_hidden
>  #endif
>
> +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> +    && IS_IN (libc) && !defined LIBC_NONSHARED
> +# pragma GCC visibility push(hidden)
> +#endif
> +
>  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
>
>  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> --
> 2.17.1
>

This generates bad static PIE on i386.   This patch is needed:

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index f4dd735555..72276a5c48 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,7 +434,9 @@ for linking")
   strong_alias(real, name)
 #endif

-#if defined SHARED || defined LIBC_NONSHARED
+#if defined SHARED || defined LIBC_NONSHARED \
+  || !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+  || (BUILD_PIE_DEFAULT && IS_IN (libc))
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
H.J. Lu Jan. 15, 2021, 3:36 a.m. UTC | #8
On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > The 01/12/2021 17:19, H.J. Lu wrote:
> > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > See:
> > > > >
> > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > > >
> > > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > > >  # pragma GCC visibility push(hidden)
> > > >  #endif
> > > >
> > >
> > > This works on i686.
>
> The series i plan to commit today is in nsz/bug27072 now,
>
> This is the v4 of this patch:
>
> Hidden matters with static PIE: extern symbol access in position
> independent code usually involves GOT indirections which needs
> RELATIVE relocs in a static linked PIE. Hidden visibility avoids
> indirections and RELATIVE relocs on targets that can access symbols
> pc-relative.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> static libraries can use hidden visibility to avoid indirections,
> however the test system links objects from libcrypt.a into dynamic
> linked test binaries so hidden does not work there.  I think mixing
> static and shared libc components in the same binary should not be
> supported usage, but to be safe only use hidden in libc.a.
>
> There are targets (i686) where hidden visibility functions are
> problematic in PIE code so hidden cannot be applied to all symbols.
> Then static PIE requires extern object access without relocations
> (e.g. by relying on copy relocations in shared libraries instead of
> GOT access in PIE code). See bug 14961.

It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
additional patches to make static PIE to work on i386 and x86-64.
I am enclosing my patches.  Please include them in your patch set.

> From -static-pie linked 'int main(){}' this shaves off 73 relative
> relocs on aarch64 and reduces code size too.
H.J. Lu Jan. 15, 2021, 4:29 a.m. UTC | #9
On Thu, Jan 14, 2021 at 7:36 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > > The 01/12/2021 17:19, H.J. Lu wrote:
> > > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > See:
> > > > > >
> > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > > > >
> > > > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > >  # pragma GCC visibility push(hidden)
> > > > >  #endif
> > > > >
> > > >
> > > > This works on i686.
> >
> > The series i plan to commit today is in nsz/bug27072 now,
> >
> > This is the v4 of this patch:
> >
> > Hidden matters with static PIE: extern symbol access in position
> > independent code usually involves GOT indirections which needs
> > RELATIVE relocs in a static linked PIE. Hidden visibility avoids
> > indirections and RELATIVE relocs on targets that can access symbols
> > pc-relative.
> >
> > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > static libraries can use hidden visibility to avoid indirections,
> > however the test system links objects from libcrypt.a into dynamic
> > linked test binaries so hidden does not work there.  I think mixing
> > static and shared libc components in the same binary should not be
> > supported usage, but to be safe only use hidden in libc.a.
> >
> > There are targets (i686) where hidden visibility functions are
> > problematic in PIE code so hidden cannot be applied to all symbols.
> > Then static PIE requires extern object access without relocations
> > (e.g. by relying on copy relocations in shared libraries instead of
> > GOT access in PIE code). See bug 14961.
>
> It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
> additional patches to make static PIE to work on i386 and x86-64.
> I am enclosing my patches.  Please include them in your patch set.
>
> > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > relocs on aarch64 and reduces code size too.
>
>
> --
> H.J.

commit c5ffa46591550d945b009f0e3bcf66603d48ac0b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Jan 14 13:26:29 2021 -0800

    i386: Call _dl_aux_init after relocating static PIE

is too complicated.  I will submit a simple version.
Szabolcs Nagy Jan. 15, 2021, 11:25 a.m. UTC | #10
The 01/14/2021 19:36, H.J. Lu wrote:
> On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > There are targets (i686) where hidden visibility functions are
> > problematic in PIE code so hidden cannot be applied to all symbols.
> > Then static PIE requires extern object access without relocations
> > (e.g. by relying on copy relocations in shared libraries instead of
> > GOT access in PIE code). See bug 14961.
> 
> It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
> additional patches to make static PIE to work on i386 and x86-64.
> I am enclosing my patches.  Please include them in your patch set.

it is about avoiding GOT for object access, which copy
relocations can do. hidden does it better, but you said
that does not work on i686 so i removed it (assuming
you know the implications: every pie object access must
be local and copy relocated in shared libraries)

morally all symbols should be hidden in static pie
because everything is local (the code is only linked
into static exectuables). this is useful outside the
start code too to avoid GOT indirections.

i686 does not want to set up EBX for hidden extern calls,
which is needed for ifuncs, so making everything hidden
does not work.

options:

(1) fix extern hidden pie calls on i686 (by making
    them the same as default vis pie calls so ifuncs
    work).

(2) annotate ifuncs (avoid hidden for them, ifuncs
    cannot appear in early start code anyway because
    of IRELATIVE): this can be difficult to maintain.

(3) annotate early object accesses to be hidden so
    RELATIVE relocs are avoided. (most targets want
    all objects to be hidden, but this solves bug
    27072 without causing problems on i686)

(4) make pie always use copy relocations on i686.
    (and then no hidden annotation is needed, object
    access is always local in pie).

my patches assumed (4), but that seems to not work so
i think doing (3) is reasonable: you either need a few
carefully placed 'pragma GCC visibility push(hidden)'
or an attribute_hidden_pie_data on object declarations
that may be used by the early start code.

> From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 14 Jan 2021 18:39:24 -0800
> Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras
> 
> Add extra-test-objs to test-extras so that they are compiled with
> -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.

this makes sense.

> From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 14 Jan 2021 16:40:43 -0800
> Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden
> 
> On i386, all calls to IFUNC functions must go through PLT and calls to
> hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
> may not be set up for local calls to hidden IFUNC functions.
> 
> Even if we can't make all libc symbols hidden for static PIE on i386, we
> must make all symbols used by _dl_relocate_static_pie hidden.
> ---
>  elf/dl-reloc-static-pie.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> index a8d964061e..cc34c2d2fe 100644
> --- a/elf/dl-reloc-static-pie.c
> +++ b/elf/dl-reloc-static-pie.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #if ENABLE_STATIC_PIE
> +#pragma GCC visibility push(hidden)

yes, this is option (3). you will also need it in _dl_aux_init
and __libc_init_secure and __tunables_init.

> From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 14 Jan 2021 13:26:29 -0800
> Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE
> 
> In i386 static PIE, we need to call _dl_aux_init after relocating static
> PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in
> sync.  Also in i386 static PIE, since __libc_init_secure is called before
> ARCH_SETUP_TLS, it must use "int $0x80" for system calls.  Update
> __libc_init_secure to use __geteuid_startup, __getuid_startup,
> __getegid_startup and __getgid_startup.

the syscall part i understand, but auxv vs tls i don't:

i thought you only need to ensure that objects are hidden
visibility in _dl_aux_init.

i think the dependency order is:

1 auxv
2 libc_secure
3 tunables
4 cpu features
5 self reloc
6 setup _dl_phdr from __ehdr_start
7 setup tls

i got 6 wrong in my patch: setup tls can use _dl_phdr,
i will fix it.

moving auxv a bit later is possible (if you don't mind
syscalls in libc_secure and nothing requires it in
cpu features), but i don't see how that's related to
tls.
H.J. Lu Jan. 15, 2021, 1:43 p.m. UTC | #11
On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/14/2021 19:36, H.J. Lu wrote:
> > On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > > There are targets (i686) where hidden visibility functions are
> > > problematic in PIE code so hidden cannot be applied to all symbols.
> > > Then static PIE requires extern object access without relocations
> > > (e.g. by relying on copy relocations in shared libraries instead of
> > > GOT access in PIE code). See bug 14961.
> >
> > It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
> > additional patches to make static PIE to work on i386 and x86-64.
> > I am enclosing my patches.  Please include them in your patch set.
>
> it is about avoiding GOT for object access, which copy
> relocations can do. hidden does it better, but you said
> that does not work on i686 so i removed it (assuming
> you know the implications: every pie object access must
> be local and copy relocated in shared libraries)
>
> morally all symbols should be hidden in static pie
> because everything is local (the code is only linked
> into static exectuables). this is useful outside the
> start code too to avoid GOT indirections.
>
> i686 does not want to set up EBX for hidden extern calls,
> which is needed for ifuncs, so making everything hidden
> does not work.
>
> options:
>
> (1) fix extern hidden pie calls on i686 (by making
>     them the same as default vis pie calls so ifuncs
>     work).
>
> (2) annotate ifuncs (avoid hidden for them, ifuncs
>     cannot appear in early start code anyway because
>     of IRELATIVE): this can be difficult to maintain.
>
> (3) annotate early object accesses to be hidden so
>     RELATIVE relocs are avoided. (most targets want
>     all objects to be hidden, but this solves bug
>     27072 without causing problems on i686)
>
> (4) make pie always use copy relocations on i686.
>     (and then no hidden annotation is needed, object
>     access is always local in pie).

Linker doesn't generate copy relocations for static PIE.
The problem is GOT indirections which require RELATIVE
relocations.

> my patches assumed (4), but that seems to not work so
> i think doing (3) is reasonable: you either need a few
> carefully placed 'pragma GCC visibility push(hidden)'
> or an attribute_hidden_pie_data on object declarations
> that may be used by the early start code.
>
> > From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 14 Jan 2021 18:39:24 -0800
> > Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras
> >
> > Add extra-test-objs to test-extras so that they are compiled with
> > -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.
>
> this makes sense.
>
> > From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 14 Jan 2021 16:40:43 -0800
> > Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden
> >
> > On i386, all calls to IFUNC functions must go through PLT and calls to
> > hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
> > may not be set up for local calls to hidden IFUNC functions.
> >
> > Even if we can't make all libc symbols hidden for static PIE on i386, we
> > must make all symbols used by _dl_relocate_static_pie hidden.
> > ---
> >  elf/dl-reloc-static-pie.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> > index a8d964061e..cc34c2d2fe 100644
> > --- a/elf/dl-reloc-static-pie.c
> > +++ b/elf/dl-reloc-static-pie.c
> > @@ -17,6 +17,7 @@
> >     <https://www.gnu.org/licenses/>.  */
> >
> >  #if ENABLE_STATIC_PIE
> > +#pragma GCC visibility push(hidden)
>
> yes, this is option (3). you will also need it in _dl_aux_init
> and __libc_init_secure and __tunables_init.

I will try it.

> > From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 14 Jan 2021 13:26:29 -0800
> > Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE
> >
> > In i386 static PIE, we need to call _dl_aux_init after relocating static
> > PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in
> > sync.  Also in i386 static PIE, since __libc_init_secure is called before
> > ARCH_SETUP_TLS, it must use "int $0x80" for system calls.  Update
> > __libc_init_secure to use __geteuid_startup, __getuid_startup,
> > __getegid_startup and __getgid_startup.
>
> the syscall part i understand, but auxv vs tls i don't:
>
> i thought you only need to ensure that objects are hidden
> visibility in _dl_aux_init.
>
> i think the dependency order is:
>
> 1 auxv
> 2 libc_secure
> 3 tunables
> 4 cpu features
> 5 self reloc
> 6 setup _dl_phdr from __ehdr_start
> 7 setup tls
>
> i got 6 wrong in my patch: setup tls can use _dl_phdr,
> i will fix it.

On i386, setup tls uses auxv.  But there are GOT
indirections in _dl_aux_init () which require RELATIVE
relocations.

> moving auxv a bit later is possible (if you don't mind
> syscalls in libc_secure and nothing requires it in
> cpu features), but i don't see how that's related to
> tls.
Szabolcs Nagy Jan. 15, 2021, 2:27 p.m. UTC | #12
The 01/15/2021 05:43, H.J. Lu wrote:
> On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > options:
> >
> > (1) fix extern hidden pie calls on i686 (by making
> >     them the same as default vis pie calls so ifuncs
> >     work).
> >
> > (2) annotate ifuncs (avoid hidden for them, ifuncs
> >     cannot appear in early start code anyway because
> >     of IRELATIVE): this can be difficult to maintain.
> >
> > (3) annotate early object accesses to be hidden so
> >     RELATIVE relocs are avoided. (most targets want
> >     all objects to be hidden, but this solves bug
> >     27072 without causing problems on i686)
> >
> > (4) make pie always use copy relocations on i686.
> >     (and then no hidden annotation is needed, object
> >     access is always local in pie).
...
> > > --- a/elf/dl-reloc-static-pie.c
> > > +++ b/elf/dl-reloc-static-pie.c
> > > @@ -17,6 +17,7 @@
> > >     <https://www.gnu.org/licenses/>.  */
> > >
> > >  #if ENABLE_STATIC_PIE
> > > +#pragma GCC visibility push(hidden)
> >
> > yes, this is option (3). you will also need it in _dl_aux_init
> > and __libc_init_secure and __tunables_init.
> 
> I will try it.

the naive way does not seem to work:

_dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
the former needs hidden the latter does not and calls rawmemchr
which is ifunc on i686.

i think the easiest fix is to move those two functions into
separate files. (ideally we would have a small set of files
that are involved in the start code before self relocation)

now i realized that there is another option:

(5) remove all ifuncs from i686 libc.

i assume there are not many users who care about i686 performance.
H.J. Lu Jan. 15, 2021, 3:28 p.m. UTC | #13
On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/15/2021 05:43, H.J. Lu wrote:
> > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > options:
> > >
> > > (1) fix extern hidden pie calls on i686 (by making
> > >     them the same as default vis pie calls so ifuncs
> > >     work).
> > >
> > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > >     cannot appear in early start code anyway because
> > >     of IRELATIVE): this can be difficult to maintain.
> > >
> > > (3) annotate early object accesses to be hidden so
> > >     RELATIVE relocs are avoided. (most targets want
> > >     all objects to be hidden, but this solves bug
> > >     27072 without causing problems on i686)
> > >
> > > (4) make pie always use copy relocations on i686.
> > >     (and then no hidden annotation is needed, object
> > >     access is always local in pie).
> ...
> > > > --- a/elf/dl-reloc-static-pie.c
> > > > +++ b/elf/dl-reloc-static-pie.c
> > > > @@ -17,6 +17,7 @@
> > > >     <https://www.gnu.org/licenses/>.  */
> > > >
> > > >  #if ENABLE_STATIC_PIE
> > > > +#pragma GCC visibility push(hidden)
> > >
> > > yes, this is option (3). you will also need it in _dl_aux_init
> > > and __libc_init_secure and __tunables_init.
> >
> > I will try it.
>
> the naive way does not seem to work:
>
> _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> the former needs hidden the latter does not and calls rawmemchr
> which is ifunc on i686.
>
> i think the easiest fix is to move those two functions into
> separate files. (ideally we would have a small set of files
> that are involved in the start code before self relocation)
>
> now i realized that there is another option:
>
> (5) remove all ifuncs from i686 libc.
>
> i assume there are not many users who care about i686 performance.

I don't know if this will work on i686.  Since i386 doesn't have IP relative
addressing, we can't remove all RELATIVE relocations.  We need to
call _dl_aux_init again after relocating PIE.  I don't know what other symbols
are affected.  My current patches are on users/hjl/pr27072/master branch:

https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
H.J. Lu Jan. 15, 2021, 10:42 p.m. UTC | #14
On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/15/2021 05:43, H.J. Lu wrote:
> > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > options:
> > > >
> > > > (1) fix extern hidden pie calls on i686 (by making
> > > >     them the same as default vis pie calls so ifuncs
> > > >     work).
> > > >
> > > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > > >     cannot appear in early start code anyway because
> > > >     of IRELATIVE): this can be difficult to maintain.
> > > >
> > > > (3) annotate early object accesses to be hidden so
> > > >     RELATIVE relocs are avoided. (most targets want
> > > >     all objects to be hidden, but this solves bug
> > > >     27072 without causing problems on i686)
> > > >
> > > > (4) make pie always use copy relocations on i686.
> > > >     (and then no hidden annotation is needed, object
> > > >     access is always local in pie).
> > ...
> > > > > --- a/elf/dl-reloc-static-pie.c
> > > > > +++ b/elf/dl-reloc-static-pie.c
> > > > > @@ -17,6 +17,7 @@
> > > > >     <https://www.gnu.org/licenses/>.  */
> > > > >
> > > > >  #if ENABLE_STATIC_PIE
> > > > > +#pragma GCC visibility push(hidden)
> > > >
> > > > yes, this is option (3). you will also need it in _dl_aux_init
> > > > and __libc_init_secure and __tunables_init.
> > >
> > > I will try it.
> >
> > the naive way does not seem to work:
> >
> > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> > the former needs hidden the latter does not and calls rawmemchr
> > which is ifunc on i686.
> >
> > i think the easiest fix is to move those two functions into
> > separate files. (ideally we would have a small set of files
> > that are involved in the start code before self relocation)
> >
> > now i realized that there is another option:
> >
> > (5) remove all ifuncs from i686 libc.
> >
> > i assume there are not many users who care about i686 performance.
>
> I don't know if this will work on i686.  Since i386 doesn't have IP relative
> addressing, we can't remove all RELATIVE relocations.  We need to
> call _dl_aux_init again after relocating PIE.  I don't know what other symbols
> are affected.  My current patches are on users/hjl/pr27072/master branch:
>
> https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
>

The problem is

#ifdef NEED_DL_SYSINFO
/* Needed for improved syscall handling on at least x86/Linux.  */
uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
#endif

We can initialize it in _dl_aux_init instead.

I am testing this set of patches on top of yours on i686 and x86-64.
H.J. Lu Jan. 16, 2021, 12:41 a.m. UTC | #15
On Fri, Jan 15, 2021 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 01/15/2021 05:43, H.J. Lu wrote:
> > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > options:
> > > > >
> > > > > (1) fix extern hidden pie calls on i686 (by making
> > > > >     them the same as default vis pie calls so ifuncs
> > > > >     work).
> > > > >
> > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > > > >     cannot appear in early start code anyway because
> > > > >     of IRELATIVE): this can be difficult to maintain.
> > > > >
> > > > > (3) annotate early object accesses to be hidden so
> > > > >     RELATIVE relocs are avoided. (most targets want
> > > > >     all objects to be hidden, but this solves bug
> > > > >     27072 without causing problems on i686)
> > > > >
> > > > > (4) make pie always use copy relocations on i686.
> > > > >     (and then no hidden annotation is needed, object
> > > > >     access is always local in pie).
> > > ...
> > > > > > --- a/elf/dl-reloc-static-pie.c
> > > > > > +++ b/elf/dl-reloc-static-pie.c
> > > > > > @@ -17,6 +17,7 @@
> > > > > >     <https://www.gnu.org/licenses/>.  */
> > > > > >
> > > > > >  #if ENABLE_STATIC_PIE
> > > > > > +#pragma GCC visibility push(hidden)
> > > > >
> > > > > yes, this is option (3). you will also need it in _dl_aux_init
> > > > > and __libc_init_secure and __tunables_init.
> > > >
> > > > I will try it.
> > >
> > > the naive way does not seem to work:
> > >
> > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> > > the former needs hidden the latter does not and calls rawmemchr
> > > which is ifunc on i686.
> > >
> > > i think the easiest fix is to move those two functions into
> > > separate files. (ideally we would have a small set of files
> > > that are involved in the start code before self relocation)
> > >
> > > now i realized that there is another option:
> > >
> > > (5) remove all ifuncs from i686 libc.
> > >
> > > i assume there are not many users who care about i686 performance.
> >
> > I don't know if this will work on i686.  Since i386 doesn't have IP relative
> > addressing, we can't remove all RELATIVE relocations.  We need to
> > call _dl_aux_init again after relocating PIE.  I don't know what other symbols
> > are affected.  My current patches are on users/hjl/pr27072/master branch:
> >
> > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
> >
>
> The problem is
>
> #ifdef NEED_DL_SYSINFO
> /* Needed for improved syscall handling on at least x86/Linux.  */
> uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> #endif
>
> We can initialize it in _dl_aux_init instead.
>
> I am testing this set of patches on top of yours on i686 and x86-64.
>

They worked and they passed build-many-glibcs.py.
H.J. Lu Jan. 16, 2021, 1:18 p.m. UTC | #16
On Fri, Jan 15, 2021 at 4:41 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 15, 2021 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > >
> > > > The 01/15/2021 05:43, H.J. Lu wrote:
> > > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > options:
> > > > > >
> > > > > > (1) fix extern hidden pie calls on i686 (by making
> > > > > >     them the same as default vis pie calls so ifuncs
> > > > > >     work).
> > > > > >
> > > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > > > > >     cannot appear in early start code anyway because
> > > > > >     of IRELATIVE): this can be difficult to maintain.
> > > > > >
> > > > > > (3) annotate early object accesses to be hidden so
> > > > > >     RELATIVE relocs are avoided. (most targets want
> > > > > >     all objects to be hidden, but this solves bug
> > > > > >     27072 without causing problems on i686)
> > > > > >
> > > > > > (4) make pie always use copy relocations on i686.
> > > > > >     (and then no hidden annotation is needed, object
> > > > > >     access is always local in pie).
> > > > ...
> > > > > > > --- a/elf/dl-reloc-static-pie.c
> > > > > > > +++ b/elf/dl-reloc-static-pie.c
> > > > > > > @@ -17,6 +17,7 @@
> > > > > > >     <https://www.gnu.org/licenses/>.  */
> > > > > > >
> > > > > > >  #if ENABLE_STATIC_PIE
> > > > > > > +#pragma GCC visibility push(hidden)
> > > > > >
> > > > > > yes, this is option (3). you will also need it in _dl_aux_init
> > > > > > and __libc_init_secure and __tunables_init.
> > > > >
> > > > > I will try it.
> > > >
> > > > the naive way does not seem to work:
> > > >
> > > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> > > > the former needs hidden the latter does not and calls rawmemchr
> > > > which is ifunc on i686.
> > > >
> > > > i think the easiest fix is to move those two functions into
> > > > separate files. (ideally we would have a small set of files
> > > > that are involved in the start code before self relocation)
> > > >
> > > > now i realized that there is another option:
> > > >
> > > > (5) remove all ifuncs from i686 libc.
> > > >
> > > > i assume there are not many users who care about i686 performance.
> > >
> > > I don't know if this will work on i686.  Since i386 doesn't have IP relative
> > > addressing, we can't remove all RELATIVE relocations.  We need to
> > > call _dl_aux_init again after relocating PIE.  I don't know what other symbols
> > > are affected.  My current patches are on users/hjl/pr27072/master branch:
> > >
> > > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
> > >
> >
> > The problem is
> >
> > #ifdef NEED_DL_SYSINFO
> > /* Needed for improved syscall handling on at least x86/Linux.  */
> > uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> > #endif
> >
> > We can initialize it in _dl_aux_init instead.
> >
> > I am testing this set of patches on top of yours on i686 and x86-64.
> >
>
> They worked and they passed build-many-glibcs.py.
>

I combined my patches, including 4 testcases, with yours in the right
order here:

https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
Szabolcs Nagy Jan. 18, 2021, 4:22 p.m. UTC | #17
The 01/16/2021 05:18, H.J. Lu wrote:
> I combined my patches, including 4 testcases, with yours in the right
> order here:
> 
> https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master

thanks.

i moved things around a bit: if we have fine grain marking in
files that need hidden then we can just use that instead of
making everything hidden, this needed hidden in a few more
files.

i also moved the _dl_sysinfo bit into a separate patch.
i'm sending a v4 of the series.
diff mbox series

Patch

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index ea126ae70c..93e63ee889 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,13 +434,17 @@  for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED \
-  || (BUILD_PIE_DEFAULT && IS_IN (libc))
+#if defined SHARED || defined LIBC_NONSHARED
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
 #endif
 
+/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
+#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
+# pragma GCC visibility push(hidden)
+#endif
+
 #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
 
 #define attribute_relro __attribute__ ((section (".data.rel.ro")))