Hi, Sid,
> On Oct 15, 2025, at 18:01, Siddhesh Poyarekar <[email protected]> wrote:
>
> On 2025-06-25 10:07, Qing Zhao wrote:
>> Hi,
>> This is the 3rd version of the patch for:
>> Evaluate the object size by the size of the pointee type when the type
>> is a structure with flexible array member which is annotated with
>> counted_by.
>> Compared to the 2nd version of the patch at:
>> https://gcc.gnu.org/pipermail/gcc-patches/2025-May/682923.html
>> The major changes include:
>> A. Add a new --param objsz-allow-dereference-input=0|1 to control this
>> feature;
>> B. Some code reorg to the routine "insert_cond_and_size" to make it more
>> readable.
>> The patch has been bootstrapped and regression tested on both x86 and
>> aarch64.
>> Okay for trunk?
>
> Thanks for doing this, it's a strong step forward I think, but I have some
> design suggestions below. Apologies for dragging my feet on this :/
Thanks a lot for your review and suggestions, very helpful!
>
>> thanks.
>> Qing
>> ===========================================================================
>> In tree-object-size.cc, if the size is UNKNOWN after evaluating use-def
>> chain, We can evaluate the SIZE of the pointee TYPE ONLY when this TYPE
>> is a structure type with flexible array member which is attached a
>> counted_by attribute, since a structure with FAM can not be an element
>> of an array, so, the pointer must point to a single object with this
>> structure with FAM.
>> Control this behavior with a new --param objsz-allow-dereference-input=0|1
>> Default is 0.
>> This is only available for C now.
>> gcc/c/ChangeLog:
>> * c-lang.cc (LANG_HOOKS_BUILD_COUNTED_BY_REF):
>> Define to below function.
>> * c-tree.h (c_build_counted_by_ref): New extern function.
>> * c-typeck.cc (build_counted_by_ref): Rename to ...
>> (c_build_counted_by_ref): ...this.
>> (handle_counted_by_for_component_ref): Call the renamed function.
>> gcc/ChangeLog:
>> * doc/invoke.texi: Add documentation for the new option
>> --param objsz-allow-dereference-input.
>> * langhooks-def.h (LANG_HOOKS_BUILD_COUNTED_BY_REF):
>> New language hook.
>> * langhooks.h (struct lang_hooks_for_types): Add
>> build_counted_by_ref.
>> * params.opt: New param objsz-allow-dereference-input.
>> * tree-object-size.cc (struct object_size_info): Add a new field
>> insert_cf.
>> (insert_cond_and_size): New function.
>> (gimplify_size_expressions): Handle new field insert_cf.
>> (compute_builtin_object_size): Init the new field to false;
>> (is_pointee_fam_struct_with_counted_by): New function.
>> (record_with_fam_object_size): New function.
>> (collect_object_sizes_for): Call record_with_fam_object_size.
>> (dynamic_object_sizes_execute_one): Special handling for insert_cf.
>> gcc/testsuite/ChangeLog:
>> * gcc.dg/flex-array-counted-by-3.c: Update test for whole object size;
>> * gcc.dg/flex-array-counted-by-4.c: Likewise.
>> * gcc.dg/flex-array-counted-by-5.c: Likewise.
>> * gcc.dg/flex-array-counted-by-10.c: New test.
>> ---
>> gcc/c/c-lang.cc | 3 +
>> gcc/c/c-tree.h | 1 +
>> gcc/c/c-typeck.cc | 6 +-
>> gcc/doc/invoke.texi | 13 +
>> gcc/langhooks-def.h | 4 +-
>> gcc/langhooks.h | 5 +
>> gcc/params.opt | 4 +
>> .../gcc.dg/flex-array-counted-by-10.c | 41 +++
>> .../gcc.dg/flex-array-counted-by-3.c | 7 +-
>> .../gcc.dg/flex-array-counted-by-4.c | 36 ++-
>> .../gcc.dg/flex-array-counted-by-5.c | 6 +-
>> gcc/tree-object-size.cc | 305 +++++++++++++++++-
>> 12 files changed, 406 insertions(+), 25 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-10.c
>> diff --git a/gcc/c/c-lang.cc b/gcc/c/c-lang.cc
>> index c69077b2a93..e9ec9e6e64a 100644
>> --- a/gcc/c/c-lang.cc
>> +++ b/gcc/c/c-lang.cc
>> @@ -51,6 +51,9 @@ enum c_language_kind c_language = clk_c;
>> #undef LANG_HOOKS_GET_SARIF_SOURCE_LANGUAGE
>> #define LANG_HOOKS_GET_SARIF_SOURCE_LANGUAGE c_get_sarif_source_language
>> +#undef LANG_HOOKS_BUILD_COUNTED_BY_REF
>> +#define LANG_HOOKS_BUILD_COUNTED_BY_REF c_build_counted_by_ref
>> +
>> /* Each front end provides its own lang hook initializer. */
>> struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
>> index 364f51df58c..627791551b4 100644
>> --- a/gcc/c/c-tree.h
>> +++ b/gcc/c/c-tree.h
>> @@ -777,6 +777,7 @@ extern struct c_switch *c_switch_stack;
>> extern bool null_pointer_constant_p (const_tree);
>> +extern tree c_build_counted_by_ref (tree, tree, tree *);
>> inline bool
>> c_type_variably_modified_p (tree t)
>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>> index e24629be918..44031ca1ae3 100644
>> --- a/gcc/c/c-typeck.cc
>> +++ b/gcc/c/c-typeck.cc
>> @@ -2940,8 +2940,8 @@ should_suggest_deref_p (tree datum_type)
>> &(p->k)
>> */
>> -static tree
>> -build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>> +tree
>> +c_build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>> {
>> tree type = TREE_TYPE (datum);
>> if (!c_flexible_array_member_type_p (TREE_TYPE (subdatum)))
>> @@ -3039,7 +3039,7 @@ handle_counted_by_for_component_ref (location_t loc,
>> tree ref)
>> tree datum = TREE_OPERAND (ref, 0);
>> tree subdatum = TREE_OPERAND (ref, 1);
>> tree counted_by_type = NULL_TREE;
>> - tree counted_by_ref = build_counted_by_ref (datum, subdatum,
>> + tree counted_by_ref = c_build_counted_by_ref (datum, subdatum,
>> &counted_by_type);
>> if (counted_by_ref)
>> ref = build_access_with_size_for_counted_by (loc, ref,
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 693bd57691e..1d3277d5e71 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -17566,6 +17566,19 @@ Work bound when discovering transitive relations
>> from existing relations.
>> @item min-pagesize
>> Minimum page size for warning and early break vectorization purposes.
>> +@item objsz-allow-dereference-input
>> +Use this option to allow the size expressions generated by the builtin
>> +@code{__builtin_dynamic_object_size} to dereference its input pointer.
>> +This may allow the builtin function to get the size of an object when
>> +the size information is embedded in the object itself. For example,
>> +if a structure with a flexible array members at its end is annotated with
>> +the @code{counted_by} attribute, the size information of an object with
>> +such structure type is embedded in the object itself.
>
> Another newline here to make it a new paragraph?
Sure.
>
>> +Use this parameter with caution because in cases where a non-NULL input
>> +pointer is not known to be valid, e.g. when it points to memory that is
>> +either protected or freed, enabling this parameter may result in
>> dereferencing
>> +that invalid pointer, potentially introducing additional undefined behavior.
>> +
>> @item openacc-kernels
>> Specify mode of OpenACC `kernels' constructs handling.
>> With @option{--param=openacc-kernels=decompose}, OpenACC `kernels'
>> diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h
>> index 010aaedd9de..98f3d0027d9 100644
>> --- a/gcc/langhooks-def.h
>> +++ b/gcc/langhooks-def.h
>> @@ -224,6 +224,7 @@ extern tree lhd_unit_size_without_reusable_padding
>> (tree);
>> #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE lhd_type_dwarf_attribute
>> #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING
>> lhd_unit_size_without_reusable_padding
>> #define LANG_HOOKS_CLASSTYPE_AS_BASE hook_tree_const_tree_null
>> +#define LANG_HOOKS_BUILD_COUNTED_BY_REF NULL
>> #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \
>> LANG_HOOKS_MAKE_TYPE, \
>> @@ -251,7 +252,8 @@ extern tree lhd_unit_size_without_reusable_padding
>> (tree);
>> LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO, \
>> LANG_HOOKS_TYPE_DWARF_ATTRIBUTE, \
>> LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING, \
>> - LANG_HOOKS_CLASSTYPE_AS_BASE \
>> + LANG_HOOKS_CLASSTYPE_AS_BASE, \
>> + LANG_HOOKS_BUILD_COUNTED_BY_REF \
>> }
>> /* Declaration hooks. */
>> diff --git a/gcc/langhooks.h b/gcc/langhooks.h
>> index cb03c8348e3..4f750955920 100644
>> --- a/gcc/langhooks.h
>> +++ b/gcc/langhooks.h
>> @@ -190,6 +190,11 @@ struct lang_hooks_for_types
>> i.e., type without virtual base classes or tail padding. Returns
>> NULL_TREE otherwise. */
>> tree (*classtype_as_base) (const_tree);
>> +
>> + /* Build a REF to the object that represents the counted_by per the
>> attribute
>> + counted_by attached to the field. Otherwise return NULL_TREE. Set the
>> + TYPE of the counted_by field to the last parameter. */
>> + tree (*build_counted_by_ref) (tree, tree, tree *);
>> };
>> /* Language hooks related to decls and the symbol table. */
>> diff --git a/gcc/params.opt b/gcc/params.opt
>> index 31aa0bd5753..d48367ef9ea 100644
>> --- a/gcc/params.opt
>> +++ b/gcc/params.opt
>> @@ -840,6 +840,10 @@ The minimum probability of reaching a source block for
>> interblock speculative sc
>> Common Joined UInteger Var(param_min_vect_loop_bound) Param Optimization
>> If -ftree-vectorize is used, the minimal loop bound of a loop to be
>> considered for vectorization.
>> +-param=objsz-allow-dereference-input=
>> +Common Joined UInteger Var(param_objsz_allow_dereference_input)
>> IntegerRange(0, 1) Param
>> +Allow the size expression generated by the __builtin_dynamic_object_size
>> function to dereference its input pointer.
>> +
>> -param=openacc-kernels=
>> Common Joined Enum(openacc_kernels) Var(param_openacc_kernels)
>> Init(OPENACC_KERNELS_PARLOOPS) Param
>> --param=openacc-kernels=[decompose|parloops] Specify mode of OpenACC
>> 'kernels' constructs handling.
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-10.c
>> b/gcc/testsuite/gcc.dg/flex-array-counted-by-10.c
>> new file mode 100644
>> index 00000000000..d46874fc816
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-10.c
>> @@ -0,0 +1,41 @@
>> +/* Test the attribute counted_by and its usage in
>> + __builtin_dynamic_object_size for whole object: when the pointer to the
>> whole
>> + object is NULL. */
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 --param objsz-allow-dereference-input=1" } */
>> +
>> +#include "builtin-object-size-common.h"
>> +struct annotated {
>> + int count;
>> + char array[] __attribute__((counted_by (count)));
>> +};
>> +
>> +static int __attribute__((__noinline__,__noipa__)) size_of (struct
>> annotated * obj)
>> +{
>> + return __builtin_dynamic_object_size (obj, 0);
>> +}
>> +
>> +static int __attribute__((__noinline__,__noipa__)) size_of_1 (struct
>> annotated * obj)
>> +{
>> + return __builtin_dynamic_object_size (obj, 1);
>> +}
>> +
>> +static int __attribute__((__noinline__,__noipa__)) size_of_2 (struct
>> annotated * obj)
>> +{
>> + return __builtin_dynamic_object_size (obj, 2);
>> +}
>> +
>> +static int __attribute__((__noinline__,__noipa__)) size_of_3 (struct
>> annotated * obj)
>> +{
>> + return __builtin_dynamic_object_size (obj, 3);
>> +}
>> +
>> +int main()
>> +{
>> + EXPECT (size_of (0), -1);
>> + EXPECT (size_of_1 (0), -1);
>> + EXPECT (size_of_2 (0), 0);
>> + EXPECT (size_of_3 (0), 0);
>> + return 0;
>> +}
>> +
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>> b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>> index 78f50230e89..dba1ca646b8 100644
>> --- a/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>> @@ -1,7 +1,7 @@
>> /* Test the attribute counted_by and its usage in
>> * __builtin_dynamic_object_size. */
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 --param objsz-allow-dereference-input=1" } */
>> #include "builtin-object-size-common.h"
>> @@ -53,6 +53,11 @@ void __attribute__((__noinline__)) test ()
>> array_annotated->b * sizeof (int));
>> EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1),
>> array_nested_annotated->b * sizeof (int));
>> + EXPECT(__builtin_dynamic_object_size(array_annotated, 1),
>> + sizeof (struct annotated) + array_annotated->b * sizeof (int));
>> + EXPECT(__builtin_dynamic_object_size(array_nested_annotated, 1),
>> + sizeof (struct nested_annotated)
>> + + array_nested_annotated->b * sizeof (int));
>> }
>> int main(int argc, char *argv[])
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>> b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>> index 20103d58ef5..03bfcfd26eb 100644
>> --- a/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>> @@ -4,7 +4,7 @@ allocation size mismatched with the value of counted_by
>> attribute?
>> We should always use the latest value that is hold by the counted_by
>> field. */
>> /* { dg-do run } */
>> -/* { dg-options "-O -fstrict-flex-arrays=3" } */
>> +/* { dg-options "-O -fstrict-flex-arrays=3 --param
>> objsz-allow-dereference-input=1" } */
>> #include "builtin-object-size-common.h"
>> @@ -23,7 +23,15 @@ struct annotated {
>> So, __builtin_object_size can not directly use the type of the pointee
>> to decide the size of the object the pointer points to.
>> - There are only two reliable ways:
>> + However, if the pointer points to a strucure with FAM, and the FAM is
>> + annotated with counted_by attribute, we can get the size of the pointee
>> + object by the size of the structure type and the counted_by attribute
>> + since structure with FAM cannot be an element of an array, the pointer
>> + that points to such type must point to a single object with the type,
>> + therefore, we can decide the size of the object from the size of the
>> + type for the pointee.
>> +
>> + There are other two reliable ways:
>> A. observed allocations (call to the allocation functions in the
>> routine)
>> B. observed accesses (read or write access to the location of the
>> pointer points to)
>> @@ -155,11 +163,13 @@ int main ()
>> EXPECT(__builtin_dynamic_object_size(p->array, 2), p->foo * sizeof(char));
>> EXPECT(__builtin_dynamic_object_size(p->array, 3), p->foo * sizeof(char));
>> /* When checking the pointer p, we have no observed allocation nor
>> observed
>> - access, therefore, we cannot determine the size info here. */
>> - EXPECT(__builtin_dynamic_object_size(p, 0), -1);
>> - EXPECT(__builtin_dynamic_object_size(p, 1), -1);
>> - EXPECT(__builtin_dynamic_object_size(p, 2), 0);
>> - EXPECT(__builtin_dynamic_object_size(p, 3), 0);
>> + access, however, since the pointer points to a strucure with FAM, and
>> the
>> + FAM is annotated with counted_by attribute, we can get the size of the
>> + pointee object by the size of the TYPE and the counted_by attribute. */
>> + EXPECT(__builtin_dynamic_object_size(p, 0), 19);
>> + EXPECT(__builtin_dynamic_object_size(p, 1), 19);
>> + EXPECT(__builtin_dynamic_object_size(p, 2), 19);
>> + EXPECT(__builtin_dynamic_object_size(p, 3), 19);
>> /* When checking the access p->array, we only have info on the
>> counted-by
>> value. */
>> @@ -168,11 +178,13 @@ int main ()
>> EXPECT(__builtin_dynamic_object_size(q->array, 2), q->foo * sizeof(char));
>> EXPECT(__builtin_dynamic_object_size(q->array, 3), q->foo * sizeof(char));
>> /* When checking the pointer p, we have no observed allocation nor
>> observed
>> - access, therefore, we cannot determine the size info here. */
>> - EXPECT(__builtin_dynamic_object_size(q, 0), -1);
>> - EXPECT(__builtin_dynamic_object_size(q, 1), -1);
>> - EXPECT(__builtin_dynamic_object_size(q, 2), 0);
>> - EXPECT(__builtin_dynamic_object_size(q, 3), 0);
>> + access, however, since the pointer points to a strucure with FAM, and
>> the
>> + FAM is annotated with counted_by attribute, we can get the size of the
>> + pointee object by the size of the TYPE and the counted_by attribute. */
>> + EXPECT(__builtin_dynamic_object_size(q, 0), 29);
>> + EXPECT(__builtin_dynamic_object_size(q, 1), 29);
>> + EXPECT(__builtin_dynamic_object_size(q, 2), 29);
>> + EXPECT(__builtin_dynamic_object_size(q, 3), 29);
>> DONE ();
>> }
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> index 68f9b0f7c8d..89e1857206f 100644
>> --- a/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> @@ -1,7 +1,7 @@
>> /* Test the attribute counted_by and its usage in
>> * __builtin_dynamic_object_size: when the counted_by field is negative. */
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 --param objsz-allow-dereference-input=1" } */
>> #include "builtin-object-size-common.h"
>> @@ -36,6 +36,10 @@ void __attribute__((__noinline__)) setup (int attr_count)
>> void __attribute__((__noinline__)) test ()
>> {
>> + EXPECT(__builtin_dynamic_object_size(array_annotated, 1),
>> + sizeof (struct annotated));
>> + EXPECT(__builtin_dynamic_object_size(array_nested_annotated, 1),
>> + sizeof (struct nested_annotated));
>> EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1), 0);
>> EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1), 0);
>> }
>
> Please also add tests (or run these same tests) without --param
> objsz-allow-dereference-input=1 and also with --param
> objsz-allow-dereference-input=0 to make sure that they do the right thing.
Okay, will add.
>
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index f348673ae75..6220c16df21 100644
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/tree-object-size.cc
>> @@ -24,12 +24,15 @@ along with GCC; see the file COPYING3. If not see
>> #include "backend.h"
>> #include "tree.h"
>> #include "gimple.h"
>> +#include "cfghooks.h"
>> #include "tree-pass.h"
>> #include "ssa.h"
>> #include "gimple-pretty-print.h"
>> #include "fold-const.h"
>> +#include "cfgloop.h"
>> #include "tree-object-size.h"
>> #include "gimple-iterator.h"
>> +#include "langhooks.h"
>> #include "gimple-fold.h"
>> #include "tree-cfg.h"
>> #include "tree-dfa.h"
>> @@ -45,6 +48,7 @@ struct object_size_info
>> int object_size_type;
>> unsigned char pass;
>> bool changed;
>> + bool insert_cf;
>
> Instead of making this an OSI flag with a true/false value to support this
> specific change, how about adding a new member to the object_size struct
> called null_size? This makes sure that the idea of NULL-checked sizes more
> general purpose and allow future extension to it. I can see this being
> useful for, e.g. pr121786 and maybe more in future.
>
> You would need:
>
> 1. A new member null_size
> 2. Add a new optional boolean argument null_checked to object_sizes_set, set
> by default to false. When the argument is true, object_sizes_set should set
> null_size to size_unknown (object_size_type).
Yeah, this is a good idea, I will update the patch based on this to see how it
works.
>
>> bitmap visited, reexamine;
>> unsigned int *depths;
>> unsigned int *stack, *tos;
>> @@ -1086,6 +1090,153 @@ propagate_unknowns (object_size_info *osi, tree
>> expr, bitmap unknowns)
>> }
>> }
>> +/* Given a DEF_STMT, which is the DEF_STMT of the SSA_NAME whose
>> + object size is queried, and COND, THEN_SIZE, ELSE_SIZE, the final
>> + RESULT.
>> +
>> + Generate the gimple sequence for the following:
>> +
>> + result = COND ? THEN_SIZE : ELSE_SIZE;
>> +
>> + and then insert this new sequence in the proper position based on
>> + DEF_STMT:
>> +
>> + A. If DEF_STMT is valid, the generated gimple sequence should be
>> + inserted AFTER the DEF_STMT;
>> +
>> + B. If DEF_STMT is GIMPLE_NOP, i.e., no def is available, such as
>> + the parameter case. Under such situation, the new generated
>> + gimple sequence should be inserted in the very beginning of the
>> + current basic block.
>> +
>> + The following is the new generated IR if DEF_STMT is valid:
>> +
>> + OLD:
>> + cur_bb:
>> + DEF_STMT;
>> +
>> + NEW:
>> + cur_bb:
>> + DEF_STMT;
>> + if (COND) then goto then_bb
>> + else goto else_bb
>> +
>> + then_bb: (very likely)
>> + tmp_size_1 = THEN_SIZE;
>> + goto cur_bb_post;
>> +
>> + else_bb: (unlikely)
>> + tmp_size_2 = ELSE_SIZE;
>> + goto cur_bb_post;
>> +
>> + cur_bb_post:
>> + tmp_size_3 = PHI (tmp_size_1, tmp_size_2);
>> + result = tmp_size_3;
>> + */
>> +
>> +static void
>> +insert_cond_and_size (gimple *def_stmt, tree cond,
>> + tree then_size, tree else_size,
>> + tree result)
>> +{
>> + enum gimple_code code = gimple_code (def_stmt);
>> + basic_block cur_bb;
>> + edge e;
>> + gimple_stmt_iterator gsi;
>> + /* Split the cur_bb to cur_bb and cur_bb_post based on DEF_STMT.
>> + if DEF_STMT is NOT GIMPLE_NOP, split it after DEF_STMT, otherwise,
>> + split it after the label of the block. */
>> + if (code == GIMPLE_NOP)
>> + {
>> + cur_bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun));
>> + e = split_block (cur_bb, (gimple *) NULL);
>> + gsi = gsi_end_bb (cur_bb);
>> + }
>> + else
>> + {
>> + cur_bb = gimple_bb (def_stmt);
>> + e = split_block (cur_bb, def_stmt);
>> + gsi = gsi_for_stmt (def_stmt);
>> + }
>> +
>> + basic_block cur_bb_post = e->dest;
>> +
>> + /* Create new basic blocks then_bb and else_bb. */
>> + basic_block then_bb = create_empty_bb (cur_bb);
>> + basic_block else_bb = create_empty_bb (then_bb);
>> + add_bb_to_loop (then_bb, cur_bb->loop_father);
>> + add_bb_to_loop (else_bb, cur_bb->loop_father);
>> + loops_state_set (LOOPS_NEED_FIXUP);
>> +
>> + /* Set up the edges between cur_bb, then_bb, else_bb, and cur_bb_post.
>> + OLD:
>> + cur_bb
>> + |e
>> + V
>> + cur_bb_post
>> +
>> + NEW:
>> + cur_bb
>> + /e0 \e1
>> + V V
>> + then_bb else_bb
>> + \e2 /e3
>> + V
>> + cur_bb_post
>> + */
>> + cur_bb_post->count = e->count ();
>> + remove_edge (e);
>> + edge e0 = make_edge (cur_bb, then_bb, EDGE_TRUE_VALUE);
>> + e0->probability = profile_probability::very_likely ();
>> + then_bb->count = e0->count ();
>> + edge e1 = make_edge (cur_bb, else_bb, EDGE_FALSE_VALUE);
>> + e1->probability = profile_probability::very_unlikely ();
>> + else_bb->count = e1->count ();
>> + edge e2 = make_single_succ_edge (then_bb, cur_bb_post, EDGE_FALLTHRU);
>> + edge e3 = make_single_succ_edge (else_bb, cur_bb_post, EDGE_FALLTHRU);
>> +
>> + /* Update dominance info for the newly created blocks. */
>> + if (dom_info_available_p (CDI_DOMINATORS))
>> + {
>> + set_immediate_dominator (CDI_DOMINATORS, then_bb, cur_bb);
>> + set_immediate_dominator (CDI_DOMINATORS, else_bb, cur_bb);
>> + set_immediate_dominator (CDI_DOMINATORS, cur_bb_post, cur_bb);
>> + }
>> +
>> + /* Insert new gimples into corresponding blocks. */
>> +
>> + /* Insert the new COND gimple into cur_bb after GSI. */
>> + gcond *cond_stmt = gimple_build_cond_from_tree (cond, NULL_TREE,
>> NULL_TREE);
>> + gsi_insert_after (&gsi, cond_stmt, GSI_NEW_STMT);
>> +
>> + /* Insert the new assign gimple tmp_size_1 = THEN_SIZE into then_bb. */
>> + tree tmp_size = create_tmp_var (TREE_TYPE (result), "tmp_size");
>> + gimple_seq seq = NULL;
>> + tree tmp_size_1 = force_gimple_operand (then_size, &seq, true, tmp_size);
>> + gimple_stmt_iterator new_gsi = gsi_start_bb (then_bb);
>> + gsi_insert_seq_after (&new_gsi, seq, GSI_LAST_NEW_STMT);
>> +
>> + /* Insert the new assign gimple tmp_size_2 = ELSE_SIZE into else_bb. */
>> + seq = NULL;
>> + tree tmp_size_2 = force_gimple_operand (else_size, &seq, true, tmp_size);
>> + new_gsi = gsi_start_bb (else_bb);
>> + gsi_insert_seq_after (&new_gsi, seq, GSI_LAST_NEW_STMT);
>> +
>> + /* Insert the new phi gimple tmp_size_3 = phi (tmp_size_1, tmp_size_2)
>> + into cur_bb_post. */
>> + tree tmp_size_3 = make_ssa_name (tmp_size);
>> + gphi *phi = create_phi_node (tmp_size_3, cur_bb_post);
>> + add_phi_arg (phi, tmp_size_1, e2, UNKNOWN_LOCATION);
>> + add_phi_arg (phi, tmp_size_2, e3, UNKNOWN_LOCATION);
>> +
>> + /* Then insert a new assign gimple result = tmp_size_3 into cur_bb_post.
>> */
>> + gassign *assign_stmt = gimple_build_assign (result, tmp_size_3);
>> + new_gsi = gsi_start_bb (cur_bb_post);
>> + gsi_insert_before (&new_gsi, assign_stmt, GSI_NEW_STMT);
>> +
>> + return;
>> +}
>> +
>> /* Walk through size expressions that need reexamination and generate
>> statements for them. */
>> @@ -1167,14 +1318,30 @@ gimplify_size_expressions (object_size_info *osi)
>> if (size_expr)
>> {
>> - gimple_stmt_iterator gsi;
>> - if (code == GIMPLE_NOP)
>> - gsi = gsi_start_bb (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
>> + if (osi->insert_cf)
>
> This would then become something like:
>
> if (osize.null_size != NULL_TREE)
> {
> insert_null_checked (stmt, size_expr, osize.null_size);
> }
> else
> ...
>
> and then you clean up insert_cond_and_size above to specifically build an
> expression that bails out if PTR is NULL. I suppose insert_null_checked will
> need the pointer too then.
>
>> + {
>> + gcc_assert (TREE_CODE (size_expr) == MODIFY_EXPR);
>> + tree result = TREE_OPERAND (size_expr, 0);
>> + size_expr = TREE_OPERAND (size_expr, 1);
>> +
>> + gcc_assert (TREE_CODE (size_expr) == COND_EXPR);
>> + tree cond = COND_EXPR_COND (size_expr);
>> + tree then_size = COND_EXPR_THEN (size_expr);
>> + tree else_size = COND_EXPR_ELSE (size_expr);
>> + insert_cond_and_size (stmt, cond,
>> + then_size, else_size, result);
>> + }
>> else
>> - gsi = gsi_for_stmt (stmt);
>> -
>> - force_gimple_operand (size_expr, &seq, true, NULL);
>> - gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING);
>> + {
>> + gimple_stmt_iterator gsi;
>> + if (code == GIMPLE_NOP)
>> + gsi = gsi_start_bb (single_succ
>> + (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
>> + else
>> + gsi = gsi_for_stmt (stmt);
>> + force_gimple_operand (size_expr, &seq, true, NULL);
>> + gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING);
>> + }
>> }
>> }
>> @@ -1279,6 +1446,7 @@ compute_builtin_object_size (tree ptr, int
>> object_size_type,
>> expression needs to be gimplified. */
>> osi.pass = 0;
>> osi.changed = false;
>> + osi.insert_cf = false;
>> collect_object_sizes_for (&osi, ptr);
>> if (object_size_type & OST_DYNAMIC)
>> @@ -1403,6 +1571,109 @@ expr_object_size (struct object_size_info *osi, tree
>> ptr, tree value)
>> object_sizes_set (osi, varno, bytes, wholesize);
>> }
>> +/* Check whether the pointee type of the VAR is a structure with flexible
>> + array member attached a counted_by attribute. */
>> +
>> +static bool
>> +is_pointee_fam_struct_with_counted_by (tree var)
>> +{
>> + if (!POINTER_TYPE_P (TREE_TYPE (var)))
>> + return false;
>> +
>> + const_tree pointee_type = TREE_TYPE (TREE_TYPE (var));
>> + if (!flexible_array_type_p (pointee_type))
>> + return false;
>> +
>> + if (TREE_CODE (pointee_type) != RECORD_TYPE)
>> + return false;
>> +
>> + tree last = last_field (pointee_type);
>> + /* Check whether the last FAM field has a counted_by attribute. */
>> + if (last && lookup_attribute ("counted_by", DECL_ATTRIBUTES (last)))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +/* Compute an object size expression for VAR, whose pointee TYPE is a
>> + structure with flexible array member. */
>> +
>> +static void
>> +record_with_fam_object_size (struct object_size_info *osi, tree var)
>> +{
>> + int object_size_type = osi->object_size_type;
>> + tree size = size_unknown (object_size_type);
>> + gcc_assert (is_pointee_fam_struct_with_counted_by (var));
>> +
>> + tree pointee_type = TREE_TYPE (TREE_TYPE (var));
>> + tree counted_by_ref = NULL_TREE;
>> + tree counted_by_type = NULL_TREE;
>> + tree last = last_field (pointee_type);
>> +
>> + /* build a counted_by reference. */
>> + if (lang_hooks.types.build_counted_by_ref)
>> + {
>> + tree datum = build1 (INDIRECT_REF, pointee_type, var);
>> + counted_by_ref
>> + = lang_hooks.types.build_counted_by_ref (datum, last,
>> + &counted_by_type);
>> + }
>> +
>> + /* If the counted_by reference is available, the size of the whole
>> structure
>> + can be computed. */
>> + if (counted_by_ref)
>> + {
>> + tree element_type = TREE_TYPE (TREE_TYPE (last));
>> + tree element_size = TYPE_SIZE_UNIT (element_type);
>> + size = fold_build2 (MEM_REF, counted_by_type, counted_by_ref,
>> + build_int_cst (ptr_type_node, 0));
>> + /* If counted_by is a negative value, treat it as zero. */
>> + if (!TYPE_UNSIGNED (counted_by_type))
>> + {
>> + tree cond_expr = fold_build2 (LT_EXPR, boolean_type_node,
>> + unshare_expr (size),
>> + build_zero_cst (counted_by_type));
>> + size = fold_build3 (COND_EXPR, integer_type_node, cond_expr,
>> + build_zero_cst (counted_by_type), size);
>> + }
>
> Instead of a COND_EXPR, how about building a MAX(0, size) when size is signed?
Thanks, this might be simpler.
>
>> +
>> + /* The total size of the whole object is computed as:
>> + MAX (sizeof (pointee_type),
>> + offsetof (pointee_type, last) + counted_by * element_size). */
>> + size = size_binop (MULT_EXPR,
>> + fold_convert (sizetype, size),
>> + fold_convert (sizetype, element_size));
>> + size = size_binop (PLUS_EXPR,
>> + byte_position (last),
>> + size);
>> + size = size_binop (MAX_EXPR,
>> + TYPE_SIZE_UNIT (pointee_type),
>> + size);
>> +
>> + /* We should guard the size expression with the check to see whether
>> the
>> + original pointer is NULL or not since a NULL pointer might be passed
>> + and this is valid. */
>> + tree cond_expr = fold_build2 (EQ_EXPR, boolean_type_node, var,
>> + build_zero_cst (TREE_TYPE (var)));
>> + size = fold_build3 (COND_EXPR, integer_type_node, cond_expr,
>> + size_unknown (object_size_type), size);
>> + size = fold_convert (sizetype, size);
>
> With the new null_size, this is then not needed…
Right.
>
>> +
>> + if (!todo)
>> + todo = TODO_update_ssa | TODO_cleanup_cfg;
>> + }
>> + /* Initialize to 0 for maximum size and M1U for minimum size so that
>> + it gets immediately overridden. */
>> + object_sizes_initialize (osi, SSA_NAME_VERSION (var),
>> + size_initval (object_size_type),
>> + size_initval (object_size_type));
>> +
>> + /* The size expression should be put into a new block in the new added
>> + conditional control flow. */
>> + osi->insert_cf = true;
>> + object_sizes_set (osi, SSA_NAME_VERSION (var), size, size);
>
> ... and you can just call
> object_sizes_set (osi, SSA_NAME_VERSION (var), size, size, true);
>
> to let gimplify_size_expressions generate the NULL check.
Yeah.
>
>> + return;
>> +}
>> /* Compute object_sizes for PTR, defined to the result of a call. */
>> @@ -1922,6 +2193,19 @@ collect_object_sizes_for (struct object_size_info
>> *osi, tree var)
>> gcc_unreachable ();
>> }
>> + /* If the size is UNKNOWN after evaluating use-def chain, We can evaluate
>> + the SIZE of the pointee TYPE ONLY when this TYPE is a structure type
>> + with flexible array member that is attached a counted_by attribute,
>> + Since a structure with FAM can not be an element of an array. So,
>> + PTR must point to an single object with this strucure with FAM.
>
> Suggest: Since a structure with FAM cannot be an element of an array, PTR
> must point to at most a single object.
Sure.
>
>> + Control this with param_objsz_allow_dereference_input since doing this
>> + might add additional undefined behavior to the original source code.
>> */
>> + if (object_sizes_unknown_p (object_size_type, varno)
>> + && object_size_type & OST_DYNAMIC
>> + && param_objsz_allow_dereference_input
>> + && is_pointee_fam_struct_with_counted_by (var))
>> + record_with_fam_object_size (osi, var);
>> +
>> /* Dynamic sizes use placeholder temps to return an answer, so it is
>> always
>> safe to set COMPUTED for them. */
>> if ((object_size_type & OST_DYNAMIC)
>> @@ -2188,6 +2472,13 @@ dynamic_object_sizes_execute_one
>> (gimple_stmt_iterator *i, gimple *call)
>> /* fold_builtin_call_array may wrap the result inside a
>> NOP_EXPR. */
>> STRIP_NOPS (result);
>> +
>> + /* when insert_cf is true, the cfg is changed, and the ogiginal basic
>> block
>> + * associted with the iterator i might be different than the new basic
>> + * block associated with the corresponding stmt. */
>> + if (gsi_stmt (*i)->bb != gsi_bb (*i))
>> + i->bb = gsi_stmt (*i)->bb;
>> +
>
> Updates the bb of the iterator so that we can continue iterating safely.
> Seems legit, but I'm not 100% confident about it.
Right now, due to the CFG change after adding the conditions and “then”/“else”
blocks, the associated basic block with iterator “I”
Is changed, but not automatically, I am not sure whether there is any other way
to fix this issue more elegantly?
Thanks.
Qing
>
>> gimplify_and_update_call_from_tree (i, result);
>> if (dump_file && (dump_flags & TDF_DETAILS))
>
> Thanks,
> Sid
