[pushed 1/2] [gdb/testsuite] Fix data alignment in gdb.arch/i386-{avx, sse}.exp

[pushed 1/2] [gdb/testsuite] Fix data alignment in gdb.arch/i386-{avx, sse}.exp

[Tom de Vries]

When running test-case gdb.arch/i386-avx.exp with clang I ran into:
...
(gdb) PASS: gdb.arch/i386-avx.exp: set first breakpoint in main
continue^M
Continuing.^M
^M
Program received signal SIGSEGV, Segmentation fault.^M
0x000000000040052b in main (argc=1, argv=0x7fffffffd3c8) at i386-avx.c:54^M
54        asm ("vmovaps 0(%0), %%ymm0\n\t"^M
(gdb) FAIL: gdb.arch/i386-avx.exp: continue to breakpoint: \
  continue to first breakpoint in main
...

The problem is that the vmovaps insn requires an 256-bit (or 32-byte) aligned
address, and it's only 16-byte aligned:
...
(gdb) p /x $rax
$1 = 0x601030
...

Fix this by using a sufficiently aligned address, using _Alignas.

Compile using -std=gnu11 to support _Alignas.

Likewise in gdb.arch/i386-sse.exp.

Tested on x86_64-linux, with both gcc and clang.
---
 gdb/testsuite/gdb.arch/i386-avx.c   |  5 ++++-
 gdb/testsuite/gdb.arch/i386-avx.exp | 11 ++++++++---
 gdb/testsuite/gdb.arch/i386-sse.c   |  5 ++++-
 gdb/testsuite/gdb.arch/i386-sse.exp | 11 ++++++++---
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index 4e938399a24..765026c83fb 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -20,8 +20,11 @@
 #include <stdio.h>
 #include "nat/x86-cpuid.h"
 
+/* Align sufficient to be able to use vmovaps.  */
+#define ALIGN 32
+
 typedef struct {
-  float f[8];
+  _Alignas (ALIGN) float f[8];
 } v8sf_t;
 
 
diff --git a/gdb/testsuite/gdb.arch/i386-avx.exp b/gdb/testsuite/gdb.arch/i386-avx.exp
index 1b61a65d605..d721b8c4624 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.exp
+++ b/gdb/testsuite/gdb.arch/i386-avx.exp
@@ -35,12 +35,17 @@ if [get_compiler_info] {
     return -1
 }
 
-set additional_flags ""
+set flags { debug }
+
+# C11 for _Alignas, gnu for asm.
+lappend flags additional_flags=-std=gnu11
+
 if { [test_compiler_info gcc*] || [test_compiler_info clang*] } {
-    set additional_flags "additional_flags=-mavx -I${srcdir}/.."
+    lappend flags "additional_flags=-mavx -I${srcdir}/.."
 }
 
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+	  $flags] != "" } {
     unsupported "compiler does not support AVX"
     return
 }
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index a5941a4071e..10bd4b62ff0 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -20,8 +20,11 @@
 #include <stdio.h>
 #include "nat/x86-cpuid.h"
 
+/* Align sufficient to be able to use movaps.  */
+#define ALIGN 16
+
 typedef struct {
-  float f[4];
+  _Alignas (ALIGN) float f[4];
 } v4sf_t;
 
 
diff --git a/gdb/testsuite/gdb.arch/i386-sse.exp b/gdb/testsuite/gdb.arch/i386-sse.exp
index 1ee3a8444b3..15649e435f7 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.exp
+++ b/gdb/testsuite/gdb.arch/i386-sse.exp
@@ -30,12 +30,17 @@ if [get_compiler_info] {
     return -1
 }
 
-set additional_flags ""
+set flags { debug }
+
+# C11 for _Alignas, gnu for asm.
+lappend flags additional_flags=-std=gnu11
+
 if { [test_compiler_info gcc*] || [test_compiler_info clang*] } {
-    set additional_flags "additional_flags=-msse -I${srcdir}/.."
+    lappend flags "additional_flags=-msse -I${srcdir}/.."
 }
 
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+	  $flags] != "" } {
     unsupported "compiler does not support SSE"
     return
 }

base-commit: f21dbd7c8038acee8ece4d57f2454083c37e98f6
-- 
2.31.1

[pushed 2/2] [gdb/testsuite] Use precise align in gdb.arch/i386-{avx, sse}.exp

[Tom de Vries]

Test-cases gdb.arch/i386-{avx,sse}.exp use assembly instructions that require
the memory operands to be aligned to a certain boundary, and the test-cases
use C11's _Alignas to make that happen.

The draw-back of using _Alignas is that while it does enforce a minimum
alignment, the actual alignment may be bigger, which makes the following
scenario possible:
- copy say, gdb.arch/i386-avx.c as basis for a new test-case
- run the test-case and observe a PASS
- commit the new test-case in the supposition that the test-case is correct
  and well-tested
- run later into a failure on a different test setup (which may be a setup
  where reproduction and investigation is more difficult and time-consuming),
  and find out that the specified alignment was incorrect and should have been
  updated to say, 64 bytes.  The initial PASS occurred only because the actual
  alignment happened to be greater than required.

The idea of having precise alignment as a means of having more predictable
execution which allows flushing out bugs earlier, has been filed as PR
gcc/103095.

Add a new file lib/precise-aligned-alloc.c with functions
precise_aligned_alloc and precise_aligned_dup, to support precise alignment.

