Re: [PATCH] [gdb/testsuite] Fix gdb.arch/i386-avx.exp with clang

[Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>]

* Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> [2021-11-05 10:43:38 +0100]:

> On 11/5/21 10:33 AM, Andrew Burgess wrote:
> > * Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> [2021-11-04 14:55:59 +0100]:
> > 
> >> 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 copying to a sufficiently aligned address.
> >>
> >> Tested on x86_64-linux, with both gcc and clang.
> >> ---
> >>  gdb/testsuite/gdb.arch/i386-avx.c | 28 +++++++++++++++++++++++++++-
> >>  1 file changed, 27 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
> >> index 4e938399a24..9b5323f9f76 100644
> >> --- a/gdb/testsuite/gdb.arch/i386-avx.c
> >> +++ b/gdb/testsuite/gdb.arch/i386-avx.c
> >> @@ -18,6 +18,9 @@
> >>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >>  
> >>  #include <stdio.h>
> >> +#include <stdint.h>
> >> +#include <assert.h>
> >> +#include <string.h>
> >>  #include "nat/x86-cpuid.h"
> >>  
> >>  typedef struct {
> >> @@ -25,7 +28,7 @@ typedef struct {
> >>  } v8sf_t;
> >>  
> >>  
> >> -v8sf_t data[] =
> >> +v8sf_t data_orig[] =
> > 
> > I see the same problem.  Did you consider using:
> > 
> >   /* Some useful comment ....  */
> >   v8sf_t data[] __attribute__ ((aligned(32))) = ....
> > 
> > this seems to fix the problem on clang for me, and still works fine
> > with gcc.
> 
> I did consider this, and decided against it because it's not
> portable.
> 
> Note btw that there is no other usage of this:
> ...
> $ find gdb/testsuite/ -type f | xargs grep attribute.*align
> $

No, but in gdb/testsuite/lib/attribute.h we do setup a compatibility
macro for 'noclone', so there's definitely precedent for using
attributes that might not be supported everywhere.

I'd hope most production level compilers would, if they don't support
'aligned' have something similar/equivalent.

Personally, I'd go with a compatibility macro, and let folk who care
about other compilers figure out what they need when they hit the
problem.  But I'm not blocking your proposed solution if you feel
strongly about it.

Thanks,
Andrew



> ...
> 
> Btw, I now realize that gdb.arch/i386-sse.exp has the same issue, I'll
> try to factor out a solution that can used in both test-cases.
> 
> Thanks,
> - Tom