From patchwork Thu May 1 19:06:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Miller X-Patchwork-Id: 344645 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 39C121400EA for ; Fri, 2 May 2014 05:06:49 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:message-id:to:cc:subject:from:in-reply-to :references:mime-version:content-type:content-transfer-encoding; q=dns; s=default; b=bYYzChvM5/y1ltqR5PE7vIo90WCNCJRg85mr1WFmtcs ZP8GGDGL7bP31DhB1EQAqONQR0nyNJcVE5fwKIxwWRoJoofCIVayNDVrVIORykC/ Y2adKlfRzn0KYTuQFuUc/h9uGQwndiiiQu3O3mpWnm+owui4/2V2bGNl4bsORibs = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:message-id:to:cc:subject:from:in-reply-to :references:mime-version:content-type:content-transfer-encoding; s=default; bh=uGVI1HXtw4f30ovCtOu3bNioQI0=; b=ZNOlEjeP8UzsAfxKi y8553U7zB1qasURW1NS0ZXdJ/2B1nZwT8RPhEhR6OzbVLVjFjiV4V/JJT+oxTFBx ZkICUg9fBNDjJIaZ1GelipNmrBorgNB0rMvAE5PmBKIEJD1nVmM1OPghD9e6klsk mg52bl/i97XvrCUkAtXNZJZf3A= Received: (qmail 16268 invoked by alias); 1 May 2014 19:06:43 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 16254 invoked by uid 89); 1 May 2014 19:06:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: shards.monkeyblade.net Date: Thu, 01 May 2014 15:06:33 -0400 (EDT) Message-Id: <20140501.150633.524985722540419460.davem@davemloft.net> To: carlos@redhat.com Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] Fix v9/64-bit strcmp when string ends in multiple zero bytes. From: David Miller In-Reply-To: <5361DCC9.1000408@redhat.com> References: <20140430.160000.1171411871854680001.davem@davemloft.net> <5361DCC9.1000408@redhat.com> Mime-Version: 1.0 From: "Carlos O'Donell" Date: Thu, 01 May 2014 01:34:01 -0400 > On 04/30/2014 04:00 PM, David Miller wrote: >> + /* Test cases where there are multiple zero bytes after the first. */ >> + >> + for (size_t i = 0; i < 8; i++) > > Cover the full length of the available strings e.g. MIN(l1, l2); That evaluates to "3", which would test less. :-) The important bit is to make sure we cover placing the initial terminating zero byte at every byte offset within a word, therefore testing up to 16 characters more than covers all of the possible cases. >> + { >> + int exp_result; >> + >> + for (CHAR val = 0x01; val < 0x10; val++) > > Permute over all char values e.g. [0x1,0xff] or val < 0x100; Ok, and we have to use 'int' for this otherwise we loop forever. >> + { >> + for (size_t j = 0; j < i; j++) >> + { >> + s1[j] = val; >> + s2[j] = val; >> + } >> + >> + s1[i] = 0x00; >> + s2[i] = val; >> + >> + for (size_t j = i + 1; j < 16; j++) >> + { >> + s1[j] = 0x00; >> + s2[j] = 0x00; >> + } > > As i only moves forward and j fills with val up to i, > this loop clears more than it should? > > Hoist this out of the loop and just initialize both of > the strings to 0x00. Agreed, this was excessive. > OK with those changes and verification that it still > catches the original failure case and hasn't exponentially > blown up the test time :} Thanks for your feedback Carlos, here is what I will commit. ==================== Fix v9/64-bit strcmp when string ends in multiple zero bytes. [BZ #16885] * sysdeps/sparc/sparc64/strcmp.S: Fix end comparison handling when multiple zero bytes exist at the end of a string. Reported by Aurelien Jarno * string/test-strcmp.c (check): Add explicit test for situations where there are multiple zero bytes after the first. --- ChangeLog | 10 ++++++++++ string/test-strcmp.c | 28 ++++++++++++++++++++++++++++ sysdeps/sparc/sparc64/strcmp.S | 31 +++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/ChangeLog b/ChangeLog index 55724f6..581d243 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2014-05-01 David S. Miller + + [BZ #16885] + * sysdeps/sparc/sparc64/strcmp.S: Fix end comparison handling when + multiple zero bytes exist at the end of a string. + Reported by Aurelien Jarno + + * string/test-strcmp.c (check): Add explicit test for situations where + there are multiple zero bytes after the first. + 2014-05-01 Andreas Schwab [BZ #16890] diff --git a/string/test-strcmp.c b/string/test-strcmp.c index b395dc7..fcd059f 100644 --- a/string/test-strcmp.c +++ b/string/test-strcmp.c @@ -329,6 +329,34 @@ check (void) FOR_EACH_IMPL (impl, 0) check_result (impl, s1 + i1, s2 + i2, exp_result); } + + /* Test cases where there are multiple zero bytes after the first. */ + + for (size_t i = 0; i < 16 + 1; i++) + { + s1[i] = 0x00; + s2[i] = 0x00; + } + + for (size_t i = 0; i < 16; i++) + { + int exp_result; + + for (int val = 0x01; val < 0x100; val++) + { + for (size_t j = 0; j < i; j++) + { + s1[j] = val; + s2[j] = val; + } + + s2[i] = val; + + exp_result = SIMPLE_STRCMP (s1, s2); + FOR_EACH_IMPL (impl, 0) + check_result (impl, s1, s2, exp_result); + } + } } diff --git a/sysdeps/sparc/sparc64/strcmp.S b/sysdeps/sparc/sparc64/strcmp.S index 8925396..312924a 100644 --- a/sysdeps/sparc/sparc64/strcmp.S +++ b/sysdeps/sparc/sparc64/strcmp.S @@ -121,6 +121,37 @@ ENTRY(strcmp) movleu %xcc, -1, %o0 srlx rTMP1, 7, rTMP1 + /* In order not to be influenced by bytes after the zero byte, we + * have to retain only the highest bit in the mask for the comparison + * with rSTRXOR to work properly. + */ + mov 0, rTMP2 + andcc rTMP1, 0x0100, %g0 + + movne %xcc, 8, rTMP2 + sllx rTMP1, 63 - 16, %o1 + + movrlz %o1, 16, rTMP2 + sllx rTMP1, 63 - 24, %o1 + + movrlz %o1, 24, rTMP2 + sllx rTMP1, 63 - 32, %o1 + + movrlz %o1, 32, rTMP2 + sllx rTMP1, 63 - 40, %o1 + + movrlz %o1, 40, rTMP2 + sllx rTMP1, 63 - 48, %o1 + + movrlz %o1, 48, rTMP2 + sllx rTMP1, 63 - 56, %o1 + + movrlz %o1, 56, rTMP2 + + srlx rTMP1, rTMP2, rTMP1 + + sllx rTMP1, rTMP2, rTMP1 + cmp rTMP1, rSTRXOR retl movgu %xcc, 0, %o0