From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bisque.elm.relay.mailchannels.net (bisque.elm.relay.mailchannels.net [23.83.212.18]) by sourceware.org (Postfix) with ESMTPS id 388693858C5F for ; Fri, 4 Aug 2023 16:37:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 388693858C5F 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 C93F6805DA; Fri, 4 Aug 2023 16:36:59 +0000 (UTC) Received: from pdx1-sub0-mail-a299.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 5A3BF808A7; Fri, 4 Aug 2023 16:36:59 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1691167019; a=rsa-sha256; cv=none; b=+DmUcaXMqUN9mr/nVRXgXNJ8+Y/BLvkK8zxyJ2qOe6ru5jtW79KnMmZmKxK4HnXbFJVjac M3nJKRHq3ax96b+YuOZvYwkV0psoqylP4c6sqdn70/MEykFbNvVi5Lyuo9qogrktv/fBEo mSol/euLSixKiJlomPWFv8ljcJHB0rrIdbTSsJWl888fKoUK3f9scd9ZKr0c/+0wG0fBqw Uzz3HtGFxwIfRYVgnnEUoypH9ImbRzr6accub1N8cU9v//huCtoEP9sgSHh/cRF6uBbMb5 TIfYMz1akL/3p7PWPuuLvJStPwy9AC8qkEImy9IFnDWcY/glBnHLfopCaiX9Zg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1691167019; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc: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=bhI6NahNbXWYnS8h3yb+4q+A7crat4AbUJYJO/eNmZs=; b=SOWVeR6Qv6tIX1MHoXPgE8f1AYk3z8ozutr3SpUcz4ls9QvXzfdBSObH5/ylhoikyW/+ma 9UZ3Hb/mQW9A73y7gpGKanEdN3aHFDD3l7LYcx/91WyVRb9U4/qO8BVYmjTqcotZy0cUna +NIp9s1VKhZ8m/2iw8mww9I9WLrSM9CW2hRkUVcVkGprMsCrzLDxz0vGut3sy7AhOR3tEc /cfaMiF6UP6vCSlZKr0PdxDMT/Kr9nyXuQy6HMtUMtr0ss7C4Fujj+pxlUCCjcye/AN3Y3 ldas/x7jvWAwXn4R/JhTUMS2GsMmsaACynsejZ2MaHSk3xap2qIvvEdD7pMosw== ARC-Authentication-Results: i=1; rspamd-849d547c58-llh4g; 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-Society-Cooperative: 1b0db99622759bdf_1691167019614_441859538 X-MC-Loop-Signature: 1691167019614:952347284 X-MC-Ingress-Time: 1691167019614 Received: from pdx1-sub0-mail-a299.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.114.181.233 (trex/6.9.1); Fri, 04 Aug 2023 16:36:59 +0000 Received: from [192.168.2.12] (bras-vprn-toroon4834w-lp130-02-142-113-138-184.dsl.bell.ca [142.113.138.184]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a299.dreamhost.com (Postfix) with ESMTPSA id 4RHWZf6BQrz6J; Fri, 4 Aug 2023 09:36:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1691167019; bh=bhI6NahNbXWYnS8h3yb+4q+A7crat4AbUJYJO/eNmZs=; h=Date:Subject:To:Cc:From:Content-Type:Content-Transfer-Encoding; b=v66mOkEGwLgScCXiEDZOLx02Aubb0AmSz7mfrTJx0PAQ6IRcdQCnM8EFf7EVYTx+E 302OcCPyMktUt6hsXHD9sG3LfPHLkbps+HNFmQdSccoPszE6VTojgHW5tERKTQk1yC PjHw8cH1KQKQkhvAKKIb61jUTfEi3iMf2ssz0GKn8brm/cgEE6bgY00gMXzKEB+rx9 vK3zLcavotnKvxYK1sZoOnF6RmVhCCSRvlwnAOfXnQ25KtYtCNzeAvSv1HqAmpvoXb 8y6OWmgmqI12QatwyY2nES/R3CkyuM5iRnfgpfSwvvifh4tWqcDAZ8Qj8V6L8jfBxd VkLcmlbG2eBvg== Message-ID: Date: Fri, 4 Aug 2023 12:36:57 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: One question on the source code of tree-object-size.cc Content-Language: en-US To: Qing Zhao Cc: Kees Cook , jakub Jelinek , Qing Zhao via Gcc-patches References: <052CB546-4787-48A9-8B87-BA909DA60388@oracle.com> <202308011549.DFC402C@keescook> <46A77E9B-7A6B-41B3-95B0-A6752671B09C@oracle.com> <7602f2ad-a4ab-0104-1679-bde15b7284a8@gotplt.org> <7E92B817-375C-4979-ACA3-C18EB2B4BA11@oracle.com> From: Siddhesh Poyarekar In-Reply-To: <7E92B817-375C-4979-ACA3-C18EB2B4BA11@oracle.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3029.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,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 2023-08-04 11:27, Qing Zhao wrote: > > >> On Aug 4, 2023, at 10:40 AM, Siddhesh Poyarekar wrote: >> >> On 2023-08-03 13:34, Qing Zhao wrote: >>> One thing I need to point out first is, currently, even for regular fixed size array in the structure, >>> We have this same issue, for example: >>> #define LENGTH 10 >>> struct fix { >>> size_t foo; >>> int array[LENGTH]; >>> }; >>> … >>> int main () >>> { >>> struct fix *p; >>> p = alloc_buf_more (); >>> expect(__builtin_object_size(p->array, 1), LENGTH * sizeof(int)); >>> expect(__builtin_object_size(p->array, 0), -1); >>> } >>> Currently, for __builtin_object_size(p->array, 0), GCC return UNKNOWN for it. >>> This is not a special issue for flexible array member. >> >> That's fine for fixed arrays at the end of a struct because the "whole object" size could be anything; `p` could be pointing to the beginning of an array for all we know. If however `array` is strictly a flex array, i.e.: >> >> ``` >> struct A >> { >> size_t foo; >> int array[]; >> }; >> ``` >> >> then there's no way in valid C to have an array of `struct fix`, > > Yes!! this is exactly the place that makes difference between structures with fixed arrays and the ones with flexible arrays. > > With such difference, I guess that using the type of the structure with flexible array member for p->array to get the size of the whole object p point to might be reasonable? Yes, that's what I'm thinking. >> so `q` must be pointing to a single element. So you could deduce: >> >> 1. the minimum size of the whole object that q points to. > > You mean that the TYPE will determine the minimum size of the whole object? (Does this include the size of the flexible array member, or only the other part of the structure except the flexible array member?) Only the constant sized part of the structure. >> Actually for minimum size we'd also need a guarantee that `alloc_buf_more` returns a valid allocated object. > > Why? Please explain a little bit here. So `alloc_buf_more` could return NULL, a valid pointer or an invalid pointer. So, we could end up returning a non-zero minimum size for an invalid or NULL pointer, which is incorrect, we don't know that. We won't need the object validity guarantee for (2) beyond, e.g. guarding against a new NULL pointer dereference because it's a *maximum* estimate; an invalid or NULL pointer would have 0 size. So for such cases, __bos(q, 0) could return sizeof(*q) + (q ? q->foo:0) and __bos(q->array, 0) could be sizeof(*q) + q->foo - offsetof(q, array) There's no need to guard against a dereference in the second case because the q->array dereference already assumes that q is valid. >> >> and >> >> 2. if you're able to determine the size of the flex array (through __element_count__(foo) for example), you could even determine the maximum size of the whole object. >> >> For (2) though, you'd break applications that overallocate and then expect to be able to use that overallocation despite the space not being reflected in the __element_count__. I think it's a bug in the application and I can't see a way for an application to be able to do this in a valid way so I'm inclined towards breaking it. > > Currently, we allow the situation when the allocation size for the whole object is larger than the value reflected in the “counted_by” attribute (the old name is __element_count__). But don’t allow the other way around (i.e, when the allocation size for the whole object is smaller than the value reflected in the “counted_by” attribute. Right, that's going to be the "break". For underallocation __bos will only end up overestimating the space available, which is not ideal, but won't end up breaking compatibility. >> >> Of course, the fact that gcc allows flex arrays to be in the middle of structs breaks the base assumption but that's something we need to get rid of anyway since there's no way for valid C programs to use that safely. > > Since GCC14, we started to deprecate this extension (allow flex array to be in the middle of structs). > https://gcc.gnu.org/pipermail/gcc-cvs/2023-June/385730.html Yes, that's what I'm banking on. Thanks, Sid