From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 25C413858C20 for ; Wed, 16 Feb 2022 14:08:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 25C413858C20 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-318-MmAQexW6MumpcFJTriLnaw-1; Wed, 16 Feb 2022 09:08:10 -0500 X-MC-Unique: MmAQexW6MumpcFJTriLnaw-1 Received: by mail-qk1-f200.google.com with SMTP id q5-20020ae9dc05000000b00507225deac5so1203013qkf.5 for ; Wed, 16 Feb 2022 06:08:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:organization:in-reply-to :content-transfer-encoding; bh=qHEpzoA0+3rNMz2tixOyjvIY0exJVbsMBiECYj/1mBw=; b=WK009trdOjH1Cxd5lXqcjMWMW0iNnMczhlNrNeUqdvTNAkIT2ape6qkLQJtVHHhYfv HJsnadMbP06fTXpn1hFMExwUkvjUrlsWboK7DrTeAFPjAULfRwU9QNSCxWavEbvRPoBQ udXxu49/xdbb6h54rIi4w5QBfU+oFQYtyM3ktkBsWpTSNC3cnLaEsmvevcVAZMHNoeiV 0r1pLNwgFuZZ6DiePcVipHXJNf5HZksNQj1itjpxbO/bx3XJn18ca3zEtFjd9DyAtIXm 6AgqrLlUF9CXDEs07Tt114T7pJbluxAc3KOgk0qEBbyXinYlxDuSMNCX6Bdz6OZ+yREV SMpg== X-Gm-Message-State: AOAM530QxVPTZVPWDhov3nxAxk1dDRrAsCU2P1HHtHePo4gH8NqPfFg+ u//ZPA/KrnnyDyGdYhay0TJy9ReIYuYzDY7sEq6dv6vSwQJpsDDFX7XH2w0YcjUHgOeH5Y47rtJ LLVksIOA45xr6pC1vqoDZ X-Received: by 2002:a05:622a:2c6:b0:2d2:6502:316b with SMTP id a6-20020a05622a02c600b002d26502316bmr2020263qtx.36.1645020489963; Wed, 16 Feb 2022 06:08:09 -0800 (PST) X-Google-Smtp-Source: ABdhPJz43Trq1y0K5msDRYFwaaEukqxV2VAqHJL//L4tgaFluGkDVCr0WCP2xLbIgRplwd6BpvWpPA== X-Received: by 2002:a05:622a:2c6:b0:2d2:6502:316b with SMTP id a6-20020a05622a02c600b002d26502316bmr2020235qtx.36.1645020489676; Wed, 16 Feb 2022 06:08:09 -0800 (PST) Received: from [192.168.0.241] (135-23-175-80.cpe.pppoe.ca. [135.23.175.80]) by smtp.gmail.com with ESMTPSA id bm27sm17971327qkb.5.2022.02.16.06.08.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Feb 2022 06:08:09 -0800 (PST) Message-ID: <867c131c-cec4-d8d6-5f24-cd51de94f1b2@redhat.com> Date: Wed, 16 Feb 2022 09:08:07 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Subject: Re: [PATCH v3] x86: Fallback {str|wcs}cmp RTM in the ncmp overflow case [BZ #28896] To: "H.J. Lu" , Noah Goldstein Cc: GNU C Library References: <20220215162751.281955-1-goldstein.w.n@gmail.com> <20220216080935.3284536-1-goldstein.w.n@gmail.com> From: Carlos O'Donell Organization: Red Hat In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Feb 2022 14:08:19 -0000 On 2/16/22 08:17, H.J. Lu via Libc-alpha wrote: > On Wed, Feb 16, 2022 at 12:09 AM Noah Goldstein wrote: >> >> In the overflow fallback strncmp-avx2-rtm and wcsncmp-avx2-rtm would >> call strcmp-avx2 and wcsncmp-avx2 respectively. This would have >> not checks around vzeroupper and would trigger spurious >> aborts. This commit fixes that. >> >> test-strcmp, test-strncmp, test-wcscmp, and test-wcsncmp all pass on >> AVX2 machines with and without RTM. >> >> Co-authored-by: H.J. Lu >> --- >> sysdeps/x86/Makefile | 2 +- >> sysdeps/x86/tst-strncmp-rtm.c | 14 +++++++++++++- >> sysdeps/x86_64/multiarch/strcmp-avx2.S | 8 ++------ >> sysdeps/x86_64/multiarch/strncmp-avx2-rtm.S | 1 + >> sysdeps/x86_64/multiarch/strncmp-avx2.S | 1 + >> sysdeps/x86_64/multiarch/wcsncmp-avx2-rtm.S | 2 +- >> sysdeps/x86_64/multiarch/wcsncmp-avx2.S | 2 +- >> 7 files changed, 20 insertions(+), 10 deletions(-) >> >> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile >> index 6cf708335c..d110f7b7f2 100644 >> --- a/sysdeps/x86/Makefile >> +++ b/sysdeps/x86/Makefile >> @@ -109,7 +109,7 @@ CFLAGS-tst-memset-rtm.c += -mrtm >> CFLAGS-tst-strchr-rtm.c += -mrtm >> CFLAGS-tst-strcpy-rtm.c += -mrtm >> CFLAGS-tst-strlen-rtm.c += -mrtm >> -CFLAGS-tst-strncmp-rtm.c += -mrtm >> +CFLAGS-tst-strncmp-rtm.c += -mrtm -Wno-error >> CFLAGS-tst-strrchr-rtm.c += -mrtm >> endif >> >> diff --git a/sysdeps/x86/tst-strncmp-rtm.c b/sysdeps/x86/tst-strncmp-rtm.c >> index 09ed6fa0d6..ebc94a3a6d 100644 >> --- a/sysdeps/x86/tst-strncmp-rtm.c >> +++ b/sysdeps/x86/tst-strncmp-rtm.c >> @@ -16,6 +16,7 @@ >> License along with the GNU C Library; if not, see >> . */ >> >> +#include >> #include >> >> #define LOOP 3000 >> @@ -45,8 +46,19 @@ function (void) >> return 1; >> } >> >> +__attribute__ ((noinline, noclone)) >> +static int >> +function_overflow (void) >> +{ >> + if (strncmp (string1, string2, SIZE_MAX) == 0) >> + return 0; >> + else >> + return 1; >> +} >> + >> static int >> do_test (void) >> { >> - return do_test_1 ("strncmp", LOOP, prepare, function); >> + return (do_test_1 ("strncmp", LOOP, prepare, function) >> + || do_test_1 ("strncmp", LOOP, prepare, function_overflow)); >> } >> diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S >> index 99e5349be8..6da0e1a248 100644 >> --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S >> +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S >> @@ -193,10 +193,10 @@ L(ret_zero): >> .p2align 4,, 5 >> L(one_or_less): >> jb L(ret_zero) >> -# ifdef USE_AS_WCSCMP >> /* 'nbe' covers the case where length is negative (large >> unsigned). */ >> - jnbe __wcscmp_avx2 >> + jnbe OVERFLOW_STRCMP >> +# ifdef USE_AS_WCSCMP >> movl (%rdi), %edx >> xorl %eax, %eax >> cmpl (%rsi), %edx >> @@ -205,10 +205,6 @@ L(one_or_less): >> negl %eax >> orl $1, %eax >> # else >> - /* 'nbe' covers the case where length is negative (large >> - unsigned). */ >> - >> - jnbe __strcmp_avx2 >> movzbl (%rdi), %eax >> movzbl (%rsi), %ecx >> subl %ecx, %eax >> diff --git a/sysdeps/x86_64/multiarch/strncmp-avx2-rtm.S b/sysdeps/x86_64/multiarch/strncmp-avx2-rtm.S >> index 37d1224bb9..68bad365ba 100644 >> --- a/sysdeps/x86_64/multiarch/strncmp-avx2-rtm.S >> +++ b/sysdeps/x86_64/multiarch/strncmp-avx2-rtm.S >> @@ -1,3 +1,4 @@ >> #define STRCMP __strncmp_avx2_rtm >> #define USE_AS_STRNCMP 1 >> +#define OVERFLOW_STRCMP __strcmp_avx2_rtm >> #include "strcmp-avx2-rtm.S" >> diff --git a/sysdeps/x86_64/multiarch/strncmp-avx2.S b/sysdeps/x86_64/multiarch/strncmp-avx2.S >> index 1678bcc235..f138e9f1fd 100644 >> --- a/sysdeps/x86_64/multiarch/strncmp-avx2.S >> +++ b/sysdeps/x86_64/multiarch/strncmp-avx2.S >> @@ -1,3 +1,4 @@ >> #define STRCMP __strncmp_avx2 >> #define USE_AS_STRNCMP 1 >> +#define OVERFLOW_STRCMP __strcmp_avx2 >> #include "strcmp-avx2.S" >> diff --git a/sysdeps/x86_64/multiarch/wcsncmp-avx2-rtm.S b/sysdeps/x86_64/multiarch/wcsncmp-avx2-rtm.S >> index 4e88c70cc6..f467582cbe 100644 >> --- a/sysdeps/x86_64/multiarch/wcsncmp-avx2-rtm.S >> +++ b/sysdeps/x86_64/multiarch/wcsncmp-avx2-rtm.S >> @@ -1,5 +1,5 @@ >> #define STRCMP __wcsncmp_avx2_rtm >> #define USE_AS_STRNCMP 1 >> #define USE_AS_WCSCMP 1 >> - >> +#define OVERFLOW_STRCMP __wcscmp_avx2_rtm >> #include "strcmp-avx2-rtm.S" >> diff --git a/sysdeps/x86_64/multiarch/wcsncmp-avx2.S b/sysdeps/x86_64/multiarch/wcsncmp-avx2.S >> index 4fa1de4d3f..e9ede522b8 100644 >> --- a/sysdeps/x86_64/multiarch/wcsncmp-avx2.S >> +++ b/sysdeps/x86_64/multiarch/wcsncmp-avx2.S >> @@ -1,5 +1,5 @@ >> #define STRCMP __wcsncmp_avx2 >> #define USE_AS_STRNCMP 1 >> #define USE_AS_WCSCMP 1 >> - >> +#define OVERFLOW_STRCMP __wcscmp_avx2 >> #include "strcmp-avx2.S" >> -- >> 2.25.1 >> > > LGTM. > > Reviewed-by: H.J. Lu > > Thanks. > This fails CI for i686. https://patchwork.sourceware.org/project/glibc/patch/20220216080935.3284536-1-goldstein.w.n@gmail.com/ -- Cheers, Carlos.