From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from insect.birch.relay.mailchannels.net (insect.birch.relay.mailchannels.net [23.83.209.93]) by sourceware.org (Postfix) with ESMTPS id BAD6E3835767 for ; Wed, 22 Jun 2022 07:09:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BAD6E3835767 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 110CA821885; Wed, 22 Jun 2022 07:09:17 +0000 (UTC) Received: from pdx1-sub0-mail-a304.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 5ED0782178F; Wed, 22 Jun 2022 07:09:16 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1655881756; a=rsa-sha256; cv=none; b=OoGl5sNkqUg3Okh8s+NneNaA261YxbB66cp3hsKyY89QbQZ7144e6m7cT2dK72b51JtHRm zCgyi76BFVYugYLP8HZtUMeto7DjFJSPSBSnes7UyScCkrRLQmKpX+pxfIUO932wzu6bir S6fILITb40no42JzNCTYEPivkAk+vTr2Wv1sOGqPq8Jz/bAzqqVIuA8VsJ8LnJ1nvQgO20 wimPPUq/+9Zg24j6wsvdmsD9soYHF6oGvrdBC1fpqpCZKPJXQosRUn4iZx0YQYEt+WVju3 XQ3/XxxJe8tPlasF87ntRD3NMrn7iRZBJ+T0MOldiYPwAGl1ajx6fzQTn6J+3g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1655881756; 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:dkim-signature; bh=+XQwPHPdGeqf/feE0077o8rus2Zssi61LAoU0c9lMIA=; b=q6caxcc9rZNiiqgsva2FEpwLf6dI6EFJIrvaFjEoAA9PHnRpsTOkpcyMefQ+Peqw4TkaUo 9N7OHLhnICfiM81qUZr2kC+cBg01D8q89libsME390Zajeo0AzyUOq42Ye8m9S5knwVr1C pLGkho0gZgHJqV7lj85UiwoScDx1akoOILfqKXo4bAir7KXzlK7wKasTwwri+61XN83t7V uxEUqoH+pcUGbKgpI966EGcLTOTqG14TdR6xRxPE/LlbCCVBXzG50vye2z+P/oQ8NtT4YM t2rlWJZv38L4mpu7YfihWZqoa8IFbcjBagZz1SWD112qJUo2+lCJA7yAbx+E6A== ARC-Authentication-Results: i=1; rspamd-848669fb87-q5qbq; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Shade-Occur: 61bae90d59212a83_1655881756629_1101451230 X-MC-Loop-Signature: 1655881756629:3432041710 X-MC-Ingress-Time: 1655881756628 Received: from pdx1-sub0-mail-a304.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.125.123.1 (trex/6.7.1); Wed, 22 Jun 2022 07:09:16 +0000 Received: from [192.168.86.152] (unknown [103.199.173.7]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a304.dreamhost.com (Postfix) with ESMTPSA id 4LSZHt73hJz6G; Wed, 22 Jun 2022 00:09:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1655881756; bh=+XQwPHPdGeqf/feE0077o8rus2Zssi61LAoU0c9lMIA=; h=Date:Subject:To:From:Content-Type:Content-Transfer-Encoding; b=mShtCCkBFmptgAjGHq0qy6Sa8fWCTNu8kOgKfNYwzg/NJGmXNzuAlWtRL6YAUSfcB MQ810m2M+wsBGOtvKuTrjWHhCfawn9H9lkd0x0hf0ebyu6khp3n9qPRT2M5p2FxNy9 kwysQ01KbvDcU7OgDSenXhTc2oTOsyDNFMCW3xZ2GU5KrE4SHgwYCuec0/rJNUZ7CO eBdFJHAiErOOit4HBpOOAGn/GMEQ7qaaouH0yiXziNnEKfPuimoPZh2MItkqCQswGo /IqtYlozVKFr5OgBq08lMviBtP1XUlLxk3x8ZXUybUH8z7Y66Fs6eZhn53NWtMqAaM O4vRSAhgmimvA== Message-ID: <982ee9ea-3169-4eb4-78d1-efc13dc3b672@gotplt.org> Date: Wed, 22 Jun 2022 12:39:07 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v1] stdlib: Remove attr_write from mbstows if dst is NULL [BZ: 29265] Content-Language: en-US To: Noah Goldstein , libc-alpha@sourceware.org References: <20220621161821.2940071-1-goldstein.w.n@gmail.com> From: Siddhesh Poyarekar In-Reply-To: <20220621161821.2940071-1-goldstein.w.n@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3035.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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, 22 Jun 2022 07:09:21 -0000 On 21/06/2022 21:48, Noah Goldstein via Libc-alpha wrote: > mbstows is defined if dst is NULL and is defined to special cased if > dst is NULL so the fortify objsize check if incorrect in that case. > > Tested on x86-64 linux. > --- > Note. I wasn't able to get the test to actually throw an error > before the change. The test would rely on whether the middle end is able to fold away the condition late enough, so it's flaky enough for us to not care about it. It's fine to have it in though as a smoke test. > > > stdlib/Makefile | 3 +++ > stdlib/bits/stdlib.h | 16 +++++++++++----- > stdlib/testmb.c | 7 +++++++ > 3 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/stdlib/Makefile b/stdlib/Makefile > index 60fc59c12c..6ef725ef74 100644 > --- a/stdlib/Makefile > +++ b/stdlib/Makefile > @@ -373,6 +373,9 @@ CFLAGS-tst-qsort.c += $(stack-align-test-flags) > CFLAGS-tst-makecontext.c += -funwind-tables > CFLAGS-tst-makecontext2.c += $(stack-align-test-flags) > > +CFLAGS-testmb.c += -D_FORTIFY_SOURCE=2 -Wall -Werror > + > + > # Run a test on the header files we use. > tests-special += $(objpfx)isomac.out > > diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h > index 277d099e22..9ab66db6a4 100644 > --- a/stdlib/bits/stdlib.h > +++ b/stdlib/bits/stdlib.h > @@ -96,6 +96,11 @@ extern size_t __mbstowcs_chk (wchar_t *__restrict __dst, > const char *__restrict __src, > size_t __len, size_t __dstlen) __THROW > __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2)); > +extern size_t __REDIRECT_NTH (__mbstowcs_chk_nulldst, > + (wchar_t *__restrict __dst, > + const char *__restrict __src, > + size_t __len), mbstowcs_chk) > + __attr_access ((__read_only__, 2)); > extern size_t __REDIRECT_NTH (__mbstowcs_alias, > (wchar_t *__restrict __dst, > const char *__restrict __src, > @@ -108,16 +113,17 @@ extern size_t __REDIRECT_NTH (__mbstowcs_chk_warn, > __warnattr ("mbstowcs called with dst buffer smaller than len " > "* sizeof (wchar_t)"); > > -__fortify_function size_t > +__always_inline __fortify_function size_t > __NTH (mbstowcs (wchar_t *__restrict __dst, const char *__restrict __src, > size_t __len)) > { > - return __glibc_fortify_n (mbstowcs, __len, sizeof (wchar_t), > - __glibc_objsize (__dst), > - __dst, __src, __len); > + if (__builtin_constant_p (__dst) && __dst == NULL) Perhaps a better condition would be __builtin_constant_p (__dst == NULL) && __dst == NULL so that it does not rely on __dst to be a constant. For example, it could be a variable that the compiler decides can have only a NULL value. > + return __mbstowcs_chk_nulldst (__dst, __src, __len); > + else > + return __glibc_fortify_n (mbstowcs, __len, sizeof (wchar_t), > + __glibc_objsize (__dst), __dst, __src, __len); OK. > } > > - > extern size_t __wcstombs_chk (char *__restrict __dst, > const wchar_t *__restrict __src, > size_t __len, size_t __dstlen) __THROW > diff --git a/stdlib/testmb.c b/stdlib/testmb.c > index 45dae7db61..6ac4dfd21d 100644 > --- a/stdlib/testmb.c > +++ b/stdlib/testmb.c > @@ -16,6 +16,13 @@ main (int argc, char *argv[]) > lose = 1; > } > > + i = mbstowcs (NULL, "bar", 4); > + if (!(i == 3 && w[1] == 'a')) > + { > + puts ("mbstowcs FAILED2!"); > + lose = 1; > + } > + > mbstowcs (w, "blah", 5); > i = wcstombs (c, w, 10); > if (i != 4) OK.