From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x741.google.com (mail-qk1-x741.google.com [IPv6:2607:f8b0:4864:20::741]) by sourceware.org (Postfix) with ESMTPS id 177073870909 for ; Mon, 4 May 2020 18:40:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 177073870909 Received: by mail-qk1-x741.google.com with SMTP id t3so651417qkg.1 for ; Mon, 04 May 2020 11:40:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VkLpuGPSqIqnMeHa/kPwDxxJIgLTgCgbhkedaCUS/ow=; b=ERvUnSy2Bfcc0UZFQ1pyfzETS4oUNYEVd4ULrss00mO4sls0z00SxO7MLdnfz7qPT0 Tbxvagjzb5ta2APoFH1PjsAq6HkbyRZYiEjoFs0Xiq6kFq3tWhDZwW5+5M9GgTH8NZaM 34xObQ+yiXvt819T/4FgA10/nkfwovB/uG6GucT+rN4c28NhjAhmcnoXKe/dK0xgnjOy 2sxSEfF8mvr0qrpRKqY/DzicZrJ5j9SbjQQ14XHHeJ4AFkLCYmaIlgCkuO9UyqtYg/i7 ju7tlTf1eQ0ED0TJEQb15p7OuM2JXZQkppu2nZTKMMbCfycltBZgpwIc0hhr1up4s2Eg 8lYw== X-Gm-Message-State: AGi0PubJEXtwzxRzx1BtglAKJQk10NKd6kHBDknzlHFATehJj40EqBXo OfYHNQmqMfYNDTRvouXUnVfIyhIT X-Google-Smtp-Source: APiQypJHv7KBdpJjXbMs/lhfinugU3oFJ7WGWuvdq+/chsgK8Mx4OGkB9kLnRBmA+HfE5TtoSrpC+Q== X-Received: by 2002:a37:a785:: with SMTP id q127mr566737qke.179.1588617602944; Mon, 04 May 2020 11:40:02 -0700 (PDT) Received: from [192.168.0.41] (174-16-121-251.hlrn.qwest.net. [174.16.121.251]) by smtp.gmail.com with ESMTPSA id k33sm11257862qtd.22.2020.05.04.11.40.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 May 2020 11:40:02 -0700 (PDT) Subject: Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219] From: Martin Sebor To: DJ Delorie Cc: libc-alpha@sourceware.org References: <8d359caf-0522-c753-af3d-2680d54a0cb8@gmail.com> Message-ID: <4ec40000-9842-209d-545a-d19ba41fa3a7@gmail.com> Date: Mon, 4 May 2020 12:40:00 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <8d359caf-0522-c753-af3d-2680d54a0cb8@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Mon, 04 May 2020 18:40:06 -0000 On 5/4/20 11:34 AM, Martin Sebor wrote: > On 5/1/20 4:02 PM, DJ Delorie wrote: >> Martin Sebor writes: >>> Thanks for the careful review! >> >> This new version LGTM. >> >> Reviewed-by: DJ Delorie >> >> (but IMHO a second set of eyes would be good for this one) I missed this suggestion. I'd certainly welcome another pair of eyes on this. Just to be clear: unlike _FORTIFY_SOURCE, the effect of the changes is only to trigger warnings in response to the detected overflow, not actually cause calls to abort at runtime in those cases. Martin >> >>> Yes, that's wrong.  Good catch!  I completely missed stdio when >>> testing so I also didn't notice I forgot to add the attribute to >>> fgets() itself.  I've fixed that in the updated patch. >> >> Ok. >> >>>> IMHO comment should state that the first argument is index 1. >>>> >>>> IMHO should document what happens when size-index is missing. >>> >>> I've tweaked the comment a bit.  I hesitate to go into a lot of >>> detail here and would expect people needing it to read the manual. >> >> Right, but there should be just enough info for someone adding a new use >> of it to know what to do, without requiring the gcc docs.  The new >> comment is fine. >> >>>> __buf[???] >>> >>> When size-index is missing at least one byte of the array must be >>> accessible (or the pointer must be null).  There's no way to specify >>> a constant size with the current syntax.  In the future I'd like to >>> try to teach GCC to get it from the argument itself (for ordinary >>> arrays as well as for VLAs): >> >> Makes sense, just didn't know. >> >>>> NOTE: does not use the __attr_access macro >>> >>> Fixed, thanks. >> >> Ok. >> > > Thanks.  I have committed the latest patch in > 06febd8c6705c816b2f32ee7aa1f4c0184b05248. > > Martin