From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from earwig.elm.relay.mailchannels.net (earwig.elm.relay.mailchannels.net [23.83.212.54]) by sourceware.org (Postfix) with ESMTPS id 506803858C5F for ; Thu, 5 Oct 2023 19:31:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 506803858C5F 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 2851D840FEB; Thu, 5 Oct 2023 19:31:46 +0000 (UTC) Received: from pdx1-sub0-mail-a312.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 739E0840C6D; Thu, 5 Oct 2023 19:31:45 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1696534305; a=rsa-sha256; cv=none; b=QtNg8bHM/n9sp9aSwR4nJN3yGvBReoxTfk47uUu1aGPXX8vcF7mBwaLU8RzJPUEB8P5e9W 2k4JQRvyAgYQlAJdFwkvnKelqQYn8jOW/9OPNc4cP6PNj+FCp5dQjOOQerzSFMq+/v+pIu StN4wSOH3G+V0TLThArd1NDRQX6SD+1JnnjM/7X+y0l66TuuVqNPgiqXRG0zKBhTrZ4ka3 UQAI8pYeML91vBedgRtG5tm5XjLnyCukQrMrljwKAwPVQXiwQvlD5REXYJjb2LPvN6VDUy 3aeMi051kPPlU/Uzx72v/F6gOjYK+BCZUnKP16/l0xIe4nv4+7rZsBSuTv0Pzw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1696534305; 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=imhvMg54YW/NeRQ5TN5wpw1vdB24v0gJp+TzlFjI9HU=; b=oj9cJZEy7NPEDywz44oLs0OB+90Sg8JAS+kgPq05VQJCaFm34+ym10wKUNlyUOJN0oi/K6 sacipOdGhJpn8ufMYGBtOf6k8ilcbBseOmtJWjqUFzvPd1EXC0uOfGzdB3bgIfzDE1kYmp hiY8931BwZ9pMTsWCSYI27iNAfHceNc7qY7r3tpfFaEGQ4mnnT2mkKvRAs8ZqrEQubL2uI n77L/GiXfM3Xr4iSHVvNFbeXYztc1vznal/r6Hn38ueWazA1//mlGsNthFG8MoWFV62jIe my6/FDMmU4ruTc6Wodj08ZVsRDWdkWbdZjYumDLYrNPEL/m2Ea62x/zxWeWAuQ== ARC-Authentication-Results: i=1; rspamd-7d5dc8fd68-wnsqp; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Good X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Towering-Broad: 7f0c82317560ce94_1696534305777_1932550319 X-MC-Loop-Signature: 1696534305777:3479582300 X-MC-Ingress-Time: 1696534305776 Received: from pdx1-sub0-mail-a312.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.109.140.241 (trex/6.9.1); Thu, 05 Oct 2023 19:31:45 +0000 Received: from [192.168.2.12] (bras-vprn-toroon4834w-lp130-02-142-113-138-41.dsl.bell.ca [142.113.138.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a312.dreamhost.com (Postfix) with ESMTPSA id 4S1hWh3Z6Kz9B; Thu, 5 Oct 2023 12:31:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1696534305; bh=imhvMg54YW/NeRQ5TN5wpw1vdB24v0gJp+TzlFjI9HU=; h=Date:Subject:From:To:Cc:Content-Type:Content-Transfer-Encoding; b=awTl3cDHudP9lHbVwasMhCH4d3ITcYeT2MkejCIKrHTN2VDZNXnYKGBSsABAc7SjL HY7x7NjMEhbxpIEkUoOL6QUQsCxgO6s9Rnkf/0JYrAIVQQBMLG13h70NmInjrXkgZ5 1B50BgHQ5MlxU01WDzN0UpYGa2AxQWCHlc6bBFZw5zEI09zwCMt7pTQHjvwp2v7MsK t3F/sVlexcAtqnXc1+kgzwk7GNljRclG4aRjUej93In/BHcfIv3QA2GJagffCKmZJR SBdynh3M/teFQouE3OPO355U2yQf8KEgkcfnIlCgvuZFdwwolfAlSZAyaHk2c12bV1 bnOiHpz971JmQ== Message-ID: Date: Thu, 5 Oct 2023 15:31:43 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [V3][PATCH 1/3] Provide counted_by attribute to flexible array member field (PR108896) Content-Language: en-US From: Siddhesh Poyarekar To: Qing Zhao , joseph@codesourcery.com, richard.guenther@gmail.com, jakub@redhat.com, gcc-patches@gcc.gnu.org Cc: keescook@chromium.org, uecker@tugraz.at, isanbard@gmail.com References: <20230825152425.2417656-1-qing.zhao@oracle.com> <20230825152425.2417656-2-qing.zhao@oracle.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3037.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,URIBL_BLOCKED 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-10-05 14:51, Siddhesh Poyarekar wrote: > On 2023-08-25 11:24, Qing Zhao wrote: >> Provide a new counted_by attribute to flexible array member field. > > The obligatory "I can't ack the patch but here's a review" disclaimer :) > >> >> 'counted_by (COUNT)' >>       The 'counted_by' attribute may be attached to the flexible array >>       member of a structure.  It indicates that the number of the >>       elements of the array is given by the field named "COUNT" in the >>       same structure as the flexible array member.  GCC uses this >>       information to improve the results of the array bound sanitizer and >>       the '__builtin_dynamic_object_size'. >> >>       For instance, the following code: >> >>            struct P { >>              size_t count; >>              char other; >>              char array[] __attribute__ ((counted_by (count))); >>            } *p; >> >>       specifies that the 'array' is a flexible array member whose number >>       of elements is given by the field 'count' in the same structure. >> >>       The field that represents the number of the elements should have an >>       integer type.  An explicit 'counted_by' annotation defines a >>       relationship between two objects, 'p->array' and 'p->count', that >>       'p->array' has _at least_ 'p->count' number of elements available. >>       This relationship must hold even after any of these related objects >>       are updated.  It's the user's responsibility to make sure this >>       relationship to be kept all the time.  Otherwise the results of the >>       array bound sanitizer and the '__builtin_dynamic_object_size' might >>       be incorrect. >> >>       For instance, in the following example, the allocated array has >>       less elements than what's specified by the 'sbuf->count', this is >>       an user error.  As a result, out-of-bounds access to the array >>       might not be detected. >> >>            #define SIZE_BUMP 10 >>            struct P *sbuf; >>            void alloc_buf (size_t nelems) >>            { >>              sbuf = (struct P *) malloc (MAX (sizeof (struct P), >>                                             (offsetof (struct P, >> array[0]) >>                                              + nelems * sizeof (char)))); >>              sbuf->count = nelems + SIZE_BUMP; >>              /* This is invalid when the sbuf->array has less than >> sbuf->count >>                 elements.  */ >>            } >> >>       In the following example, the 2nd update to the field 'sbuf->count' >>       of the above structure will permit out-of-bounds access to the >>       array 'sbuf>array' as well. >> >>            #define SIZE_BUMP 10 >>            struct P *sbuf; >>            void alloc_buf (size_t nelems) >>            { >>              sbuf = (struct P *) malloc (MAX (sizeof (struct P), >>                                             (offsetof (struct P, >> array[0]) >>                                              + (nelems + SIZE_BUMP) * >> sizeof (char)))); >>              sbuf->count = nelems; >>              /* This is valid when the sbuf->array has at least >> sbuf->count >>                 elements.  */ >>            } >>            void use_buf (int index) >>            { >>              sbuf->count = sbuf->count + SIZE_BUMP + 1; >>              /* Now the value of sbuf->count is larger than the number >>                 of elements of sbuf->array.  */ >>              sbuf->array[index] = 0; >>              /* then the out-of-bound access to this array >>                 might not be detected.  */ >>            } >> >> gcc/c-family/ChangeLog: >> >>     PR C/108896 >>     * c-attribs.cc (handle_counted_by_attribute): New function. >>     (attribute_takes_identifier_p): Add counted_by attribute to the list. >>     * c-common.cc (c_flexible_array_member_type_p): ...To this. >>     * c-common.h (c_flexible_array_member_type_p): New prototype. >> >> gcc/c/ChangeLog: >> >>     PR C/108896 >>     * c-decl.cc (flexible_array_member_type_p): Renamed and moved to... >>     (add_flexible_array_elts_to_size): Use renamed function. >>     (is_flexible_array_member_p): Use renamed function. >>     (verify_counted_by_attribute): New function. >>     (finish_struct): Use renamed function and verify counted_by >>     attribute. >> >> gcc/ChangeLog: >> >>     PR C/108896 >>     * doc/extend.texi: Document attribute counted_by. >>     * tree.cc (get_named_field): New function. >>     * tree.h (get_named_field): New prototype. >> >> gcc/testsuite/ChangeLog: >> >>     PR C/108896 >>     * gcc.dg/flex-array-counted-by.c: New test. >> --- >>   gcc/c-family/c-attribs.cc                    | 54 ++++++++++++- >>   gcc/c-family/c-common.cc                     | 13 ++++ >>   gcc/c-family/c-common.h                      |  1 + >>   gcc/c/c-decl.cc                              | 79 +++++++++++++++----- >>   gcc/doc/extend.texi                          | 77 +++++++++++++++++++ >>   gcc/testsuite/gcc.dg/flex-array-counted-by.c | 40 ++++++++++ >>   gcc/tree.cc                                  | 40 ++++++++++ >>   gcc/tree.h                                   |  5 ++ >>   8 files changed, 291 insertions(+), 18 deletions(-) >>   create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by.c >> >> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc >> index e2792ca6898b..65e4f6639109 100644 >> --- a/gcc/c-family/c-attribs.cc >> +++ b/gcc/c-family/c-attribs.cc >> @@ -103,6 +103,8 @@ static tree handle_warn_if_not_aligned_attribute >> (tree *, tree, tree, >>                             int, bool *); >>   static tree handle_strict_flex_array_attribute (tree *, tree, tree, >>                            int, bool *); >> +static tree handle_counted_by_attribute (tree *, tree, tree, >> +                       int, bool *); >>   static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ; >>   static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ; >>   static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, >> bool *); >> @@ -373,6 +375,8 @@ const struct attribute_spec >> c_common_attribute_table[] = >>                     handle_warn_if_not_aligned_attribute, NULL }, >>     { "strict_flex_array",      1, 1, true, false, false, false, >>                     handle_strict_flex_array_attribute, NULL }, >> +  { "counted_by",          1, 1, true, false, false, false, >> +                  handle_counted_by_attribute, NULL }, >>     { "weak",                   0, 0, true,  false, false, false, >>                     handle_weak_attribute, NULL }, >>     { "noplt",                   0, 0, true,  false, false, false, >> @@ -601,7 +605,8 @@ attribute_takes_identifier_p (const_tree attr_id) >>     else if (!strcmp ("mode", spec->name) >>          || !strcmp ("format", spec->name) >>          || !strcmp ("cleanup", spec->name) >> -       || !strcmp ("access", spec->name)) >> +       || !strcmp ("access", spec->name) >> +       || !strcmp ("counted_by", spec->name)) >>       return true; >>     else >>       return targetm.attribute_takes_identifier_p (attr_id); >> @@ -2555,6 +2560,53 @@ handle_strict_flex_array_attribute (tree *node, >> tree name, >>     return NULL_TREE; >>   } >> +/* Handle a "counted_by" attribute; arguments as in >> +   struct attribute_spec.handler.  */ >> + >> +static tree >> +handle_counted_by_attribute (tree *node, tree name, >> +                 tree args, int ARG_UNUSED (flags), >> +                 bool *no_add_attrs) >> +{ >> +  tree decl = *node; >> +  tree argval = TREE_VALUE (args); >> + >> +  /* This attribute only applies to field decls of a structure.  */ >> +  if (TREE_CODE (decl) != FIELD_DECL) >> +    { >> +      error_at (DECL_SOURCE_LOCATION (decl), >> +        "%qE attribute may not be specified for non-field" >> +        " declaration %q+D", name, decl); >> +      *no_add_attrs = true; >> +    } > > Applies only to struct fields.  OK. > >> +  /* This attribute only applies to field with array type.  */ >> +  else if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE) >> +    { >> +      error_at (DECL_SOURCE_LOCATION (decl), >> +        "%qE attribute may not be specified for a non-array field", >> +        name); >> +      *no_add_attrs = true; >> +    } > > The struct field should also be an array.  OK. > >> +  /* This attribute only applies to a C99 flexible array member >> type.  */ >> +  else if (! c_flexible_array_member_type_p (TREE_TYPE (decl))) >> +    { >> +      error_at (DECL_SOURCE_LOCATION (decl), >> +        "%qE attribute may not be specified for a non" >> +        " flexible array member field", >> +        name); >> +      *no_add_attrs = true; >> +    } > > Additionally, the field should be a *flex* array.  OK.  Could this be > reworded to: > >   %qE attribute only applies to C99 flexible array members > > That would make it clear that the GNU extension flex arrays (i.e. any > last array member of a struct) don't support this attribute. > >> +  /* The argument should be an identifier.  */ >> +  else if (TREE_CODE (argval) != IDENTIFIER_NODE) >> +    { >> +      error_at (DECL_SOURCE_LOCATION (decl), >> +        "% argument not an identifier"); >> +      *no_add_attrs = true; >> +    } > > Argument should be an identifier, and the check for validity of the > identifier comes later in finish_struct.  OK. > >> + >> +  return NULL_TREE; >> +} >> + >>   /* Handle a "weak" attribute; arguments as in >>      struct attribute_spec.handler.  */ >> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc >> index 9fbaeb437a12..a18937245c2a 100644 >> --- a/gcc/c-family/c-common.cc >> +++ b/gcc/c-family/c-common.cc >> @@ -9521,6 +9521,19 @@ c_common_finalize_early_debug (void) >>         (*debug_hooks->early_global_decl) (cnode->decl); >>   } >> +/* Determine whether TYPE is a ISO C99 flexible array memeber type >> "[]".  */ >> +bool >> +c_flexible_array_member_type_p (const_tree type) >> +{ >> +  if (TREE_CODE (type) == ARRAY_TYPE >> +      && TYPE_SIZE (type) == NULL_TREE >> +      && TYPE_DOMAIN (type) != NULL_TREE >> +      && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) >> +    return true; >> + >> +  return false; >> +} >> + > > Hoist flexible_array_member_type_p to use more widely.  OK. > >>   /* Get the LEVEL of the strict_flex_array for the ARRAY_FIELD based >> on the >>      values of attribute strict_flex_array and the >> flag_strict_flex_arrays.  */ >>   unsigned int >> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h >> index 78fc5248ba68..c29bb429062b 100644 >> --- a/gcc/c-family/c-common.h >> +++ b/gcc/c-family/c-common.h >> @@ -909,6 +909,7 @@ extern tree fold_for_warn (tree); >>   extern tree c_common_get_narrower (tree, int *); >>   extern bool get_attribute_operand (tree, unsigned HOST_WIDE_INT *); >>   extern void c_common_finalize_early_debug (void); >> +extern bool c_flexible_array_member_type_p (const_tree); >>   extern unsigned int c_strict_flex_array_level_of (tree); >>   extern bool c_option_is_from_cpp_diagnostics (int); >> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc >> index 1f9eb44dbaa2..e943b49b5230 100644 >> --- a/gcc/c/c-decl.cc >> +++ b/gcc/c/c-decl.cc >> @@ -5173,19 +5173,6 @@ set_array_declarator_inner (struct c_declarator >> *decl, >>     return decl; >>   } >> -/* Determine whether TYPE is a ISO C99 flexible array memeber type >> "[]".  */ >> -static bool >> -flexible_array_member_type_p (const_tree type) >> -{ >> -  if (TREE_CODE (type) == ARRAY_TYPE >> -      && TYPE_SIZE (type) == NULL_TREE >> -      && TYPE_DOMAIN (type) != NULL_TREE >> -      && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) >> -    return true; >> - >> -  return false; >> -} >> - >>   /* Determine whether TYPE is a one-element array type "[1]".  */ >>   static bool >>   one_element_array_type_p (const_tree type) >> @@ -5222,7 +5209,7 @@ add_flexible_array_elts_to_size (tree decl, tree >> init) >>     elt = CONSTRUCTOR_ELTS (init)->last ().value; >>     type = TREE_TYPE (elt); >> -  if (flexible_array_member_type_p (type)) >> +  if (c_flexible_array_member_type_p (type)) >>       { >>         complete_array_type (&type, elt, false); >>         DECL_SIZE (decl) >> @@ -9094,7 +9081,7 @@ is_flexible_array_member_p (bool is_last_field, >>     bool is_zero_length_array = zero_length_array_type_p (TREE_TYPE (x)); >>     bool is_one_element_array = one_element_array_type_p (TREE_TYPE (x)); >> -  bool is_flexible_array = flexible_array_member_type_p (TREE_TYPE (x)); >> +  bool is_flexible_array = c_flexible_array_member_type_p (TREE_TYPE >> (x)); >>     unsigned int strict_flex_array_level = >> c_strict_flex_array_level_of (x); > > Simple refactoring.  OK. > >> @@ -9124,6 +9111,61 @@ is_flexible_array_member_p (bool is_last_field, >>     return false; >>   } >> +/* Verify the argument of the counted_by attribute of the flexible array > > Verify *that* the argument... > >> +   member FIELD_DECL is a valid field of the containing structure's >> fieldlist, >> +   FIELDLIST, Report error and remove this attribute when it's not.  */ >> +static void >> +verify_counted_by_attribute (tree fieldlist, tree field_decl) >> +{ >> +  tree attr_counted_by = lookup_attribute ("counted_by", >> +                       DECL_ATTRIBUTES (field_decl)); >> + >> +  if (!attr_counted_by) >> +    return; >> + >> +  /* If there is an counted_by attribute attached to the field, >> +     verify it.  */ >> + >> +  const char *fieldname >> +    = IDENTIFIER_POINTER (TREE_VALUE (TREE_VALUE (attr_counted_by))); >> + >> +  /* Verify the argument of the attrbute is a valid field of the > > s/attrbute/attribute/ > >> +     containing structure.  */ >> + >> +  tree counted_by_field = get_named_field (fieldlist, fieldname); >> + >> +  /* Error when the field is not found in the containing structure.  */ >> +  if (!counted_by_field) >> +    { >> +      error_at (DECL_SOURCE_LOCATION (field_decl), >> +        "%qE attribute argument not a field declaration" >> +        " in the same structure, ignore it", >> +        (get_attribute_name (attr_counted_by))); > > Probably someone with English as a first language would make a better > suggestion, but how about: > >   Argument specified in %qE attribute is not a field declaration in the >   same structure, ignoring it. > >> + >> +      DECL_ATTRIBUTES (field_decl) >> +    = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl)); >> +    } >> +  else >> +  /* Error when the field is not with an integer type.  */ > > Suggest: Flag an error when the field is not of an integer type. > >> +    { >> +      while (TREE_CHAIN (counted_by_field)) >> +    counted_by_field = TREE_CHAIN (counted_by_field); >> +      tree real_field = TREE_VALUE (counted_by_field); >> + >> +      if (TREE_CODE (TREE_TYPE (real_field)) != INTEGER_TYPE) >> +    { >> +      error_at (DECL_SOURCE_LOCATION (field_decl), >> +             "%qE attribute argument not a field declaration" >> +             " with integer type, ignore it", >> +             (get_attribute_name (attr_counted_by))); > > Suggest: > >   Argument specified in %qE attribute is not of an integer type, >   ignoring it. > >> + >> +      DECL_ATTRIBUTES (field_decl) >> +        = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl)); >> +    } >> +    } >> + >> +  return; I forgot to mention the redundant return here. >> +} >>   /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T. >>      LOC is the location of the RECORD_TYPE or UNION_TYPE's definition. >> @@ -9244,7 +9286,7 @@ finish_struct (location_t loc, tree t, tree >> fieldlist, tree attributes, >>       DECL_PACKED (x) = 1; >>         /* Detect flexible array member in an invalid context.  */ >> -      if (flexible_array_member_type_p (TREE_TYPE (x))) >> +      if (c_flexible_array_member_type_p (TREE_TYPE (x))) >>       { >>         if (TREE_CODE (t) == UNION_TYPE) >>           { >> @@ -9265,6 +9307,9 @@ finish_struct (location_t loc, tree t, tree >> fieldlist, tree attributes, >>               "members"); >>             TREE_TYPE (x) = error_mark_node; >>           } >> +      /* if there is a counted_by attribute attached to this field, >> +         verify it.  */ >> +      verify_counted_by_attribute (fieldlist, x); >>       } >>         if (pedantic && TREE_CODE (t) == RECORD_TYPE >> @@ -9279,7 +9324,7 @@ finish_struct (location_t loc, tree t, tree >> fieldlist, tree attributes, >>        when x is an array and is the last field.  */ >>         if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE) >>       TYPE_INCLUDES_FLEXARRAY (t) >> -      = is_last_field && flexible_array_member_type_p (TREE_TYPE (x)); >> +      = is_last_field && c_flexible_array_member_type_p (TREE_TYPE (x)); >>         /* Recursively set TYPE_INCLUDES_FLEXARRAY for the context of >> x, t >>        when x is an union or record and is the last field.  */ >>         else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))) >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >> index 97eaacf8a7ec..ea6240646936 100644 >> --- a/gcc/doc/extend.texi >> +++ b/gcc/doc/extend.texi >> @@ -7617,6 +7617,83 @@ When both the attribute and the option present >> at the same time, the level of >>   the strictness for the specific trailing array field is determined >> by the >>   attribute. >> +@cindex @code{counted_by} variable attribute >> +@item counted_by (@var{count}) >> +The @code{counted_by} attribute may be attached to the flexible array >> +member of a structure.  It indicates that the number of the elements >> of the >> +array is given by the field named "@var{count}" in the same structure >> as the >> +flexible array member.  GCC uses this information to improve the >> results of >> +the array bound sanitizer and the @code{__builtin_dynamic_object_size}. > > Maybe specify somehow that this only applies to C99 flexible arrays?  Like: > >   The @code{counted_by} attribute may be attached to the C99 flexible >   array member of a structure... > >> + >> +For instance, the following code: >> + >> +@smallexample >> +struct P @{ >> +  size_t count; >> +  char other; >> +  char array[] __attribute__ ((counted_by (count))); >> +@} *p; >> +@end smallexample >> + >> +@noindent >> +specifies that the @code{array} is a flexible array member whose >> number of >> +elements is given by the field @code{count} in the same structure. >> + >> +The field that represents the number of the elements should have an >> integer >> +type.  An explicit @code{counted_by} annotation defines a >> relationship between >> +two objects, @code{p->array} and @code{p->count}, that >> @code{p->array} has >> +@emph{at least} @code{p->count} number of elements available.  This >> relationship >> +must hold even after any of these related objects are updated.  It's >> the user's >> +responsibility to make sure this relationship to be kept all the time. >> +Otherwise the results of the array bound sanitizer and the >> +@code{__builtin_dynamic_object_size} might be incorrect. > > Suggest: > >   The field that represents the number of the elements must have an >   integer type.  An explicit @code{counted_by} annotation defines a >   relationship between two objects, @code{p->array} and @code{p->count}, >   that @code{p->array} has @emph{at least} @code{p->count} number of >   elements available.  The user is responsible to ensure that this >   relationship is consistently maintained during the lifetime of the >   object. Failure to do so may result in results of the array bound >   sanitizer and the @code{__builtin_dynamic_object_size} being >   undefined. > >> + >> +For instance, in the following example, the allocated array has less >> elements >> +than what's specified by the @code{sbuf->count}, this is an user >> error. As a >> +result, out-of-bounds access to the array might not be detected. >> + >> +@smallexample >> +#define SIZE_BUMP 10 >> +struct P *sbuf; >> +void alloc_buf (size_t nelems) >> +@{ >> +  sbuf = (struct P *) malloc (MAX (sizeof (struct P), >> +                   (offsetof (struct P, array[0]) >> +                    + nelems * sizeof (char)))); >> +  sbuf->count = nelems + SIZE_BUMP; >> +  /* This is invalid when the sbuf->array has less than sbuf->count >> +     elements.  */ >> +@} >> +@end smallexample >> + >> +In the following example, the 2nd update to the field >> @code{sbuf->count} of >> +the above structure will permit out-of-bounds access to the array >> +@code{sbuf>array} as well. >> + >> +@smallexample >> +#define SIZE_BUMP 10 >> +struct P *sbuf; >> +void alloc_buf (size_t nelems) >> +@{ >> +  sbuf = (struct P *) malloc (MAX (sizeof (struct P), >> +                   (offsetof (struct P, array[0]) >> +                    + (nelems + SIZE_BUMP) * sizeof (char)))); >> +  sbuf->count = nelems; >> +  /* This is valid when the sbuf->array has at least sbuf->count >> +     elements.  */ >> +@} >> +void use_buf (int index) >> +@{ >> +  sbuf->count = sbuf->count + SIZE_BUMP + 1; >> +  /* Now the value of sbuf->count is larger than the number >> +     of elements of sbuf->array.  */ >> +  sbuf->array[index] = 0; >> +  /* then the out-of-bound access to this array >> +     might not be detected.  */ >> +@} >> +@end smallexample >> + >> + > > I'm unsure if we should be demonstrating example code with undefined > behaviour.  Maybe the documentation is better off without it. > >>   @cindex @code{alloc_size} variable attribute >>   @item alloc_size (@var{position}) >>   @itemx alloc_size (@var{position-1}, @var{position-2}) >> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by.c >> b/gcc/testsuite/gcc.dg/flex-array-counted-by.c >> new file mode 100644 >> index 000000000000..f8ce9776bf86 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by.c >> @@ -0,0 +1,40 @@ >> +/* testing the correct usage of attribute counted_by.  */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2" } */ >> + >> +#include >> + >> +int size; >> +int x __attribute ((counted_by (size))); /* { dg-error "attribute may >> not be specified for non-field declaration" } */ >> + >> +struct trailing { >> +  int count; >> +  int field __attribute ((counted_by (count))); /* { dg-error >> "attribute may not be specified for a non-array field" } */ >> +}; >> + >> +struct trailing_1 { >> +  int count; >> +  int array_1[0] __attribute ((counted_by (count))); /* { dg-error >> "attribute may not be specified for a non flexible array member field" >> } */ >> +}; >> + >> +int count; >> +struct trailing_array_2 { >> +  int count; >> +  int array_2[] __attribute ((counted_by ("count"))); /* { dg-error >> "argument not an identifier" } */ >> +}; >> + >> +struct trailing_array_3 { >> +  int other; >> +  int array_3[] __attribute ((counted_by (L"count"))); /* { dg-error >> "argument not an identifier" } */ >> +}; >> + >> +struct trailing_array_4 { >> +  int other; >> +  int array_4[] __attribute ((counted_by (count))); /* { dg-error >> "attribute argument not a field declaration in the same structure, >> ignore it" } */ >> +}; >> + >> +int count; >> +struct trailing_array_5 { >> +  float count; >> +  int array_5[] __attribute ((counted_by (count))); /* { dg-error >> "attribute argument not a field declaration with integer type, ignore >> it" } */ >> +}; > > Tests look OK in principle, but may need updating for the error message. > >> diff --git a/gcc/tree.cc b/gcc/tree.cc >> index 420857b110c4..fcd36ae0cd74 100644 >> --- a/gcc/tree.cc >> +++ b/gcc/tree.cc >> @@ -12745,6 +12745,46 @@ array_ref_element_size (tree exp) >>       return SUBSTITUTE_PLACEHOLDER_IN_EXPR (TYPE_SIZE_UNIT >> (elmt_type), exp); >>   } >> +/* Given a field list, FIELDLIST, of a structure/union, return a >> TREE_LIST, >> +   with each TREE_VALUE a FIELD_DECL stepping down the chain to the >> FIELD >> +   whose name is FIELDNAME, which is the last TREE_VALUE of the list. >> +   return NULL_TREE if such field is not found.  Normally this list >> is of >> +   length one, but if the field is embedded with (nested) anonymous >> structures >> +   or unions, this list steps down the chain to the field.  */ >> +tree >> +get_named_field (tree fieldlist, const char *fieldname) >> +{ >> +  tree named_field = NULL_TREE; >> +  for (tree field = fieldlist; field; field = DECL_CHAIN (field)) >> +    { >> +      if (TREE_CODE (field) != FIELD_DECL) >> +    continue; >> +      if (DECL_NAME (field) != NULL) >> +    if (strcmp (IDENTIFIER_POINTER (DECL_NAME (field)), fieldname) == 0) >> +      { >> +        named_field = tree_cons (NULL_TREE, field, named_field); >> +        break; >> +      } >> +    else >> +      continue; >> +      /* if the field is an anonymous struct/union, we will check the >> nested >> +     fields inside it recursively.  */ >> +      else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))) >> +      if ((named_field = get_named_field (TYPE_FIELDS (TREE_TYPE >> (field)), >> +                          fieldname)) != NULL_TREE) >> +        { >> +          named_field = tree_cons (NULL_TREE, field, named_field); >> +          break; >> +        } >> +      else >> +        continue; >> +      else >> +    continue; >> +    } >> +  return named_field; >> +} > > Descending recursively into the field list of a struct to find an > identifier with a matching name.  OK. > >> + >> + >>   /* Return a tree representing the lower bound of the array mentioned in >>      EXP, an ARRAY_REF or an ARRAY_RANGE_REF.  */ >> diff --git a/gcc/tree.h b/gcc/tree.h >> index 4c04245e2b1b..4859becaa1e7 100644 >> --- a/gcc/tree.h >> +++ b/gcc/tree.h >> @@ -5619,6 +5619,11 @@ extern tree get_base_address (tree t); >>      of EXP, an ARRAY_REF or an ARRAY_RANGE_REF.  */ >>   extern tree array_ref_element_size (tree); >> +/* Given a field list, FIELDLIST, of a structure/union, return the >> FIELD whose >> +   name is FIELDNAME, return NULL_TREE if such field is not found. >> +   searching nested anonymous structure/union recursively.  */ >> +extern tree get_named_field (tree, const char *); >> + >>   /* Return a typenode for the "standard" C type with a given name.  */ >>   extern tree get_typenode_from_name (const char *); >