Use precise_aligned_dup in aforementioned test-cases to:
- verify that the specified alignment is indeed sufficient, rather
  than too little but accidentally over-aligned.
- prevent the same type of problems in any new test-cases based on these

Tested on x86_64-linux, with both gcc and clang.
---
 gdb/testsuite/gdb.arch/i386-avx.c         | 10 ++-
 gdb/testsuite/gdb.arch/i386-sse.c         | 11 ++-
 gdb/testsuite/lib/precise-aligned-alloc.c | 87 +++++++++++++++++++++++
 3 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/lib/precise-aligned-alloc.c

diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index 765026c83fb..7936e1ad1f6 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -28,7 +28,7 @@ typedef struct {
 } v8sf_t;
 
 
-v8sf_t data[] =
+v8sf_t data_orig[] =
   {
     { {  0.0,  0.125,  0.25,  0.375,  0.50,  0.625,  0.75,  0.875 } },
     { {  1.0,  1.125,  1.25,  1.375,  1.50,  1.625,  1.75,  1.875 } },
@@ -50,10 +50,16 @@ v8sf_t data[] =
 #endif
   };
 
+#include "../lib/precise-aligned-alloc.c"
 
 int
 main (int argc, char **argv)
 {
+  void *allocated_ptr;
+  v8sf_t *data
+    = precise_aligned_dup (ALIGN, sizeof (data_orig), &allocated_ptr,
+			   data_orig);
+
   asm ("vmovaps 0(%0), %%ymm0\n\t"
        "vmovaps 32(%0), %%ymm1\n\t"
        "vmovaps 64(%0), %%ymm2\n\t"
@@ -110,5 +116,7 @@ main (int argc, char **argv)
 
   puts ("Bye!"); /* second breakpoint here */
 
+  free (allocated_ptr);
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index 10bd4b62ff0..e51f88b4ed7 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -28,7 +28,7 @@ typedef struct {
 } v4sf_t;
 
 
-v4sf_t data[] =
+v4sf_t data_orig[] =
   {
     { {  0.0,  0.25,  0.50,  0.75 } },
     { {  1.0,  1.25,  1.50,  1.75 } },
@@ -65,9 +65,16 @@ have_sse (void)
     return 0;
 }
 
+#include "../lib/precise-aligned-alloc.c"
+
 int
 main (int argc, char **argv)
 {
+  void *allocated_ptr;
+  v4sf_t *data
+    = precise_aligned_dup (ALIGN, sizeof (data_orig), &allocated_ptr,
+			   data_orig);
+
   if (have_sse ())
     {
       asm ("movaps 0(%0), %%xmm0\n\t"
@@ -127,5 +134,7 @@ main (int argc, char **argv)
       puts ("Bye!"); /* second breakpoint here */
     }
 
+  free (allocated_ptr);
+
   return 0;
 }
diff --git a/gdb/testsuite/lib/precise-aligned-alloc.c b/gdb/testsuite/lib/precise-aligned-alloc.c
new file mode 100644
index 00000000000..418118cb9da
--- /dev/null
+++ b/gdb/testsuite/lib/precise-aligned-alloc.c
@@ -0,0 +1,87 @@
+/* This test file is part of GDB, the GNU debugger.
+
+   Copyright 2021 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program 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 General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <stdlib.h>
+#include <assert.h>
+#include <string.h>
+#include <stdint.h>
+
+/* Return true if address P is ALIGNMENT-byte aligned.  */
+
+static int
+is_aligned (void *p, size_t alignment)
+{
+  size_t mask = (alignment - 1);
+  return ((uintptr_t)p & mask) == 0;
+}
+
+/* Allocate SIZE memory with ALIGNMENT, and return it.  If FREE_POINTER,
+   return in it the corresponding pointer to be passed to free.
+
+   Do the alignment precisely, in other words, if an alignment of 4 is
+   requested, make sure the pointer is 4-byte aligned, but not 8-byte
+   aligned.  In other words, make sure the pointer is not overaligned.
+
+   The benefit of using precise alignment is that accidentally specifying
+   a too low alignment will not be compensated by accidental
+   overalignment.  */
+
+static void *
+precise_aligned_alloc (size_t alignment, size_t size, void **free_pointer)
+{
+  /* Allocate extra to compensate for "p += alignment".  */
+  size_t alloc_size = size + alignment;
+
+  /* Align extra, to be able to do precise align.  */
+  void *p = aligned_alloc (alignment * 2, alloc_size);
+  assert (p != NULL);
+  void *p_orig = p;
+  void *p_end = p + alloc_size;
+
+  /* Make p precisely aligned.  */
+  p += alignment;
+
+  /* Verify p is without bounds, and points to large enough area.  */
+  assert (p >= p_orig);
+  assert (p + size <= p_end);
+
+  /* Verify required alignment.  */
+  assert (is_aligned (p, alignment));
+
+  /* Verify required alignment is precise.  */
+  assert (! is_aligned (p, 2 * alignment));
+
+  if (free_pointer != NULL)
+    *free_pointer = p_orig;
+
+  return p;
+}
+
+/* Duplicate data SRC of size SIZE to a newly allocated, precisely aligned
+   location with alignment ALIGNMENT.  */
+
+static void *
+precise_aligned_dup (size_t alignment, size_t size, void **free_pointer,
+		     void *src)
+{
+  void *p = precise_aligned_alloc (alignment, size, free_pointer);
+
+  memcpy (p, src, size);
+
+  return p;
+}
-- 
2.31.1