* [PATCH] Properly check stack alignment [BZ #27901]
@ 2021-05-23 17:47 H.J. Lu
2021-05-24 10:56 ` Florian Weimer
0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2021-05-23 17:47 UTC (permalink / raw)
To: libc-alpha
1. Replace
if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0)
which may be optimized by compiler, with
static int
__attribute__ ((noclone, noinline))
aligned (void *p, int align)
{
return (((uintptr_t) p) & (align - 1)) != 0;
}
2. Add TEST_STACK_ALIGN_INIT to TEST_STACK_ALIGN.
3. Add a common TEST_STACK_ALIGN_INIT to check 16-byte stack alignment
for both i386 and x86-64.
---
sysdeps/generic/tst-stack-align.h | 40 ++++++++++++++++---------
sysdeps/i386/i686/tst-stack-align.h | 44 ---------------------------
sysdeps/i386/tst-stack-align.h | 41 -------------------------
sysdeps/powerpc/tst-stack-align.h | 27 +++++------------
sysdeps/x86/tst-stack-align.h | 28 ++++++++++++++++++
sysdeps/x86_64/tst-stack-align.h | 46 -----------------------------
6 files changed, 61 insertions(+), 165 deletions(-)
delete mode 100644 sysdeps/i386/i686/tst-stack-align.h
delete mode 100644 sysdeps/i386/tst-stack-align.h
create mode 100644 sysdeps/x86/tst-stack-align.h
delete mode 100644 sysdeps/x86_64/tst-stack-align.h
diff --git a/sysdeps/generic/tst-stack-align.h b/sysdeps/generic/tst-stack-align.h
index b41f9d3c12..fb48531b45 100644
--- a/sysdeps/generic/tst-stack-align.h
+++ b/sysdeps/generic/tst-stack-align.h
@@ -1,4 +1,5 @@
-/* Copyright (C) 2003-2021 Free Software Foundation, Inc.
+/* Check stack alignment. Generic version.
+ Copyright (C) 2003-2021 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -18,17 +19,28 @@
#include <stdio.h>
#include <stdint.h>
+static int
+__attribute__ ((noclone, noinline))
+aligned (void *p, int align)
+{
+ return (((uintptr_t) p) & (align - 1)) != 0;
+}
+
+#ifndef TEST_STACK_ALIGN_INIT
+# define TEST_STACK_ALIGN_INIT() 0
+#endif
+
#define TEST_STACK_ALIGN() \
- ({ \
- double _d = 12.0; \
- long double _ld = 15.0; \
- int _ret = 0; \
- printf ("double: %g %p %zu\n", _d, &_d, __alignof (double)); \
- if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("ldouble: %Lg %p %zu\n", _ld, &_ld, __alignof (long double)); \
- if ((((uintptr_t) &_ld) & (__alignof (long double) - 1)) != 0) \
- _ret = 1; \
- _ret; \
- })
+ ({ \
+ double _d = 12.0; \
+ long double _ld = 15.0; \
+ int _ret = TEST_STACK_ALIGN_INIT (); \
+ \
+ printf ("double: %g %p %zu\n", _d, &_d, __alignof (double)); \
+ _ret += aligned (&_d, __alignof (double)); \
+ \
+ printf ("ldouble: %Lg %p %zu\n", _ld, &_ld, \
+ __alignof (long double)); \
+ _ret += aligned (&_ld, __alignof (long double)); \
+ _ret; \
+ })
diff --git a/sysdeps/i386/i686/tst-stack-align.h b/sysdeps/i386/i686/tst-stack-align.h
deleted file mode 100644
index fc3852ea5d..0000000000
--- a/sysdeps/i386/i686/tst-stack-align.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/* Copyright (C) 2003-2021 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-#include <stdio.h>
-#include <stdint.h>
-#ifndef __SSE__
-#include_next <tst-stack-align.h>
-#else
-#include <xmmintrin.h>
-
-#define TEST_STACK_ALIGN() \
- ({ \
- __m128 _m; \
- double _d = 12.0; \
- long double _ld = 15.0; \
- int _ret = 0; \
- printf ("__m128: %p %zu\n", &_m, __alignof (__m128)); \
- if ((((uintptr_t) &_m) & (__alignof (__m128) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("double: %g %p %zu\n", _d, &_d, __alignof (double)); \
- if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("ldouble: %Lg %p %zu\n", _ld, &_ld, __alignof (long double)); \
- if ((((uintptr_t) &_ld) & (__alignof (long double) - 1)) != 0) \
- _ret = 1; \
- _ret; \
- })
-#endif
diff --git a/sysdeps/i386/tst-stack-align.h b/sysdeps/i386/tst-stack-align.h
deleted file mode 100644
index 7891e9a03e..0000000000
--- a/sysdeps/i386/tst-stack-align.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/* Copyright (C) 2004-2021 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-#include <stdio.h>
-#include <stdint.h>
-
-typedef struct { int i[4]; } int_al16 __attribute__((aligned (16)));
-
-#define TEST_STACK_ALIGN() \
- ({ \
- int_al16 _m; \
- double _d = 12.0; \
- long double _ld = 15.0; \
- int _ret = 0; \
- printf ("int_al16: %p %zu\n", &_m, __alignof (int_al16)); \
- if ((((uintptr_t) &_m) & (__alignof (int_al16) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("double: %g %p %zu\n", _d, &_d, __alignof (double)); \
- if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("ldouble: %Lg %p %zu\n", _ld, &_ld, __alignof (long double)); \
- if ((((uintptr_t) &_ld) & (__alignof (long double) - 1)) != 0) \
- _ret = 1; \
- _ret; \
- })
diff --git a/sysdeps/powerpc/tst-stack-align.h b/sysdeps/powerpc/tst-stack-align.h
index 2b4a3671b4..5a66fc6b33 100644
--- a/sysdeps/powerpc/tst-stack-align.h
+++ b/sysdeps/powerpc/tst-stack-align.h
@@ -1,4 +1,5 @@
-/* Copyright (C) 2005-2021 Free Software Foundation, Inc.
+/* Check stack alignment. PowerPC version.
+ Copyright (C) 2005-2021 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -15,10 +16,7 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <stdio.h>
-#include <stdint.h>
-
-#define TEST_STACK_ALIGN() \
+#define TEST_STACK_ALIGN_INIT() \
({ \
/* Altivec __vector int etc. needs 16byte aligned stack. \
Instead of using altivec.h here, use aligned attribute instead. */ \
@@ -27,20 +25,9 @@
int _i __attribute__((aligned (16))); \
int _j[3]; \
} _s = { ._i = 18, ._j[0] = 19, ._j[1] = 20, ._j[2] = 21 }; \
- double _d = 12.0; \
- long double _ld = 15.0; \
- int _ret = 0; \
printf ("__vector int: { %d, %d, %d, %d } %p %zu\n", _s._i, _s._j[0], \
_s._j[1], _s._j[2], &_s, __alignof (_s)); \
- if ((((uintptr_t) &_s) & (__alignof (_s) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("double: %g %p %zu\n", _d, &_d, __alignof (double)); \
- if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("ldouble: %Lg %p %zu\n", _ld, &_ld, __alignof (long double)); \
- if ((((uintptr_t) &_ld) & (__alignof (long double) - 1)) != 0) \
- _ret = 1; \
- _ret; \
- })
+ aligned (&_s, __alignof (_s)); \
+ })
+
+#include_next <tst-stack-align.h>
diff --git a/sysdeps/x86/tst-stack-align.h b/sysdeps/x86/tst-stack-align.h
new file mode 100644
index 0000000000..28b9e57089
--- /dev/null
+++ b/sysdeps/x86/tst-stack-align.h
@@ -0,0 +1,28 @@
+/* Check stack alignment. X86 version.
+ Copyright (C) 2021 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+typedef struct { int i[16]; } int_al16 __attribute__((aligned (16)));
+
+#define TEST_STACK_ALIGN_INIT() \
+ ({ \
+ int_al16 _m; \
+ printf ("int_al16: %p %zu\n", &_m, __alignof (int_al16)); \
+ aligned (&_m, __alignof (int_al16)); \
+ })
+
+#include_next <tst-stack-align.h>
diff --git a/sysdeps/x86_64/tst-stack-align.h b/sysdeps/x86_64/tst-stack-align.h
deleted file mode 100644
index b3f93f7cab..0000000000
--- a/sysdeps/x86_64/tst-stack-align.h
+++ /dev/null
@@ -1,46 +0,0 @@
-/* Copyright (C) 2003-2021 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-#include <stdio.h>
-#include <stdint.h>
-
-#define TEST_STACK_ALIGN() \
- ({ \
- /* AMD64 ABI mandates 16byte aligned stack. \
- Unfortunately, current GCC doesn't support __int128 or __float128 \
- types, so use aligned attribute instead. */ \
- struct _S \
- { \
- int _i __attribute__((aligned (16))); \
- int _pad[3]; \
- } _s = { ._i = 18 }; \
- double _d = 12.0; \
- long double _ld = 15.0; \
- int _ret = 0; \
- printf ("__int128: %d %p %zu\n", _s._i, &_s, __alignof (_s)); \
- if ((((uintptr_t) &_s) & (__alignof (_s) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("double: %g %p %zu\n", _d, &_d, __alignof (double)); \
- if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0) \
- _ret = 1; \
- \
- printf ("ldouble: %Lg %p %zu\n", _ld, &_ld, __alignof (long double)); \
- if ((((uintptr_t) &_ld) & (__alignof (long double) - 1)) != 0) \
- _ret = 1; \
- _ret; \
- })
--
2.31.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Properly check stack alignment [BZ #27901]
2021-05-23 17:47 [PATCH] Properly check stack alignment [BZ #27901] H.J. Lu
@ 2021-05-24 10:56 ` Florian Weimer
2021-05-24 11:27 ` Alexander Monakov
2021-05-24 13:08 ` H.J. Lu
0 siblings, 2 replies; 5+ messages in thread
From: Florian Weimer @ 2021-05-24 10:56 UTC (permalink / raw)
To: H.J. Lu via Libc-alpha
* H. J. Lu via Libc-alpha:
> 1. Replace
>
> if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0)
>
> which may be optimized by compiler, with
>
> static int
> __attribute__ ((noclone, noinline))
> aligned (void *p, int align)
> {
> return (((uintptr_t) p) & (align - 1)) != 0;
> }
I think you need to use __attribute__ ((weak)) (and drop the static) for
a fully effective compiler barrier. Due to the new global nature of the
symbol, you probably should switch to a different identifier less prone
to collisions.
GCC 8 also supports the noipa attribute, but that's too recent for us.
Thanks,
Florian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Properly check stack alignment [BZ #27901]
2021-05-24 10:56 ` Florian Weimer
@ 2021-05-24 11:27 ` Alexander Monakov
2021-05-24 11:42 ` Alexander Monakov
2021-05-24 13:08 ` H.J. Lu
1 sibling, 1 reply; 5+ messages in thread
From: Alexander Monakov @ 2021-05-24 11:27 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha
On Mon, 24 May 2021, Florian Weimer via Libc-alpha wrote:
> * H. J. Lu via Libc-alpha:
>
> > 1. Replace
> >
> > if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0)
> >
> > which may be optimized by compiler, with
> >
> > static int
> > __attribute__ ((noclone, noinline))
> > aligned (void *p, int align)
> > {
> > return (((uintptr_t) p) & (align - 1)) != 0;
> > }
>
> I think you need to use __attribute__ ((weak)) (and drop the static) for
> a fully effective compiler barrier. Due to the new global nature of the
> symbol, you probably should switch to a different identifier less prone
> to collisions.
>
> GCC 8 also supports the noipa attribute, but that's too recent for us.
... at which point it's cleaner to use a non-transparent copy:
uintptr_t p_repr;
asm("" : "=r"(p_repr) : "0"(p));
return (p_repr & (align - 1)) != 0;
But in any case this should not be needed. The patch does not seems to explain
the motivation properly (in general a pointer may be arbitrarily misaligned,
the compiler needs to see either the object from which the pointer is derived,
or a dereference of the pointer to make any assumptions about its alignment).
Alexander
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Properly check stack alignment [BZ #27901]
2021-05-24 11:27 ` Alexander Monakov
@ 2021-05-24 11:42 ` Alexander Monakov
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Monakov @ 2021-05-24 11:42 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha
On Mon, 24 May 2021, Alexander Monakov via Libc-alpha wrote:
> But in any case this should not be needed. The patch does not seems to explain
> the motivation properly (in general a pointer may be arbitrarily misaligned,
> the compiler needs to see either the object from which the pointer is derived,
> or a dereference of the pointer to make any assumptions about its alignment).
Ah, looking once again at the diffs I see that's because existing tests were
too naive and completely ineffective.
Alexander
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Properly check stack alignment [BZ #27901]
2021-05-24 10:56 ` Florian Weimer
2021-05-24 11:27 ` Alexander Monakov
@ 2021-05-24 13:08 ` H.J. Lu
1 sibling, 0 replies; 5+ messages in thread
From: H.J. Lu @ 2021-05-24 13:08 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha
On Mon, May 24, 2021 at 3:56 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > 1. Replace
> >
> > if ((((uintptr_t) &_d) & (__alignof (double) - 1)) != 0)
> >
> > which may be optimized by compiler, with
> >
> > static int
> > __attribute__ ((noclone, noinline))
> > aligned (void *p, int align)
> > {
> > return (((uintptr_t) p) & (align - 1)) != 0;
> > }
>
> I think you need to use __attribute__ ((weak)) (and drop the static) for
> a fully effective compiler barrier. Due to the new global nature of the
> symbol, you probably should switch to a different identifier less prone
> to collisions.
I changed it to
int
__attribute__ ((weak, noclone, noinline))
is_aligned (void *p, int align)
{
return (((uintptr_t) p) & (align - 1)) != 0;
}
> GCC 8 also supports the noipa attribute, but that's too recent for us.
>
> Thanks,
> Florian
>
I sent out the v2 patch.
--
H.J.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-24 13:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-23 17:47 [PATCH] Properly check stack alignment [BZ #27901] H.J. Lu
2021-05-24 10:56 ` Florian Weimer
2021-05-24 11:27 ` Alexander Monakov
2021-05-24 11:42 ` Alexander Monakov
2021-05-24 13:08 ` H.J. Lu
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).