public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] aarch64: fix string tests [BZ #26818]
@ 2021-01-25 11:03 Szabolcs Nagy
  2021-01-25 11:03 ` [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED Szabolcs Nagy
  2021-01-25 11:03 ` [PATCH 2/2] aarch64: Fix the list of tested IFUNC variants [BZ #26818] Szabolcs Nagy
  0 siblings, 2 replies; 8+ messages in thread
From: Szabolcs Nagy @ 2021-01-25 11:03 UTC (permalink / raw)
  To: libc-alpha

I'd like to commit these for 2.33. First one is a clean up,
the second on should only affect the tested ifunc variants
during make check.

Szabolcs Nagy (2):
  aarch64: Move and update the definition of MTE_ENABLED
  aarch64: Fix the list of tested IFUNC variants [BZ #26818]

 sysdeps/aarch64/multiarch/ifunc-impl-list.c | 16 +++++++++-------
 sysdeps/aarch64/multiarch/init-arch.h       | 13 ++++++++++++-
 sysdeps/aarch64/multiarch/strlen.c          | 11 +----------
 3 files changed, 22 insertions(+), 18 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED
  2021-01-25 11:03 [PATCH 0/2] aarch64: fix string tests [BZ #26818] Szabolcs Nagy
@ 2021-01-25 11:03 ` Szabolcs Nagy
  2021-01-25 13:08   ` Adhemerval Zanella
  2021-01-25 11:03 ` [PATCH 2/2] aarch64: Fix the list of tested IFUNC variants [BZ #26818] Szabolcs Nagy
  1 sibling, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2021-01-25 11:03 UTC (permalink / raw)
  To: libc-alpha

The hwcap value is now in linux 5.10 and in glibc bits/hwcap.h, so use
that definition.

Move the definition to init-arch.h so all ifunc selectors can use it
and expose an "mte" shorthand for mte enabled runtime.

For now we allow user code to enable tag checks and use PROT_MTE
mappings without libc involvment, this is not guaranteed ABI, but
can be useful for testing and debugging with MTE.
---
 sysdeps/aarch64/multiarch/init-arch.h | 11 ++++++++++-
 sysdeps/aarch64/multiarch/strlen.c    | 11 +----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
index bf8264b561..fce260d168 100644
--- a/sysdeps/aarch64/multiarch/init-arch.h
+++ b/sysdeps/aarch64/multiarch/init-arch.h
@@ -17,9 +17,18 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <ldsodefs.h>
+#include <sys/auxv.h>
+
+/* Make glibc MTE-safe on a system that supports MTE in case user code
+   enables tag checks independently of the mte_status of glibc.  There
+   is currently no ABI contract for enabling tag checks in user code,
+   but this can be useful for debugging with MTE.  */
+#define MTE_ENABLED() (GLRO(dl_hwcap2) & HWCAP2_MTE)
 
 #define INIT_ARCH()							      \
   uint64_t __attribute__((unused)) midr =				      \
     GLRO(dl_aarch64_cpu_features).midr_el1;				      \
   unsigned __attribute__((unused)) zva_size =				      \
-    GLRO(dl_aarch64_cpu_features).zva_size;
+    GLRO(dl_aarch64_cpu_features).zva_size;				      \
+  bool __attribute__((unused)) mte =					      \
+    MTE_ENABLED ();
diff --git a/sysdeps/aarch64/multiarch/strlen.c b/sysdeps/aarch64/multiarch/strlen.c
index f3c018aab4..8f38de69b5 100644
--- a/sysdeps/aarch64/multiarch/strlen.c
+++ b/sysdeps/aarch64/multiarch/strlen.c
@@ -26,21 +26,12 @@
 # include <string.h>
 # include <init-arch.h>
 
-/* This should check HWCAP2_MTE when it is available: current
-   linux kernel does not expose it, but its value is reserved.
-   This is needed to make glibc MTE-safe on future systems in
-   case user code enables MTE. The ABI contract for enabling
-   MTE is not yet specified, but it can be useful for at least
-   debugging which does not need a contract.  */
-#define FUTURE_HWCAP2_MTE (1 << 18)
-#define MTE_ENABLED() (GLRO(dl_hwcap2) & FUTURE_HWCAP2_MTE)
-
 extern __typeof (__redirect_strlen) __strlen;
 
 extern __typeof (__redirect_strlen) __strlen_mte attribute_hidden;
 extern __typeof (__redirect_strlen) __strlen_asimd attribute_hidden;
 
-libc_ifunc (__strlen, (MTE_ENABLED () ? __strlen_mte : __strlen_asimd));
+libc_ifunc (__strlen, (mte ? __strlen_mte : __strlen_asimd));
 
 # undef strlen
 strong_alias (__strlen, strlen);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] aarch64: Fix the list of tested IFUNC variants [BZ #26818]
  2021-01-25 11:03 [PATCH 0/2] aarch64: fix string tests [BZ #26818] Szabolcs Nagy
  2021-01-25 11:03 ` [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED Szabolcs Nagy
@ 2021-01-25 11:03 ` Szabolcs Nagy
  2021-01-25 12:46   ` [PATCH v2] " Szabolcs Nagy
  1 sibling, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2021-01-25 11:03 UTC (permalink / raw)
  To: libc-alpha

Some IFUNC variants are not compatible with BTI and MTE so don't
set them as usable for testing and benchmarking on a BTI or MTE
enabled system.

As far as IFUNC selectors are concerned a system is BTI enabled if
the cpu supports it and glibc was built with BTI branch protection.

Most IFUNC variants are BTI compatible, but thunderx2 memcpy and
memmove use a jump table with indirect jump, without a BTI j.

Fixes bug 26818.
---
 sysdeps/aarch64/multiarch/ifunc-impl-list.c | 16 +++++++++-------
 sysdeps/aarch64/multiarch/init-arch.h       |  2 ++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/sysdeps/aarch64/multiarch/ifunc-impl-list.c b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
index a69eae65f0..ce9e5e35bf 100644
--- a/sysdeps/aarch64/multiarch/ifunc-impl-list.c
+++ b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
@@ -40,29 +40,31 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
   /* Support sysdeps/aarch64/multiarch/memcpy.c and memmove.c.  */
   IFUNC_IMPL (i, name, memcpy,
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_thunderx)
-	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_thunderx2)
+	      IFUNC_IMPL_ADD (array, i, memcpy, !bti, __memcpy_thunderx2)
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_falkor)
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_simd)
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_generic))
   IFUNC_IMPL (i, name, memmove,
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx)
-	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx2)
+	      IFUNC_IMPL_ADD (array, i, memmove, !bti, __memmove_thunderx2)
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_falkor)
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_simd)
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_generic))
   IFUNC_IMPL (i, name, memset,
 	      /* Enable this on non-falkor processors too so that other cores
 		 can do a comparative analysis with __memset_generic.  */
-	      IFUNC_IMPL_ADD (array, i, memset, (zva_size == 64), __memset_falkor)
-	      IFUNC_IMPL_ADD (array, i, memset, (zva_size == 64), __memset_emag)
-	      IFUNC_IMPL_ADD (array, i, memset, 1, __memset_kunpeng)
+	      IFUNC_IMPL_ADD (array, i, memset, (zva_size == 64) && !mte,
+			      __memset_falkor)
+	      IFUNC_IMPL_ADD (array, i, memset, (zva_size == 64) && !mte,
+			      __memset_emag)
+	      IFUNC_IMPL_ADD (array, i, memset, !mte, __memset_kunpeng)
 	      IFUNC_IMPL_ADD (array, i, memset, 1, __memset_generic))
   IFUNC_IMPL (i, name, memchr,
-	      IFUNC_IMPL_ADD (array, i, memchr, 1, __memchr_nosimd)
+	      IFUNC_IMPL_ADD (array, i, memchr, !mte, __memchr_nosimd)
 	      IFUNC_IMPL_ADD (array, i, memchr, 1, __memchr_generic))
 
   IFUNC_IMPL (i, name, strlen,
-	      IFUNC_IMPL_ADD (array, i, strlen, 1, __strlen_asimd)
+	      IFUNC_IMPL_ADD (array, i, strlen, !mte, __strlen_asimd)
 	      IFUNC_IMPL_ADD (array, i, strlen, 1, __strlen_mte))
 
   return i;
diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
index fce260d168..a167699e74 100644
--- a/sysdeps/aarch64/multiarch/init-arch.h
+++ b/sysdeps/aarch64/multiarch/init-arch.h
@@ -30,5 +30,7 @@
     GLRO(dl_aarch64_cpu_features).midr_el1;				      \
   unsigned __attribute__((unused)) zva_size =				      \
     GLRO(dl_aarch64_cpu_features).zva_size;				      \
+  bool __attribute__((unused)) bti =					      \
+    HAVE_AARCH64_BTI && GLRO(dl_aarch64_cpu_features).bti;		      \
   bool __attribute__((unused)) mte =					      \
     MTE_ENABLED ();
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] aarch64: Fix the list of tested IFUNC variants [BZ #26818]
  2021-01-25 11:03 ` [PATCH 2/2] aarch64: Fix the list of tested IFUNC variants [BZ #26818] Szabolcs Nagy
@ 2021-01-25 12:46   ` Szabolcs Nagy
  2021-01-25 14:59     ` Adhemerval Zanella
  0 siblings, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2021-01-25 12:46 UTC (permalink / raw)
  To: libc-alpha

Some IFUNC variants are not compatible with BTI and MTE so don't
set them as usable for testing and benchmarking on a BTI or MTE
enabled system.

As far as IFUNC selectors are concerned a system is BTI enabled if
the cpu supports it and glibc was built with BTI branch protection.

Most IFUNC variants are BTI compatible, but thunderx2 memcpy and
memmove use a jump table with indirect jump, without a BTI j.

Fixes bug 26818.
---

v2:
- memset variants are mte compatible, so no reason to disable them
  for an mte enabled system.

 sysdeps/aarch64/multiarch/ifunc-impl-list.c | 8 ++++----
 sysdeps/aarch64/multiarch/init-arch.h       | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/sysdeps/aarch64/multiarch/ifunc-impl-list.c b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
index a69eae65f0..99a8c68aac 100644
--- a/sysdeps/aarch64/multiarch/ifunc-impl-list.c
+++ b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
@@ -40,13 +40,13 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
   /* Support sysdeps/aarch64/multiarch/memcpy.c and memmove.c.  */
   IFUNC_IMPL (i, name, memcpy,
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_thunderx)
-	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_thunderx2)
+	      IFUNC_IMPL_ADD (array, i, memcpy, !bti, __memcpy_thunderx2)
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_falkor)
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_simd)
 	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_generic))
   IFUNC_IMPL (i, name, memmove,
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx)
-	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx2)
+	      IFUNC_IMPL_ADD (array, i, memmove, !bti, __memmove_thunderx2)
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_falkor)
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_simd)
 	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_generic))
@@ -58,11 +58,11 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
 	      IFUNC_IMPL_ADD (array, i, memset, 1, __memset_kunpeng)
 	      IFUNC_IMPL_ADD (array, i, memset, 1, __memset_generic))
   IFUNC_IMPL (i, name, memchr,
-	      IFUNC_IMPL_ADD (array, i, memchr, 1, __memchr_nosimd)
+	      IFUNC_IMPL_ADD (array, i, memchr, !mte, __memchr_nosimd)
 	      IFUNC_IMPL_ADD (array, i, memchr, 1, __memchr_generic))
 
   IFUNC_IMPL (i, name, strlen,
-	      IFUNC_IMPL_ADD (array, i, strlen, 1, __strlen_asimd)
+	      IFUNC_IMPL_ADD (array, i, strlen, !mte, __strlen_asimd)
 	      IFUNC_IMPL_ADD (array, i, strlen, 1, __strlen_mte))
 
   return i;
diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
index fce260d168..a167699e74 100644
--- a/sysdeps/aarch64/multiarch/init-arch.h
+++ b/sysdeps/aarch64/multiarch/init-arch.h
@@ -30,5 +30,7 @@
     GLRO(dl_aarch64_cpu_features).midr_el1;				      \
   unsigned __attribute__((unused)) zva_size =				      \
     GLRO(dl_aarch64_cpu_features).zva_size;				      \
+  bool __attribute__((unused)) bti =					      \
+    HAVE_AARCH64_BTI && GLRO(dl_aarch64_cpu_features).bti;		      \
   bool __attribute__((unused)) mte =					      \
     MTE_ENABLED ();
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED
  2021-01-25 11:03 ` [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED Szabolcs Nagy
@ 2021-01-25 13:08   ` Adhemerval Zanella
  2021-01-25 13:42     ` Szabolcs Nagy
  0 siblings, 1 reply; 8+ messages in thread
From: Adhemerval Zanella @ 2021-01-25 13:08 UTC (permalink / raw)
  To: libc-alpha, Szabolcs Nagy



On 25/01/2021 08:03, Szabolcs Nagy via Libc-alpha wrote:
> The hwcap value is now in linux 5.10 and in glibc bits/hwcap.h, so use
> that definition.
> 
> Move the definition to init-arch.h so all ifunc selectors can use it
> and expose an "mte" shorthand for mte enabled runtime.
> 
> For now we allow user code to enable tag checks and use PROT_MTE
> mappings without libc involvment, this is not guaranteed ABI, but
> can be useful for testing and debugging with MTE.
> ---
>  sysdeps/aarch64/multiarch/init-arch.h | 11 ++++++++++-
>  sysdeps/aarch64/multiarch/strlen.c    | 11 +----------
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
> index bf8264b561..fce260d168 100644
> --- a/sysdeps/aarch64/multiarch/init-arch.h
> +++ b/sysdeps/aarch64/multiarch/init-arch.h
> @@ -17,9 +17,18 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <ldsodefs.h>
> +#include <sys/auxv.h>
> +
> +/* Make glibc MTE-safe on a system that supports MTE in case user code
> +   enables tag checks independently of the mte_status of glibc.  There
> +   is currently no ABI contract for enabling tag checks in user code,
> +   but this can be useful for debugging with MTE.  */
> +#define MTE_ENABLED() (GLRO(dl_hwcap2) & HWCAP2_MTE)
>  
>  #define INIT_ARCH()							      \
>    uint64_t __attribute__((unused)) midr =				      \
>      GLRO(dl_aarch64_cpu_features).midr_el1;				      \
>    unsigned __attribute__((unused)) zva_size =				      \
> -    GLRO(dl_aarch64_cpu_features).zva_size;
> +    GLRO(dl_aarch64_cpu_features).zva_size;				      \
> +  bool __attribute__((unused)) mte =					      \
> +    MTE_ENABLED ();

Why not use mte_state and thus also enable MTE selection for the case
of tunables force enable it for USE_MTAG support?

> diff --git a/sysdeps/aarch64/multiarch/strlen.c b/sysdeps/aarch64/multiarch/strlen.c
> index f3c018aab4..8f38de69b5 100644
> --- a/sysdeps/aarch64/multiarch/strlen.c
> +++ b/sysdeps/aarch64/multiarch/strlen.c
> @@ -26,21 +26,12 @@
>  # include <string.h>
>  # include <init-arch.h>
>  
> -/* This should check HWCAP2_MTE when it is available: current
> -   linux kernel does not expose it, but its value is reserved.
> -   This is needed to make glibc MTE-safe on future systems in
> -   case user code enables MTE. The ABI contract for enabling
> -   MTE is not yet specified, but it can be useful for at least
> -   debugging which does not need a contract.  */
> -#define FUTURE_HWCAP2_MTE (1 << 18)
> -#define MTE_ENABLED() (GLRO(dl_hwcap2) & FUTURE_HWCAP2_MTE)
> -
>  extern __typeof (__redirect_strlen) __strlen;
>  
>  extern __typeof (__redirect_strlen) __strlen_mte attribute_hidden;
>  extern __typeof (__redirect_strlen) __strlen_asimd attribute_hidden;
>  
> -libc_ifunc (__strlen, (MTE_ENABLED () ? __strlen_mte : __strlen_asimd));
> +libc_ifunc (__strlen, (mte ? __strlen_mte : __strlen_asimd));
>  
>  # undef strlen
>  strong_alias (__strlen, strlen);
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED
  2021-01-25 13:08   ` Adhemerval Zanella
@ 2021-01-25 13:42     ` Szabolcs Nagy
  2021-01-25 14:58       ` Adhemerval Zanella
  0 siblings, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2021-01-25 13:42 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

The 01/25/2021 10:08, Adhemerval Zanella wrote:
> On 25/01/2021 08:03, Szabolcs Nagy via Libc-alpha wrote:
> > The hwcap value is now in linux 5.10 and in glibc bits/hwcap.h, so use
> > that definition.
> > 
> > Move the definition to init-arch.h so all ifunc selectors can use it
> > and expose an "mte" shorthand for mte enabled runtime.
> > 
> > For now we allow user code to enable tag checks and use PROT_MTE
> > mappings without libc involvment, this is not guaranteed ABI, but
> > can be useful for testing and debugging with MTE.
> > ---
> >  sysdeps/aarch64/multiarch/init-arch.h | 11 ++++++++++-
> >  sysdeps/aarch64/multiarch/strlen.c    | 11 +----------
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
> > index bf8264b561..fce260d168 100644
> > --- a/sysdeps/aarch64/multiarch/init-arch.h
> > +++ b/sysdeps/aarch64/multiarch/init-arch.h
> > @@ -17,9 +17,18 @@
> >     <https://www.gnu.org/licenses/>.  */
> >  
> >  #include <ldsodefs.h>
> > +#include <sys/auxv.h>
> > +
> > +/* Make glibc MTE-safe on a system that supports MTE in case user code
> > +   enables tag checks independently of the mte_status of glibc.  There
> > +   is currently no ABI contract for enabling tag checks in user code,
> > +   but this can be useful for debugging with MTE.  */
> > +#define MTE_ENABLED() (GLRO(dl_hwcap2) & HWCAP2_MTE)
> >  
> >  #define INIT_ARCH()							      \
> >    uint64_t __attribute__((unused)) midr =				      \
> >      GLRO(dl_aarch64_cpu_features).midr_el1;				      \
> >    unsigned __attribute__((unused)) zva_size =				      \
> > -    GLRO(dl_aarch64_cpu_features).zva_size;
> > +    GLRO(dl_aarch64_cpu_features).zva_size;				      \
> > +  bool __attribute__((unused)) mte =					      \
> > +    MTE_ENABLED ();
> 
> Why not use mte_state and thus also enable MTE selection for the case
> of tunables force enable it for USE_MTAG support?

that's what the comment is meant to explain.

mte is enabled via a prctl, in principle user code can do that
(e.g. i have test code that does that as well as ld_preloaded
malloc implementation, this only works if glibc is mte safe
independently of whether glibc has heap tagging or not. no ABI
contract means that we don't guarantee that glibc will be
mte safe because we don't know yet how this will be used, the
plan is to have some form of opt-in/out mechanism eventually
to request particular mte state, but for now i want glibc to
be mte safe on any mte system)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED
  2021-01-25 13:42     ` Szabolcs Nagy
@ 2021-01-25 14:58       ` Adhemerval Zanella
  0 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2021-01-25 14:58 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: libc-alpha



On 25/01/2021 10:42, Szabolcs Nagy wrote:
> The 01/25/2021 10:08, Adhemerval Zanella wrote:
>> On 25/01/2021 08:03, Szabolcs Nagy via Libc-alpha wrote:
>>> The hwcap value is now in linux 5.10 and in glibc bits/hwcap.h, so use
>>> that definition.
>>>
>>> Move the definition to init-arch.h so all ifunc selectors can use it
>>> and expose an "mte" shorthand for mte enabled runtime.
>>>
>>> For now we allow user code to enable tag checks and use PROT_MTE
>>> mappings without libc involvment, this is not guaranteed ABI, but
>>> can be useful for testing and debugging with MTE.
>>> ---
>>>  sysdeps/aarch64/multiarch/init-arch.h | 11 ++++++++++-
>>>  sysdeps/aarch64/multiarch/strlen.c    | 11 +----------
>>>  2 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
>>> index bf8264b561..fce260d168 100644
>>> --- a/sysdeps/aarch64/multiarch/init-arch.h
>>> +++ b/sysdeps/aarch64/multiarch/init-arch.h
>>> @@ -17,9 +17,18 @@
>>>     <https://www.gnu.org/licenses/>.  */
>>>  
>>>  #include <ldsodefs.h>
>>> +#include <sys/auxv.h>
>>> +
>>> +/* Make glibc MTE-safe on a system that supports MTE in case user code
>>> +   enables tag checks independently of the mte_status of glibc.  There
>>> +   is currently no ABI contract for enabling tag checks in user code,
>>> +   but this can be useful for debugging with MTE.  */
>>> +#define MTE_ENABLED() (GLRO(dl_hwcap2) & HWCAP2_MTE)
>>>  
>>>  #define INIT_ARCH()							      \
>>>    uint64_t __attribute__((unused)) midr =				      \
>>>      GLRO(dl_aarch64_cpu_features).midr_el1;				      \
>>>    unsigned __attribute__((unused)) zva_size =				      \
>>> -    GLRO(dl_aarch64_cpu_features).zva_size;
>>> +    GLRO(dl_aarch64_cpu_features).zva_size;				      \
>>> +  bool __attribute__((unused)) mte =					      \
>>> +    MTE_ENABLED ();
>>
>> Why not use mte_state and thus also enable MTE selection for the case
>> of tunables force enable it for USE_MTAG support?
> 
> that's what the comment is meant to explain.
> 
> mte is enabled via a prctl, in principle user code can do that
> (e.g. i have test code that does that as well as ld_preloaded
> malloc implementation, this only works if glibc is mte safe
> independently of whether glibc has heap tagging or not. no ABI
> contract means that we don't guarantee that glibc will be
> mte safe because we don't know yet how this will be used, the
> plan is to have some form of opt-in/out mechanism eventually
> to request particular mte state, but for now i want glibc to
> be mte safe on any mte system)
> 

Ok, fair enough.  The patch looks ok for 2.33.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] aarch64: Fix the list of tested IFUNC variants [BZ #26818]
  2021-01-25 12:46   ` [PATCH v2] " Szabolcs Nagy
@ 2021-01-25 14:59     ` Adhemerval Zanella
  0 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2021-01-25 14:59 UTC (permalink / raw)
  To: libc-alpha



On 25/01/2021 09:46, Szabolcs Nagy via Libc-alpha wrote:
> Some IFUNC variants are not compatible with BTI and MTE so don't
> set them as usable for testing and benchmarking on a BTI or MTE
> enabled system.
> 
> As far as IFUNC selectors are concerned a system is BTI enabled if
> the cpu supports it and glibc was built with BTI branch protection.
> 
> Most IFUNC variants are BTI compatible, but thunderx2 memcpy and
> memmove use a jump table with indirect jump, without a BTI j.
> 
> Fixes bug 26818.

LGTM to 2.33, thanks.

> ---
> 
> v2:
> - memset variants are mte compatible, so no reason to disable them
>   for an mte enabled system.
> 
>  sysdeps/aarch64/multiarch/ifunc-impl-list.c | 8 ++++----
>  sysdeps/aarch64/multiarch/init-arch.h       | 2 ++
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/aarch64/multiarch/ifunc-impl-list.c b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> index a69eae65f0..99a8c68aac 100644
> --- a/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> @@ -40,13 +40,13 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>    /* Support sysdeps/aarch64/multiarch/memcpy.c and memmove.c.  */
>    IFUNC_IMPL (i, name, memcpy,
>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_thunderx)
> -	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_thunderx2)
> +	      IFUNC_IMPL_ADD (array, i, memcpy, !bti, __memcpy_thunderx2)
>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_falkor)
>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_simd)
>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_generic))
>    IFUNC_IMPL (i, name, memmove,
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx)
> -	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx2)
> +	      IFUNC_IMPL_ADD (array, i, memmove, !bti, __memmove_thunderx2)
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_falkor)
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_simd)
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_generic))
> @@ -58,11 +58,11 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>  	      IFUNC_IMPL_ADD (array, i, memset, 1, __memset_kunpeng)
>  	      IFUNC_IMPL_ADD (array, i, memset, 1, __memset_generic))
>    IFUNC_IMPL (i, name, memchr,
> -	      IFUNC_IMPL_ADD (array, i, memchr, 1, __memchr_nosimd)
> +	      IFUNC_IMPL_ADD (array, i, memchr, !mte, __memchr_nosimd)
>  	      IFUNC_IMPL_ADD (array, i, memchr, 1, __memchr_generic))
>  
>    IFUNC_IMPL (i, name, strlen,
> -	      IFUNC_IMPL_ADD (array, i, strlen, 1, __strlen_asimd)
> +	      IFUNC_IMPL_ADD (array, i, strlen, !mte, __strlen_asimd)
>  	      IFUNC_IMPL_ADD (array, i, strlen, 1, __strlen_mte))
>  
>    return i;
> diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
> index fce260d168..a167699e74 100644
> --- a/sysdeps/aarch64/multiarch/init-arch.h
> +++ b/sysdeps/aarch64/multiarch/init-arch.h
> @@ -30,5 +30,7 @@
>      GLRO(dl_aarch64_cpu_features).midr_el1;				      \
>    unsigned __attribute__((unused)) zva_size =				      \
>      GLRO(dl_aarch64_cpu_features).zva_size;				      \
> +  bool __attribute__((unused)) bti =					      \
> +    HAVE_AARCH64_BTI && GLRO(dl_aarch64_cpu_features).bti;		      \
>    bool __attribute__((unused)) mte =					      \
>      MTE_ENABLED ();
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-01-25 14:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 11:03 [PATCH 0/2] aarch64: fix string tests [BZ #26818] Szabolcs Nagy
2021-01-25 11:03 ` [PATCH 1/2] aarch64: Move and update the definition of MTE_ENABLED Szabolcs Nagy
2021-01-25 13:08   ` Adhemerval Zanella
2021-01-25 13:42     ` Szabolcs Nagy
2021-01-25 14:58       ` Adhemerval Zanella
2021-01-25 11:03 ` [PATCH 2/2] aarch64: Fix the list of tested IFUNC variants [BZ #26818] Szabolcs Nagy
2021-01-25 12:46   ` [PATCH v2] " Szabolcs Nagy
2021-01-25 14:59     ` Adhemerval Zanella

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 read-only IMAP folder(s) and NNTP newsgroup(s).