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 1543A3851405; Wed, 9 Nov 2022 07:57:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1543A3851405 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 F1FFA224E9; Wed, 9 Nov 2022 07:57:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1667980629; 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=Ib1F41x/ErUYdDynP4HGSHUHFAOZFHI0NyWlcEawdnA=; b=Eh036MXJjNwEwygnomdpIuDwPE7et+NsO9fSXvbPF5q81TepcDV9Mt3LMCYpPSzsmSsD1s SNI7EiJ0Y+Gz6b02EeCUDJOTVbW0rfOcdr21RFnhhTrJDQhfvus4StcSeqK0pTlZgSQNbJ KPHnmrUvOnQNfQHv0xk2igD7RkRC1R4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1667980629; 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=Ib1F41x/ErUYdDynP4HGSHUHFAOZFHI0NyWlcEawdnA=; b=7YoF/50MNhVGUHrpLbab7C9CLvWYC9lTX12Eb23bFk0HDnTLmeIKBr3+fd1sj2MLW4N753 xA4P/CuhR5sIALAg== 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 AC4EA2C141; Wed, 9 Nov 2022 07:57:08 +0000 (UTC) Date: Wed, 9 Nov 2022 07:57:08 +0000 (UTC) From: Richard Biener To: Qing Zhao cc: joseph@codesourcery.com, gcc-patches@gcc.gnu.org, keescook@chromium.org, siddhesh@gcc.gnu.org Subject: Re: [PATCH 1/2] Change the name of array_at_struct_end_p to array_ref_flexible_size_p In-Reply-To: <20221108145113.955321-2-qing.zhao@oracle.com> Message-ID: References: <20221108145113.955321-1-qing.zhao@oracle.com> <20221108145113.955321-2-qing.zhao@oracle.com> 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=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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, 8 Nov 2022, Qing Zhao wrote: > The name of the utility routine "array_at_struct_end_p" is misleading > and should be changed to a new name that more accurately reflects its > real meaning. > > The routine "array_at_struct_end_p" is used to check whether an array > reference is to an array whose actual size might be larger than its > upper bound implies, which includes 3 different cases: > > A. a ref to a flexible array member at the end of a structure; > B. a ref to an array with a different type against the original decl; > C. a ref to an array that was passed as a parameter; > > The old name only reflects the above case A, therefore very confusing > when reading the corresponding gcc source code. > > In this patch, A new name "array_ref_flexible_size_p" is used to replace > the old name. > > All the references to the routine "array_at_struct_end_p" was replaced > with this new name, and the corresponding comments were updated to make > them clean and consistent. Since you seem to feel strongly about this - OK. Thanks, Richard. > gcc/ChangeLog: > > * gimple-array-bounds.cc (trailing_array): Replace > array_at_struct_end_p with new name and update comments. > * gimple-fold.cc (get_range_strlen_tree): Likewise. > * gimple-ssa-warn-restrict.cc (builtin_memref::builtin_memref): > Likewise. > * graphite-sese-to-poly.cc (bounds_are_valid): Likewise. > * tree-if-conv.cc (idx_within_array_bound): Likewise. > * tree-object-size.cc (addr_object_size): Likewise. > * tree-ssa-alias.cc (component_ref_to_zero_sized_trailing_array_p): > Likewise. > (stmt_kills_ref_p): Likewise. > * tree-ssa-loop-niter.cc (idx_infer_loop_bounds): Likewise. > * tree-ssa-strlen.cc (maybe_set_strlen_range): Likewise. > * tree.cc (array_at_struct_end_p): Rename to ... > (array_ref_flexible_size_p): ... this. > (component_ref_size): Replace array_at_struct_end_p with new name. > * tree.h (array_at_struct_end_p): Rename to ... > (array_ref_flexible_size_p): ... this. > --- > gcc/gimple-array-bounds.cc | 4 ++-- > gcc/gimple-fold.cc | 6 ++---- > gcc/gimple-ssa-warn-restrict.cc | 5 +++-- > gcc/graphite-sese-to-poly.cc | 4 ++-- > gcc/tree-if-conv.cc | 7 +++---- > gcc/tree-object-size.cc | 2 +- > gcc/tree-ssa-alias.cc | 8 ++++---- > gcc/tree-ssa-loop-niter.cc | 15 +++++++-------- > gcc/tree-ssa-strlen.cc | 2 +- > gcc/tree.cc | 11 ++++++----- > gcc/tree.h | 8 ++++---- > 11 files changed, 35 insertions(+), 37 deletions(-) > > diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc > index e190b93aa85..fbf448e045d 100644 > --- a/gcc/gimple-array-bounds.cc > +++ b/gcc/gimple-array-bounds.cc > @@ -129,7 +129,7 @@ get_ref_size (tree arg, tree *pref) > } > > /* Return true if REF is (likely) an ARRAY_REF to a trailing array member > - of a struct. It refines array_at_struct_end_p by detecting a pointer > + of a struct. It refines array_ref_flexible_size_p by detecting a pointer > to an array and an array parameter declared using the [N] syntax (as > opposed to a pointer) and returning false. Set *PREF to the decl or > expression REF refers to. */ > @@ -167,7 +167,7 @@ trailing_array (tree arg, tree *pref) > return false; > } > > - return array_at_struct_end_p (arg); > + return array_ref_flexible_size_p (arg); > } > > /* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc > index 9055cd8982d..cafd331ca98 100644 > --- a/gcc/gimple-fold.cc > +++ b/gcc/gimple-fold.cc > @@ -1690,13 +1690,11 @@ get_range_strlen_tree (tree arg, bitmap visited, strlen_range_kind rkind, > /* Handle a MEM_REF into a DECL accessing an array of integers, > being conservative about references to extern structures with > flexible array members that can be initialized to arbitrary > - numbers of elements as an extension (static structs are okay). > - FIXME: Make this less conservative -- see > - component_ref_size in tree.cc. */ > + numbers of elements as an extension (static structs are okay). */ > tree ref = TREE_OPERAND (TREE_OPERAND (arg, 0), 0); > if ((TREE_CODE (ref) == PARM_DECL || VAR_P (ref)) > && (decl_binds_to_current_def_p (ref) > - || !array_at_struct_end_p (arg))) > + || !array_ref_flexible_size_p (arg))) > { > /* Fail if the offset is out of bounds. Such accesses > should be diagnosed at some point. */ > diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc > index b7ed15c8902..832456ba6fc 100644 > --- a/gcc/gimple-ssa-warn-restrict.cc > +++ b/gcc/gimple-ssa-warn-restrict.cc > @@ -296,8 +296,9 @@ builtin_memref::builtin_memref (pointer_query &ptrqry, gimple *stmt, tree expr, > tree basetype = TREE_TYPE (base); > if (TREE_CODE (basetype) == ARRAY_TYPE) > { > - if (ref && array_at_struct_end_p (ref)) > - ; /* Use the maximum possible offset for last member arrays. */ > + if (ref && array_ref_flexible_size_p (ref)) > + ; /* Use the maximum possible offset for an array that might > + have flexible size. */ > else if (tree basesize = TYPE_SIZE_UNIT (basetype)) > if (TREE_CODE (basesize) == INTEGER_CST) > /* Size could be non-constant for a variable-length type such > diff --git a/gcc/graphite-sese-to-poly.cc b/gcc/graphite-sese-to-poly.cc > index 51ba3af204f..7eb24c1c991 100644 > --- a/gcc/graphite-sese-to-poly.cc > +++ b/gcc/graphite-sese-to-poly.cc > @@ -536,9 +536,9 @@ bounds_are_valid (tree ref, tree low, tree high) > || !tree_fits_shwi_p (high)) > return false; > > - /* 1-element arrays at end of structures may extend over > + /* An array that has flexible size may extend over > their declared size. */ > - if (array_at_struct_end_p (ref) > + if (array_ref_flexible_size_p (ref) > && operand_equal_p (low, high, 0)) > return false; > > diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc > index a83b013d2ad..34bb507ff3b 100644 > --- a/gcc/tree-if-conv.cc > +++ b/gcc/tree-if-conv.cc > @@ -763,10 +763,9 @@ idx_within_array_bound (tree ref, tree *idx, void *dta) > if (TREE_CODE (ref) != ARRAY_REF) > return false; > > - /* For arrays at the end of the structure, we are not guaranteed that they > - do not really extend over their declared size. However, for arrays of > - size greater than one, this is unlikely to be intended. */ > - if (array_at_struct_end_p (ref)) > + /* For arrays that might have flexible sizes, it is not guaranteed that they > + do not extend over their declared size. */ > + if (array_ref_flexible_size_p (ref)) > return false; > > ev = analyze_scalar_evolution (loop, *idx); > diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc > index 1f04cb80fd0..2e5d267d8ce 100644 > --- a/gcc/tree-object-size.cc > +++ b/gcc/tree-object-size.cc > @@ -633,7 +633,7 @@ addr_object_size (struct object_size_info *osi, const_tree ptr, > v = NULL_TREE; > break; > } > - is_flexible_array_mem_ref = array_at_struct_end_p (v); > + is_flexible_array_mem_ref = array_ref_flexible_size_p (v); > while (v != pt_var && TREE_CODE (v) == COMPONENT_REF) > if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0))) > != UNION_TYPE > diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc > index b4c65da5718..d3a91b1f238 100644 > --- a/gcc/tree-ssa-alias.cc > +++ b/gcc/tree-ssa-alias.cc > @@ -1073,7 +1073,7 @@ component_ref_to_zero_sized_trailing_array_p (tree ref) > && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 1))) == ARRAY_TYPE > && (!TYPE_SIZE (TREE_TYPE (TREE_OPERAND (ref, 1))) > || integer_zerop (TYPE_SIZE (TREE_TYPE (TREE_OPERAND (ref, 1))))) > - && array_at_struct_end_p (ref)); > + && array_ref_flexible_size_p (ref)); > } > > /* Worker for aliasing_component_refs_p. Most parameters match parameters of > @@ -3433,10 +3433,10 @@ stmt_kills_ref_p (gimple *stmt, ao_ref *ref) > } > /* Finally check if the lhs has the same address and size as the > base candidate of the access. Watch out if we have dropped > - an array-ref that was at struct end, this means ref->ref may > - be outside of the TYPE_SIZE of its base. */ > + an array-ref that might have flexible size, this means ref->ref > + may be outside of the TYPE_SIZE of its base. */ > if ((! innermost_dropped_array_ref > - || ! array_at_struct_end_p (innermost_dropped_array_ref)) > + || ! array_ref_flexible_size_p (innermost_dropped_array_ref)) > && (lhs == base > || (((TYPE_SIZE (TREE_TYPE (lhs)) > == TYPE_SIZE (TREE_TYPE (base))) > diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc > index 4ffcef4f4ff..3fbbf4367ed 100644 > --- a/gcc/tree-ssa-loop-niter.cc > +++ b/gcc/tree-ssa-loop-niter.cc > @@ -3716,18 +3716,17 @@ idx_infer_loop_bounds (tree base, tree *idx, void *dta) > struct ilb_data *data = (struct ilb_data *) dta; > tree ev, init, step; > tree low, high, type, next; > - bool sign, upper = true, at_end = false; > + bool sign, upper = true, has_flexible_size = false; > class loop *loop = data->loop; > > if (TREE_CODE (base) != ARRAY_REF) > return true; > > - /* For arrays at the end of the structure, we are not guaranteed that they > - do not really extend over their declared size. However, for arrays of > - size greater than one, this is unlikely to be intended. */ > - if (array_at_struct_end_p (base)) > + /* For arrays that might have flexible sizes, it is not guaranteed that they > + do not really extend over their declared size. */ > + if (array_ref_flexible_size_p (base)) > { > - at_end = true; > + has_flexible_size = true; > upper = false; > } > > @@ -3760,9 +3759,9 @@ idx_infer_loop_bounds (tree base, tree *idx, void *dta) > sign = tree_int_cst_sign_bit (step); > type = TREE_TYPE (step); > > - /* The array of length 1 at the end of a structure most likely extends > + /* The array that might have flexible size most likely extends > beyond its bounds. */ > - if (at_end > + if (has_flexible_size > && operand_equal_p (low, high, 0)) > return true; > > diff --git a/gcc/tree-ssa-strlen.cc b/gcc/tree-ssa-strlen.cc > index 5afbae1b72e..b87c7c7ce1f 100644 > --- a/gcc/tree-ssa-strlen.cc > +++ b/gcc/tree-ssa-strlen.cc > @@ -1987,7 +1987,7 @@ maybe_set_strlen_range (tree lhs, tree src, tree bound) > suggests if it's treated as a poor-man's flexible array member. */ > src = TREE_OPERAND (src, 0); > if (TREE_CODE (src) != MEM_REF > - && !array_at_struct_end_p (src)) > + && !array_ref_flexible_size_p (src)) > { > tree type = TREE_TYPE (src); > tree size = TYPE_SIZE_UNIT (type); > diff --git a/gcc/tree.cc b/gcc/tree.cc > index 04603c8c902..d2b0b34a725 100644 > --- a/gcc/tree.cc > +++ b/gcc/tree.cc > @@ -12710,8 +12710,8 @@ array_ref_up_bound (tree exp) > return NULL_TREE; > } > > -/* Returns true if REF is an array reference, component reference, > - or memory reference to an array whose actual size might be larger > +/* Returns true if REF is an array reference, a component reference, > + or a memory reference to an array whose actual size might be larger > than its upper bound implies, there are multiple cases: > A. a ref to a flexible array member at the end of a structure; > B. a ref to an array with a different type against the original decl; > @@ -12726,10 +12726,10 @@ array_ref_up_bound (tree exp) > int test (uint8_t *p, uint32_t t[1][1], int n) { > for (int i = 0; i < 4; i++, p++) > t[i][0] = ...; > +*/ > > - FIXME, the name of this routine need to be changed to be more accurate. */ > bool > -array_at_struct_end_p (tree ref) > +array_ref_flexible_size_p (tree ref) > { > /* the TYPE for this array referece. */ > tree atype = NULL_TREE; > @@ -12862,6 +12862,7 @@ array_at_struct_end_p (tree ref) > return afield_decl ? !DECL_NOT_FLEXARRAY (afield_decl) : true; > } > > + > /* Return a tree representing the offset, in bytes, of the field referenced > by EXP. This does not include any offset in DECL_FIELD_BIT_OFFSET. */ > > @@ -12957,7 +12958,7 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */) > return (tree_int_cst_equal (memsize, TYPE_SIZE_UNIT (memtype)) > ? memsize : NULL_TREE); > > - bool trailing = array_at_struct_end_p (ref); > + bool trailing = array_ref_flexible_size_p (ref); > bool zero_length = integer_zerop (memsize); > if (!trailing && !zero_length) > /* MEMBER is either an interior array or is an array with > diff --git a/gcc/tree.h b/gcc/tree.h > index a50f7b2be9d..0fcdd6b06d0 100644 > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -5548,10 +5548,10 @@ extern tree array_ref_up_bound (tree); > EXP, an ARRAY_REF or an ARRAY_RANGE_REF. */ > extern tree array_ref_low_bound (tree); > > -/* Returns true if REF is an array reference or a component reference > - to an array at the end of a structure. If this is the case, the array > - may be allocated larger than its upper bound implies. */ > -extern bool array_at_struct_end_p (tree); > +/* Returns true if REF is an array reference, a component reference, > + or a memory reference to an array whose actual size might be larger > + than its upper bound implies. */ > +extern bool array_ref_flexible_size_p (tree); > > /* Return a tree representing the offset, in bytes, of the field referenced > by EXP. This does not include any offset in DECL_FIELD_BIT_OFFSET. */ > -- 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)