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 DF29F3858D33 for ; Tue, 10 Jan 2023 22:48:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DF29F3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673390883; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=H5KgALOnYlbWpZBEg5CZ6bZjThVOdPQBDZqvk9FGkmY=; b=gJ436A+qHes/bAQ2Xvp4C8TnodbMyOfHvdUpK6J7YOzorg14sIOvamRya//kgR/6RAvPVZ Mg/EX1ddwEgg1Cq6tQOT6KUkJVjC1yXocqYMsRJuARxfbvw9BM3GTltoOYDXvWOZ0e4dVL MyWWTdFi1JdsTyj4D+uwy7cytZLNSSY= Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-654-_VMfBQpCPRm5438MFSBTHg-1; Tue, 10 Jan 2023 17:48:02 -0500 X-MC-Unique: _VMfBQpCPRm5438MFSBTHg-1 Received: by mail-io1-f72.google.com with SMTP id d24-20020a5d9bd8000000b006ee2ddf6d77so7791811ion.6 for ; Tue, 10 Jan 2023 14:48:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=H5KgALOnYlbWpZBEg5CZ6bZjThVOdPQBDZqvk9FGkmY=; b=obtImm53HtnHcUJq2jchi71/Vz09a8IQ98cqhn4h3UABUCwM2/WEVlF8bvy/jnJwDB 3J87/wiuWH23GNkE36FWy8Q2SMyLvUANM7F6pFQg+RYwMYGdxhDJLmuAXvR1CKtr+zze +SgZteWszJOCBtfyqIKg2ykkdHQ/h4jh9DiTVGgu9KSMOXwL/Y6flRdNXQR2nsfhBDx2 jqfsHsrV+NyI0fkPqLvtDSBM2vS4pKCd+a3l7EX/h9hLE6Ow1GOGiPItIzJdtclNLonx 2auCuiVTOj66AEOde1GembkT61ObsADe4ydTxpQlbe2c1rEpo0NetDx//3hJO08YD0Sm PahQ== X-Gm-Message-State: AFqh2kr2gfiDv2J+AASZKY/q/qbNTw/iJJEPDsEFD0r0VUs0Iy6ixqJx V9JC952/jgbFp45gWagoaWsmyulQRnTVqnaGC8ARHfuvzIiCRQk7V5uOLrhxtq72oWsMFpOcRFd lCu8fuvv7goAhmByBWsGe X-Received: by 2002:a5e:db45:0:b0:6bc:d70f:8b38 with SMTP id r5-20020a5edb45000000b006bcd70f8b38mr138038iop.18.1673390881611; Tue, 10 Jan 2023 14:48:01 -0800 (PST) X-Google-Smtp-Source: AMrXdXte4JWJxb7BPv8v99ZVCgYiW0TSUZ7oQv+DW9dVb/83vRM/tB3IQZ2sQvzsUz19bUcGzha4Lw== X-Received: by 2002:a5e:db45:0:b0:6bc:d70f:8b38 with SMTP id r5-20020a5edb45000000b006bcd70f8b38mr138022iop.18.1673390881291; Tue, 10 Jan 2023 14:48:01 -0800 (PST) Received: from [192.168.0.241] (192-0-145-146.cpe.teksavvy.com. [192.0.145.146]) by smtp.gmail.com with ESMTPSA id s185-20020a0251c2000000b0038a56594026sm4039288jaa.66.2023.01.10.14.47.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 10 Jan 2023 14:48:00 -0800 (PST) Message-ID: <773a0c5e-406b-c3c0-a19d-77985a2d54ac@redhat.com> Date: Tue, 10 Jan 2023 17:47:58 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH 3/4] string: Suppress -Wmaybe-unitialized for wordcopy [BZ #19444] To: Adhemerval Zanella , libc-alpha@sourceware.org References: <20221229125802.2715435-1-adhemerval.zanella@linaro.org> <20221229125802.2715435-4-adhemerval.zanella@linaro.org> From: Carlos O'Donell Organization: Red Hat In-Reply-To: <20221229125802.2715435-4-adhemerval.zanella@linaro.org> 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=-12.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 12/29/22 07:58, Adhemerval Zanella via Libc-alpha wrote: I think this particular change needs a v2 and some more detailed analysis. > With GCC 6+ when compiling with -O1 warns that some MERGE macro usage > might be used uninitialized. The issue is calling the function with > len equal to 0 is undefined since the first 'switch' will not trigger > any case and then subsequent loop will potentially use uninitialized > variables. The comment does not seem to match what the code does. Calling the function with len equal to 0 results in 'case 0' executing in all of these cases. > However all usages on mem routines always called the function for > sizes larger than OP_T_THRES. This isn't always the case, you could have a call to copy just 8 bytes, which is below the 16-byt OP_T_THRES. > --- > string/wordcopy.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/string/wordcopy.c b/string/wordcopy.c > index d05718322c..3b6344115d 100644 > --- a/string/wordcopy.c > +++ b/string/wordcopy.c > @@ -18,8 +18,19 @@ > > /* BE VERY CAREFUL IF YOU CHANGE THIS CODE...! */ > > +#include > #include > +#include > +/* With GCC 6 when compiling with -O1 warns that some MERGE macro usage might > + be used uninitialized. The issue is calling the function with len equal to > + 0 is undefined since the first 'switch' will not trigger any case and then > + subsequent loop will potentially use uninitialized variables. However all > + usages on mem routines always called the function for sizes larger than This comment does not match what the code does. The switch 'case 0:' case should execute. > + OP_T_THRES. */ > +DIAG_PUSH_NEEDS_COMMENT; > +DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized"); > #include > +DIAG_POP_NEEDS_COMMENT; > > /* _wordcopy_fwd_aligned -- Copy block beginning at SRCP to > block beginning at DSTP with LEN `op_t' words (not LEN bytes!). > @@ -94,7 +105,11 @@ WORDCOPY_FWD_ALIGNED (long int dstp, long int srcp, size_t len) > { > do8: > a0 = ((op_t *) srcp)[0]; > + /* Check the comment on memcopy.h inclusion. */ > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized"); > ((op_t *) dstp)[0] = a1; > + DIAG_POP_NEEDS_COMMENT; Why is a1 considered uninitialized? The switch has case statements for every possible value. In case 1 we unconditionally set a1. We can't enter case 1 unless len was exactly 1 or any value of 1+8*n for valid n. It seems like the compiler can't see that 'OP_T_THRES <= 3 * OPSIZ' is always a true. > do7: > a1 = ((op_t *) srcp)[1]; > ((op_t *) dstp)[1] = a0; > @@ -291,7 +306,11 @@ WORDCOPY_BWD_ALIGNED (long int dstp, long int srcp, size_t len) > { > do8: > a0 = ((op_t *) srcp)[7]; > + /* Check the comment on memcopy.h inclusion. */ > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized"); > ((op_t *) dstp)[7] = a1; > + DIAG_POP_NEEDS_COMMENT; Likewise. > do7: > a1 = ((op_t *) srcp)[6]; > ((op_t *) dstp)[6] = a0; -- Cheers, Carlos.