From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47278 invoked by alias); 22 Aug 2017 14:57:04 -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 46471 invoked by uid 89); 22 Aug 2017 14:57:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] To: libc-alpha@sourceware.org References: <20170820171713.GA19531@gmail.com> <25604b34-7afb-7007-4ea8-3add9963d4b4@linux.vnet.ibm.com> <10fd8eb0-0f53-9d4b-dcd2-694a1878c994@linux.vnet.ibm.com> From: Stefan Liebler Date: Tue, 22 Aug 2017 14:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 x-cbid: 17082214-0008-0000-0000-0000048F751F X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17082214-0009-0000-0000-00001E1F884D Message-Id: <3cf66cda-dcdf-1e56-d0d9-1f015d486692@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-22_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1708220230 X-SW-Source: 2017-08/txt/msg01062.txt.bz2 On 08/22/2017 02:43 PM, H.J. Lu wrote: > On Tue, Aug 22, 2017 at 5:07 AM, Stefan Liebler wrote: >> On 08/22/2017 01:05 AM, H.J. Lu wrote: >>> >>> On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler >>> wrote: >>>> >>>> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>>>> >>>>> >>>>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler >>>>> >>>>> wrote: >>>>>> >>>>>> >>>>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>>>> >>>>>>> stratcliff.c: In function ‘do_test’: >>>>>>> cc1: error: assuming signed overflow does not occur when assuming that >>>>>>> (X >>>>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>>>> >>>>>>> OK for master? >>>>>>> >>>>>>> H.J. >>>>>>> --- >>>>>>> [BZ #21982] >>>>>>> * string/stratcliff.c (do_test): Declare size, nchars, >>>>>>> inner, >>>>>>> middle and outer with size_t instead of int. Repleace %d >>>>>>> with >>>>>>> %Zd in printf. >>>>>>> --- >>>>>>> string/stratcliff.c | 72 >>>>>>> ++++++++++++++++++++++++++--------------------------- >>>>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>>>> >>>>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>>>> index e28b0c5058..ae780379cb 100644 >>>>>>> --- a/string/stratcliff.c >>>>>>> +++ b/string/stratcliff.c >>>>>>> @@ -58,8 +58,8 @@ >>>>>>> int >>>>>>> do_test (void) >>>>>>> { >>>>>>> - int size = sysconf (_SC_PAGESIZE); >>>>>>> - int nchars = size / sizeof (CHAR); >>>>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>>>> + size_t nchars = size / sizeof (CHAR); >>>>>>> CHAR *adr; >>>>>>> CHAR *dest; >>>>>>> int result = 0; >>>>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>>>> } >>>>>>> else >>>>>>> { >>>>>>> - int inner, middle, outer; >>>>>>> + size_t inner, middle, outer; >>>>>>> >>>>>>> mprotect (adr, size, PROT_NONE); >>>>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>>>> >>>>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >>>>>>> { >>>>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>>>> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >>>>>>> STRINGIFY (STRLEN), outer, inner); >>>>>>> result = 1; >>>>>>> } >>>>>>> { >>>>>>> - printf ("%s flunked for outer = %d, middle = %d\n", >>>>>>> + printf ("%s flunked for outer = %Zd, middle = >>>>>>> %Zd\n", >>>>>>> STRINGIFY (rawmemchr), outer, middle); >>>>>>> result = 1; >>>>>>> } >>>>>>> Hi H.J. Lu, >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> I've applied your patch and the warnings does not occur anymore on >>>>>> s390. >>>>> >>>>> >>>>> >>>>> Great. >>>>> >>>>>> The outer loops of the string tests are all using the following: >>>>>> size_t nchars, outer; >>>>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>>>> >>>>>> I think we can assume, that nchars is always > 128 as it is derived by >>>>>> the >>>>>> pagesize. >>>>>> But if nchars would be equal to 128, this would result in an infinite >>>>>> loop >>>>>> (outer >= 0)? >>>>>> If nchars would be less than 128, the tests would be skipped. >>>>>> >>>>>> Should we add a check that nchars > 128 at the beginning and replace >>>>>> the >>>>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>>>> >>>>> >>>>> >>>>> This is a separate issue beyond BZ #21982. >>>>> >>>>> >>>> Your patch is introducing this behaviour. >>>> Before your patch, nchars and outer was an int and the for-loop-condition >>>> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or to >>>> skipping the test if nchars <= 128. >>>> >>> >>> How about this patch? >>> >> This solves the cases if nchars < 128. >> But if nchars == 128, then the condition of the for-loop is "size_t outer >= >> 0", which is always true. >> >> Could we check once if nchars > 128 and exit the test with an error if >> nchars is <= 128? >> Are there architectures where the page size is < 4096? >> Or where wchar_t > 4byte? >> > > Here is the updated patch. I added > > if (outer == 0) > break; > > at the end of loop. > Okay. This fixes the case nchars == 128. I've retested this patch on s390x with gcc 7 -O3 and the warnings does not occur anymore. Thanks. Stefan