From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id 94BCE384CB95 for ; Wed, 8 May 2024 07:25:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 94BCE384CB95 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 94BCE384CB95 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715153152; cv=none; b=P/4akGDora51f6riHuNNtuQkkG59rmGFCLhe0DvZFiLfcj7C8ws2IrvE+TBmlNZHVmrzx6nnDmPidXf0HEI4nMtSgpD4CCdKoBYO4r9at3vytH3pvq9+IN0Os7tWPbLtxzKZ7EigbclXG5+DoprT7wmQU4F/xRLAKjt7T4RXV9w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715153152; c=relaxed/simple; bh=E6h0JmhGmWJLsc6rIhkka2Kg7B29CLSqlxXpGV6NEt8=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:Date: From:To:Subject:Message-ID:MIME-Version; b=jTnMejhxxKFNtMb8YtVoidgYU7Qz7Hh+PUMfGnJKvhxapOBPh1j8LvudHAbVcYNH1kBCQjFCg4MjJOVL01Bl3HXCDhupXbgoDYfLPKAruR5GalFurAP7lD84XwgQWGw3zXeLrGrV+nN3ZM3q0D9j5FbVkRtcJjkgOcFdN4yByBM= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from [10.168.5.241] (unknown [10.168.5.241]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 556042289F; Wed, 8 May 2024 07:25:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1715153148; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2wkZtB8pF+zL6mTC2Ii3w6jlaU7N8hJOKDA9/yXvNiE=; b=Ohx4eouX1pv1Ph02/n2U6t+0sZun3Lt+bbXrTalWzaOq40odYET28YrneCaWZhVLtkJTHw 06VeJBFkZV1voApxEepMVkjVSA2xzdx5ydyTNqtnjuQ4flrz73Tv9bkUDuXQlosUO/3p+E qhrGtpA/w5mEpHb20BvU/R3ymVXPJDY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1715153148; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2wkZtB8pF+zL6mTC2Ii3w6jlaU7N8hJOKDA9/yXvNiE=; b=TRuEWXaEe1I2IjZAEED0iSjmDXBq4xR9mumRU1glBqPJUjKw+aF61jime9Y1UNcGSolADV UNoVURdxnkJOjvDA== Authentication-Results: smtp-out1.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1715153148; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2wkZtB8pF+zL6mTC2Ii3w6jlaU7N8hJOKDA9/yXvNiE=; b=Ohx4eouX1pv1Ph02/n2U6t+0sZun3Lt+bbXrTalWzaOq40odYET28YrneCaWZhVLtkJTHw 06VeJBFkZV1voApxEepMVkjVSA2xzdx5ydyTNqtnjuQ4flrz73Tv9bkUDuXQlosUO/3p+E qhrGtpA/w5mEpHb20BvU/R3ymVXPJDY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1715153148; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2wkZtB8pF+zL6mTC2Ii3w6jlaU7N8hJOKDA9/yXvNiE=; b=TRuEWXaEe1I2IjZAEED0iSjmDXBq4xR9mumRU1glBqPJUjKw+aF61jime9Y1UNcGSolADV UNoVURdxnkJOjvDA== Date: Wed, 8 May 2024 09:25:48 +0200 (CEST) From: Richard Biener To: Kees Cook cc: Sebastian Huber , Qing Zhao , "gcc-patches@gcc.gnu.org" Subject: Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members. In-Reply-To: <202405071225.A48DC8630B@keescook> Message-ID: <6np789q5-3207-p6ss-1ssq-92rrr4r098r9@fhfr.qr> References: <20230807142216.1857701-1-qing.zhao@oracle.com> <08858rs4-6qn8-227n-383q-o0ss0p433s07@fhfr.qr> <1fadc7c6-0ce6-4b1c-bbb6-c5d82fb96b28@embedded-brains.de> <5FE67B89-6E43-4499-9CB0-EFE7825DB846@oracle.com> <9107f1fd-0ecd-4b6b-9ad6-744a716678bd@embedded-brains.de> <34CE523E-374C-46A8-B24F-36D54DA2378A@oracle.com> <271D6756-C40D-45A6-9F8C-090F0FEFB3A8@oracle.com> <202405071225.A48DC8630B@keescook> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323328-473326324-1715153148=:2000" X-Spam-Level: X-Spamd-Result: default: False [-3.30 / 50.00]; BAYES_HAM(-3.00)[100.00%]; CTYPE_MIXED_BOGUS(1.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[multipart/mixed,text/plain]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; FROM_HAS_DN(0.00)[]; ARC_NA(0.00)[]; MIME_TRACE(0.00)[0:+,1:+]; TO_DN_EQ_ADDR_SOME(0.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; RCPT_COUNT_THREE(0.00)[4]; TO_DN_SOME(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; RCVD_COUNT_ZERO(0.00)[0]; MISSING_XM_UA(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:email,gnu.org:url] X-Spam-Score: -3.30 X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,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: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323328-473326324-1715153148=:2000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT On Tue, 7 May 2024, Kees Cook wrote: > On Tue, May 07, 2024 at 06:34:19PM +0000, Qing Zhao wrote: > > On May 7, 2024, at 13:57, Sebastian Huber wrote: > > > On 07.05.24 16:26, Qing Zhao wrote: > > > > Hi, Sebastian, > > > > Thanks for your explanation. > > > > Our goal is to deprecate the GCC extension on structure > > > > containing a flexible array member not at the end of another > > > > structure. In order to achieve this goal, we provided the warning option > > > > -Wflex-array-member-not-at-end for the users to locate all such > > > > cases in their source code and update the source code to eliminate > > > > such cases. > > > > > > What is the benefit of deprecating this GCC extension? If GCC > > > extensions are removed, then it would be nice to enable the associated > > > warnings by default. > > The goal of all of the recent array bounds and flexible array work is to > make sizing information unambiguous (e.g. via __builtin_object_size(), > __builtin_dynamic_object_size(), and the array-bounds sanitizer). For > the compiler to be able to deterministically report size information > on arrays, we needed to deprecate this case even though it had been > supported in the past. (Though we also _added_ extensions to support > for other things, like flexible arrays in unions, and the coming > __counted_by attribute.) > > For example: > > struct flex { int length; char data[]; }; > struct mid_flex { int m; struct flex flex_data; int n; int o; }; It might be reasonable to allow tag-less "anonymous" struct that's "completed" by means of the static initializer of a declared object, thus struct { int m; struct { int length, char data[]; } flex_data; int n; int o; } object = { 3, { 2, { 1, 2 } }, 4, 5 }; The frontend would make the size of data[] static, determined by the initializer. I _think_ the C standard makes object.flex_data inter-operate with struct flex from above but struct mid_flex would be an invalid type declaration and thus there's no way to have an API with a pointer to such structure(?) which makes the extension somewhat less useful. > > #define SZ(p) __builtin_dynamic_object_size(p, 1) > > void foo(struct flex *f, struct mid_flex *m) > { > printf("%zu\n", SZ(f)); > printf("%zu\n", SZ(m->flex_data)); > } > > int main(void) > { > struct mid_flex m = { .flex_data.length = 8 }; > foo(&m->flex_data, &m); > return 0; > } > > This is printing the size of the same object. But the desired results > are ambiguous. Does m->flex_data have an unknown size (i.e. SIZE_MAX) > because it's a flex array, or does it contain 8 bytes, since it overlaps > with the other structure's trailing 2 ints? > > The answer from GCC 13 was neither: > > 18446744073709551615 > 4 > > It considered flex_data to be only the size of it's non-flex-array > members, but only when there was semantic context that it was part of > another structure. (Yet more ambiguity.) > > In GCC 14, this is "resolved" to be unknown since it is a flex array > which has no sizing info, and context doesn't matter: > > 18446744073709551615 > 18446744073709551615 > > But this paves the way for the coming 'counted_by' attribute which will > allow for struct flex above to be defined as: > > struct flex { int length; char data[] __attribute__((counted_by(length))); }; > > At which point GCC can deterministically report the object size. > > Hopefully I've captured this all correctly -- Qing can correct me. :) > > > > > > > > We had a long discussion before deciding to deprecating this GCC > > > > extension. Please see details here: > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832 > > > > > > > > Yes, we do plan to enable this warning by default before final > > > > deprecation. (Might consider to enable this warning by default in > > > > GCC15… and then deprecate it in the next release) > > > > > > > > Right now, there is an ongoing work in Linux kernel to get rid of > > > > all such cases. Kees might have more information on this. > > > > > > > > > > > > The static initialization of structures with flexible array members > > > > will still work as long as the flexible array members are at the end of > > > > the structures. > > > > > > Removing the support for flexible array members in the middle of > > > compounds will make the static initialization practically infeasible. > > > > If the flexible array member is moved to the end of the compounds, > > the static initialization still work. What’s the issue here? > > > > > > My question: is it possible to update your source code to move > > > > the structure with flexible array member to the end of the containing > > > > structure? > > > > > > > > i.e, in your example, in the struct Thread_Configured_control, > > > > move the field “Thread_Control Control” to the end of the structure? > > > > > > If we move the Thread_Control to the end, how would I add a > > > configuration defined number of elements at the end? > > > > Don’t understand this, why moving the Thread_Control Control” to > > the end of the containing structure will make this a problem? > > Could you please explain this with a simplified example? > > I found your example at [2] and tried to trim/summarize it here: > > > struct _Thread_Control { > Objects_Control Object; > ... > void *extensions[]; > }; > typedef struct _Thread_Control Thread_Control; > > struct Thread_Configured_control { > Thread_Control Control; > > #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0 > void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ]; > #endif > Configuration_Scheduler_node Scheduler_nodes[ _CONFIGURE_SCHEDULER_COUNT ]; > RTEMS_API_Control API_RTEMS; > #ifdef RTEMS_POSIX_API > POSIX_API_Control API_POSIX; > #endif > #if CONFIGURE_MAXIMUM_THREAD_NAME_SIZE > 1 > char name[ CONFIGURE_MAXIMUM_THREAD_NAME_SIZE ]; > #endif > #if defined(_CONFIGURE_ENABLE_NEWLIB_REENTRANCY) && \ > !defined(_REENT_THREAD_LOCAL) > struct _reent Newlib; > #endif > }; > > #define THREAD_INFORMATION_DEFINE( name, api, cls, max ) \ > ... > static ... \ > Thread_Configured_control \ > name##_Objects[ _Objects_Maximum_per_allocation( max ) ]; \ > ... > > > I don't see any static initialization of struct _Thread_Control::extensions > nor any member initialization of the name##_Objects, and even then that > is all legal in any arrangement: > > truct flex { int length; char data[]; }; > struct mid_flex { int m; struct flex flex_data; int n; int o; }; > struct end_flex { int m; int n; struct flex flex_data; }; > > struct flex f = { .length = 2 }; > struct mid_flex m = { .m = 5 }; > struct end_flex e = { .m = 5 }; > > struct flex fa[4] = { { .length = 2 } }; > struct mid_flex ma[4] = { { .m = 5 } }; > struct end_flex ea[4] = { { .m = 5 } }; > > These all work. > > > But yes, I see why you can't move Thread_Control trivially to the end. It > looks like you're depending on the implicit overlapping memory locations > between struct _Thread_Control and things that include it as the first > struct member, like struct Thread_Configured_control above: > > cpukit/score/src/threaditerate.c: the_thread = (Thread_Control *) information->local_table[ index ]; > > (In the Linux kernel we found this kind of open casting to be very > fragile and instead use a horrific wrapper called "container_of"[3] that > does the pointer math (possibly to an offset of 0 for a direct cast) to > find the member.) > > Anyway, for avoiding the warning, you can just keep using the extension > and add -Wno-... if it ever ends up in -Wall, or you can redefine struct > _Thread_Control to avoid having the "extensions" member at all. This is > what we've done in several cases in Linux. For example if we had this > again, but made to look more like Thread_Control: > > struct flex { int foo; int bar; char data[]; }; > struct mid_flex { struct flex hdr; int n; int o; }; > > It could be changed to: > > struct flex_hdr { int foo; int bar; }; > struct flex { struct flex_hdr hdr; char data[]; }; > struct mid_flex { struct flex_hdr hdr; int n; int o; }; > > This has some collateral changes needed to reference the struct flex_hdr > members from struct flex now (f->hdr.foo instead of f->foo). Sometimes > this can be avoided by using a union, as I did in a recent refactoring > in Linux: [4] > > For more complex cases in Linux we've handled this by using our > "struct_group"[5] macro, which allows for a union and tagged struct to > be constructed: > > struct flex { > __struct_group(flex_hdr, hdr,, > int foo; > int bar; > ); > char data[]; > }; > struct mid_flex { struct flex_hdr hdr; int n; int o; }; > > Then struct flex member names don't have to change, but if anything is > trying to get at struct flex::data through struct mid_flex::hdr, that'll > need casting. But it _shouldn't_ since it has "n" and "o". > > -Kees > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html > [2] https://github.com/RTEMS/rtems > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h#n10 > [4] https://git.kernel.org/linus/896880ff30866f386ebed14ab81ce1ad3710cfc4 > [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/stddef.h?h=v6.8#n11 > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg) --8323328-473326324-1715153148=:2000--