From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 871C2385BF9D for ; Fri, 5 Nov 2021 12:23:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 871C2385BF9D Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 99EAF2171F; Fri, 5 Nov 2021 12:23:46 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 7D05A13A2C; Fri, 5 Nov 2021 12:23:46 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id RBO2HFIihWFxRQAAMHmgww (envelope-from ); Fri, 05 Nov 2021 12:23:46 +0000 Subject: Re: [PATCH] [gdb/testsuite] Fix gdb.arch/i386-avx.exp with clang To: Andrew Burgess Cc: gdb-patches@sourceware.org References: <20211104135559.5875-1-tdevries@suse.de> <20211105093300.GG918204@redhat.com> <20211105115404.GA1816063@redhat.com> From: Tom de Vries Message-ID: Date: Fri, 5 Nov 2021 13:23:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20211105115404.GA1816063@redhat.com> Content-Type: multipart/mixed; boundary="------------2D9BF1CBB5B7EC21531F628F" Content-Language: en-US X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Nov 2021 12:23:49 -0000 This is a multi-part message in MIME format. --------------2D9BF1CBB5B7EC21531F628F Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 11/5/21 12:54 PM, Andrew Burgess wrote: > * Tom de Vries via Gdb-patches [2021-11-05 10:43:38 +0100]: > >> On 11/5/21 10:33 AM, Andrew Burgess wrote: >>> * Tom de Vries via Gdb-patches [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 . */ >>>> >>>> #include >>>> +#include >>>> +#include >>>> +#include >>>> #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. > Right, I'm aware of this, but that's a typical case where we have no portable alternative. > 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. I prefer using portable constructs if possible, and reasonably maintainable. Anyway, the solution I've implemented has one further benefit: it provides precise alignment, such that accidentally specifying a too low alignment cannot be compensated by accidental surplus alignment. [ I've also filed a related gcc PR ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103095 ). ] This version also fixes i386-sse.c. Any further comments? Thanks, - Tom --------------2D9BF1CBB5B7EC21531F628F Content-Type: text/x-patch; charset=UTF-8; name="0001-gdb-testsuite-Fix-data-alignment-in-gdb.arch-i386-avx-sse-.exp.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-gdb-testsuite-Fix-data-alignment-in-gdb.arch-i386-avx-s"; filename*1="se-.exp.patch" [gdb/testsuite] Fix data alignment in gdb.arch/i386-{avx,sse}.exp 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. Likewise in gdb.arch/i386-sse.exp. Tested on x86_64-linux, with both gcc and clang. --- gdb/testsuite/gdb.arch/i386-avx.c | 9 +++- gdb/testsuite/gdb.arch/i386-sse.c | 10 +++- gdb/testsuite/lib/precise-aligned-alloc.c | 89 +++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c index 4e938399a24..255ff5ee6f5 100644 --- a/gdb/testsuite/gdb.arch/i386-avx.c +++ b/gdb/testsuite/gdb.arch/i386-avx.c @@ -25,7 +25,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 } }, @@ -47,10 +47,15 @@ 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 (32, 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" @@ -107,5 +112,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 a5941a4071e..c78a510c1a7 100644 --- a/gdb/testsuite/gdb.arch/i386-sse.c +++ b/gdb/testsuite/gdb.arch/i386-sse.c @@ -25,7 +25,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 } }, @@ -62,9 +62,15 @@ 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 (16, sizeof (data_orig), &allocated_ptr, data_orig); + if (have_sse ()) { asm ("movaps 0(%0), %%xmm0\n\t" @@ -124,5 +130,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..67b6f2bc618 --- /dev/null +++ b/gdb/testsuite/lib/precise-aligned-alloc.c @@ -0,0 +1,89 @@ +/* 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 . +*/ + +#include +#include +#include +#include + +/* 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) +{ + size_t used_alignment = 2 * alignment; + size_t used_size = size + used_alignment; + + void *p = malloc (used_size); + assert (p != NULL); + void *p_end = p + used_size; + + if (free_pointer != NULL) + *free_pointer = p; + + void *p_orig = p; + + /* Align to used_alignment. */ + size_t mask = (used_alignment - 1); + if (((uintptr_t)p & mask) == 0) + ; + else + { + p = (void*)((uintptr_t)p & ~mask); + p += used_alignment; + } + + 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. */ + mask = (alignment - 1); + assert (((uintptr_t)p & mask) == 0); + + /* Verify required alignment is precise. */ + mask = (used_alignment - 1); + assert (((uintptr_t)p & mask) != 0); + + 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; +} --------------2D9BF1CBB5B7EC21531F628F--