From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id B5CDA3858D37 for ; Tue, 21 Mar 2023 09:40:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B5CDA3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id AD93421A6B; Tue, 21 Mar 2023 09:40:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1679391641; 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=BIeIrEZFUicMX9O7K0PU/1feXznDLshywkHzNRpGUZY=; b=zlrmua9AA4zBHNFazMKPK+AsT0hsx8Zee9/Tn6spMRTg5QlBZdIxf1fW0PB0R4j+5lZJop Bn3L2GR9iHZHYYtYenCuiwE9KwWpbLgxeofdeYzvkPvHe30WZtJj2wCngaZGQRvqeCeIni zu9sW7fL2ZX/aTK5DTKJlzJdWaWpdEI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1679391641; 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=BIeIrEZFUicMX9O7K0PU/1feXznDLshywkHzNRpGUZY=; b=8eRSwrDCWm78BBvEC8ptwvSC5tUTQPrJOe5GG0NaKXjjygyUGwubXmjVLkPA6xD7gYUDkn 1G7e25Ej454QplCQ== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 9BD512C141; Tue, 21 Mar 2023 09:40:41 +0000 (UTC) Date: Tue, 21 Mar 2023 09:40:41 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: Jeff Law , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] tree: Fix up component_ref_sam_type handling of arrays of 0 sized elements [PR109215] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 Tue, 21 Mar 2023, Jakub Jelinek wrote: > Hi! > > Our documentation sadly talks about elt_type arr[0]; as zero-length arrays, > not arrays with zero elements. Unfortunately, those aren't the only arrays > which can have zero size, the same size can be also result of zero-length > element, like in GNU C struct whatever {} or in GNU C/C++ if the element > type is [0] array or combination thereof (dunno if Ada doesn't allow > something similar too). One can't do much with them, taking address of > their elements, (no-op) copying of the elements in and out. But they > behave differently from arr[0] arrays e.g. in that using non-zero indexes > in them (as long as they are within bounds as for normal arrays) is valid. > > I think this naming inaccuracy resulted in Martin designing > special_array_member in an inconsistent way, mixing size zero array members > with array members of one or two or more elements and then using the > size zero interchangeably with zero elements. > > The following patch changes that (but doesn't do any > documentation/diagnostics renaming, as this is really a corner case), > such that int_0/trail_0 for consistency is just about [0] arrays > plus [] for the latter, not one or more zero sized elements case. > > The testcase has one xfailed case for where perhaps in later GCC versions > we could add extra code to handle it, for some reason we don't diagnose > out of bounds accesses for the zero sized elements cases. It will be > harder because e.g. FRE will canonicalize &var.fld[0] and &var.fld[10] > to just one of them because they are provably the same address. > But the important thing is to fix this regression (where we warn on > completely valid code in the Linux kernel). Anyway, for further work > on this we don't really need any extra help from special_array_member, > all code can just check integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (type))), > it doesn't depend on the position of the members etc. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Thanks, Richard. > 2023-03-21 Jakub Jelinek > > PR tree-optimization/109215 > * tree.h (enum special_array_member): Adjust comments for int_0 > and trail_0. > * tree.cc (component_ref_sam_type): Clear zero_elts if memtype > has zero sized element type and the array has variable number of > elements or constant one or more elements. > (component_ref_size): Adjust comments, formatting fix. > > * gcc.dg/Wzero-length-array-bounds-3.c: New test. > > --- gcc/tree.h.jj 2023-03-14 19:11:52.296936422 +0100 > +++ gcc/tree.h 2023-03-20 18:48:23.068788830 +0100 > @@ -5579,8 +5579,8 @@ extern tree component_ref_field_offset ( > enum struct special_array_member > { > none, /* Not a special array member. */ > - int_0, /* Interior array member with size zero. */ > - trail_0, /* Trailing array member with size zero. */ > + int_0, /* Interior array member with zero elements. */ > + trail_0, /* Trailing array member with zero elements. */ > trail_1, /* Trailing array member with one element. */ > trail_n, /* Trailing array member with two or more elements. */ > int_n /* Interior array member with one or more elements. */ > --- gcc/tree.cc.jj 2023-03-10 10:38:46.551473829 +0100 > +++ gcc/tree.cc 2023-03-20 19:41:35.605580732 +0100 > @@ -13032,14 +13032,27 @@ component_ref_sam_type (tree ref) > return sam_type; > > bool trailing = false; > - (void)array_ref_flexible_size_p (ref, &trailing); > - bool zero_length = integer_zerop (memsize); > - if (!trailing && !zero_length) > - /* MEMBER is an interior array with > - more than one element. */ > + (void) array_ref_flexible_size_p (ref, &trailing); > + bool zero_elts = integer_zerop (memsize); > + if (zero_elts && integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (memtype)))) > + { > + /* If array element has zero size, verify if it is a flexible > + array member or zero length array. Clear zero_elts if > + it has one or more members or is a VLA member. */ > + if (tree dom = TYPE_DOMAIN (memtype)) > + if (tree min = TYPE_MIN_VALUE (dom)) > + if (tree max = TYPE_MAX_VALUE (dom)) > + if (TREE_CODE (min) != INTEGER_CST > + || TREE_CODE (max) != INTEGER_CST > + || !((integer_zerop (min) && integer_all_onesp (max)) > + || tree_int_cst_lt (max, min))) > + zero_elts = false; > + } > + if (!trailing && !zero_elts) > + /* MEMBER is an interior array with more than one element. */ > return special_array_member::int_n; > > - if (zero_length) > + if (zero_elts) > { > if (trailing) > return special_array_member::trail_0; > @@ -13047,7 +13060,7 @@ component_ref_sam_type (tree ref) > return special_array_member::int_0; > } > > - if (!zero_length) > + if (!zero_elts) > if (tree dom = TYPE_DOMAIN (memtype)) > if (tree min = TYPE_MIN_VALUE (dom)) > if (tree max = TYPE_MAX_VALUE (dom)) > @@ -13114,14 +13127,14 @@ component_ref_size (tree ref, special_ar > > tree afield_decl = TREE_OPERAND (ref, 1); > gcc_assert (TREE_CODE (afield_decl) == FIELD_DECL); > - /* if the trailing array is a not a flexible array member, treat it as > + /* If the trailing array is a not a flexible array member, treat it as > a normal array. */ > if (DECL_NOT_FLEXARRAY (afield_decl) > && *sam != special_array_member::int_0) > return memsize; > > if (*sam == special_array_member::int_0) > - memsize = NULL_TREE; > + memsize = NULL_TREE; > > /* For a reference to a flexible array member of a union > use the size of the union instead of the size of the member. */ > @@ -13129,7 +13142,7 @@ component_ref_size (tree ref, special_ar > memsize = TYPE_SIZE_UNIT (argtype); > } > > - /* MEMBER is either a bona fide flexible array member, or a zero-length > + /* MEMBER is either a bona fide flexible array member, or a zero-elements > array member, or an array of length one treated as such. */ > > /* If the reference is to a declared object and the member a true > --- gcc/testsuite/gcc.dg/Wzero-length-array-bounds-3.c.jj 2023-03-20 19:17:21.290648885 +0100 > +++ gcc/testsuite/gcc.dg/Wzero-length-array-bounds-3.c 2023-03-20 19:16:48.494123583 +0100 > @@ -0,0 +1,19 @@ > +/* PR tree-optimization/109215 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wall" } */ > + > +struct S {}; > +struct T { struct S s[3]; struct S t; }; > +void foo (struct S *); > + > +void > +bar (struct T *t) > +{ > + foo (&t->s[2]); /* { dg-bogus "array subscript 2 is outside the bounds of an interior zero-length array" } */ > +} > + > +void > +baz (struct T *t) > +{ > + foo (&t->s[3]); /* { dg-error "" "" { xfail *-*-* } } */ > +} > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)