From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 121242 invoked by alias); 19 Mar 2018 12:55:36 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 121134 invoked by uid 89); 19 Mar 2018 12:55:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-ot0-f193.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=EqUXwBqjE0kjtVy8v/bhJU7ZQShO4GizWtbcxUBza28=; b=A011yFXOVNcr9jSJy+G4qQ88d51cfyneCZWngCuqfLhD2c8c2Etc/tbBBoOkJHHWtu YcKtv46c8xOmJ5FiyK3bfUEL5I69/0q69vcDVgRY9ySBpWxhHyJFOjLms+355NXxntNA md5dUh3Q1UxMo4rcjL0XnG+rhPcDt2DRAKIQaHvazQVkyJISxE8lruGTrxzoqB21Eo8O GkPbez6Q/3eU5Nm2g+1znxeIALzP4NGeH0OVszaP4SdrAh9gts8SWUdP834tioEcGBUZ R5DVgxlTgzDlbPk+W2ufhvHgBMskKBzompfTOj7MmYdJczjvM5sTV69/Nwg++hurlgKh JvkA== X-Gm-Message-State: AElRT7EtKpsJl3cybQjlxTxmOe21y15APrh3PPEmpU9+rbVhjbnqNSO/ o0wGdAskuEcCXyhYsods33xX8RyJg8axxEW65um8Ow== X-Google-Smtp-Source: AG47ELvxZUV4zQXEXwZV2j7ytB93ax3ZYwSgrTtwMSYmGugX+hJZ7Gu0yzOJV0P+EtBY9K4O0quy9IiQWyKIJFU5wH0= X-Received: by 2002:a9d:387:: with SMTP id f7-v6mr8051038otf.123.1521464114863; Mon, 19 Mar 2018 05:55:14 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: From: "H.J. Lu" Date: Mon, 19 Mar 2018 12:55:00 -0000 Message-ID: Subject: Re: [PATCH] Fix i386 memmove issue [BZ #22644] To: Andrew Senkevich Cc: Andreas Schwab , libc-alpha , Max Horn , thomas@grindinggear.com Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-03/txt/msg00451.txt.bz2 On Mon, Mar 19, 2018 at 5:45 AM, Andrew Senkevich wrote: > 2018-03-14 15:59 GMT+01:00 H.J. Lu : >> On Wed, Mar 14, 2018 at 7:43 AM, Andrew Senkevich >> wrote: >>> 2018-02-19 11:13 GMT+01:00 Andreas Schwab : >>>> On Feb 19 2018, Andrew Senkevich wrote: >>>> >>>>> diff --git a/string/test-memmove.c b/string/test-memmove.c >>>>> index edc7a4c..8dc152b >>>>> --- a/string/test-memmove.c >>>>> +++ b/string/test-memmove.c >>>>> @@ -245,6 +245,49 @@ do_random_tests (void) >>>>> } >>>>> } >>>>> >>>>> +#if __SIZEOF_POINTER__ == 4 >>>>> +static void >>>>> +do_test2 (void) >>>>> +{ >>>>> + uint32_t i; >>>>> + uint32_t num = 0x20000000; >>>>> + uint32_t * large_buf = mmap (0, sizeof(uint32_t) * num, PROT_READ | >>>>> PROT_WRITE, >>>>> + MAP_PRIVATE | MAP_ANON, -1, 0); >>>>> + if (large_buf == MAP_FAILED) >>>>> + error (EXIT_FAILURE, errno, "large mmap failed"); >>>>> + >>>>> + if (!((uint32_t)(large_buf) < (0x80000000 - 128) && (0x80000000 + >>>>> 128) < (uint32_t)(&large_buf[num]))) >>>>> + { >>>>> + error (0, 0,"allocated large memory doesn't cross 0x80000000 boundary"); >>>>> + ret = 1; >>>>> + return; >>>> >>>> Please properly fold long lines, and remove the redundant parens. Also, >>>> there is no guarantee that the address range is unallocated. >>> >>> Thanks, updated patch below. Any comment or it is Ok for trunk? >> >> Please also test crossing 0x80000000 boundary on 64-bit systems. > > Hi, > > I extended test for 64-bit using MAP_FIXED which lets to hardcode > allocated address. Manual says it is less portable, but without > MAP_FIXED I was unable to get proper address for 64bits. > I checked what test fails without fix of memcpy-sse2-unaligned > implementation. Is it Ok for trunk? > > diff --git a/string/test-memmove.c b/string/test-memmove.c > index edc7a4c..5920652 100644 > --- a/string/test-memmove.c > +++ b/string/test-memmove.c > @@ -24,6 +24,7 @@ > # define TEST_NAME "memmove" > #endif > #include "test-string.h" > +#include > > char *simple_memmove (char *, const char *, size_t); > > @@ -245,6 +246,57 @@ do_random_tests (void) > } > } > > +#if __SIZEOF_POINTER__ == 4 > +# define ptr_type uint32_t > +#else > +# define ptr_type uint64_t > +#endif Please use uintptr_t instead. > +static void > +do_test2 (void) > +{ > + uint32_t num = 0x20000000; > + uint32_t * large_buf; > + > + large_buf = mmap ((void*)0x70000000, num, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0); > + > + if (large_buf == MAP_FAILED) > + error (EXIT_FAILURE, errno, "Large mmap failed"); Please EXIT_UNSUPPORTED. > + uint32_t bytes_move = 0x80000000 - (ptr_type)large_buf; > + uint32_t arr_size = bytes_move / sizeof(uint32_t); > + uint32_t i; > + > + FOR_EACH_IMPL (impl, 0) > + { > + for (i = 0; i < arr_size; i++) > + large_buf[i] = i; > + > + uint32_t * dst = &large_buf[33]; > + > +#ifdef TEST_BCOPY > + CALL (impl, (char *)large_buf, (char *)dst, bytes_move); > +#else > + CALL (impl, (char *)dst, (char *)large_buf, bytes_move); > +#endif > + > + for (i = 0; i < arr_size; i++) > + { > + if (dst[i] != i) > + { > + error (0, 0, > + "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%d\"", > + impl->name, dst, large_buf, i); > + ret = 1; > + break; > + } > + } > + } > + > + munmap((void *)large_buf, sizeof(uint32_t) * num); > +} > + > int > test_main (void) > { > @@ -284,6 +336,9 @@ test_main (void) > } > > do_random_tests (); > + > + do_test2 (); > + > return ret; > } > > diff --git a/sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S > b/sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S > index 9c3bbe7..9aa17de 100644 > --- a/sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S > +++ b/sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S > @@ -72,7 +72,7 @@ ENTRY (MEMCPY) > cmp %edx, %eax > > # ifdef USE_AS_MEMMOVE > - jg L(check_forward) > + ja L(check_forward) > > L(mm_len_0_or_more_backward): > /* Now do checks for lengths. We do [0..16], [16..32], [32..64], [64..128] > @@ -81,7 +81,7 @@ L(mm_len_0_or_more_backward): > jbe L(mm_len_0_16_bytes_backward) > > cmpl $32, %ecx > - jg L(mm_len_32_or_more_backward) > + ja L(mm_len_32_or_more_backward) > > /* Copy [0..32] and return. */ > movdqu (%eax), %xmm0 > @@ -92,7 +92,7 @@ L(mm_len_0_or_more_backward): > > L(mm_len_32_or_more_backward): > cmpl $64, %ecx > - jg L(mm_len_64_or_more_backward) > + ja L(mm_len_64_or_more_backward) > > /* Copy [0..64] and return. */ > movdqu (%eax), %xmm0 > @@ -107,7 +107,7 @@ L(mm_len_32_or_more_backward): > > L(mm_len_64_or_more_backward): > cmpl $128, %ecx > - jg L(mm_len_128_or_more_backward) > + ja L(mm_len_128_or_more_backward) > > /* Copy [0..128] and return. */ > movdqu (%eax), %xmm0 > @@ -132,7 +132,7 @@ L(mm_len_128_or_more_backward): > add %ecx, %eax > cmp %edx, %eax > movl SRC(%esp), %eax > - jle L(forward) > + jbe L(forward) > PUSH (%esi) > PUSH (%edi) > PUSH (%ebx) > @@ -269,7 +269,7 @@ L(check_forward): > add %edx, %ecx > cmp %eax, %ecx > movl LEN(%esp), %ecx > - jle L(forward) > + jbe L(forward) > > /* Now do checks for lengths. We do [0..16], [0..32], [0..64], [0..128] > separately. */ > > > -- > WBR, > Andrew -- H.J.