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: 780 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 32E5B360078 for ; Thu, 1 May 2014 12:06:46 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14307373) id A971E4138D42B; Thu, 1 May 2014 12:06:45 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx20.g.dreamhost.com 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 homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id 760EE4138D4B0 for ; Thu, 1 May 2014 12:06:45 -0700 (PDT) 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=GL3tvq6TF0pdt6ZsGYSeCEHYvjH/0OJBrXRxAreYwwo e8QVUEWx5M6HiXkiuiIY+0u9QDBHURHW4LIDj3mce6lA4qauJwD8DnMf72yAehkr KtQ/7D+o+s3/8pQE35yIk8JMtuTlfF3QBA1oJ06oVhKDBp9gqgOBO9Va8WIGayUM = 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=hziZC+0IKPi8sRwRH +TFKTN2QU9kZ1AcaTFfKocURk1P1NtDuPWHM1zokT6DpiV5OXw8pwQbZAYmN3f1C l7VIS/rPY3VZG6anMWvYnewdlFdfoCgnPdgQErRvggtN7l1QDotzCy3XJSISOEcg PF8BUndzXcroXyTJCDduPiL1f0= 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 X-DH-Original-To: glibc@patchwork.siddhesh.in 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