On Sat, Dec 18, 2021 at 06:05:08PM +0530, Siddhesh Poyarekar wrote:
Sorry for the delay.
> +size_t
> +__attribute__ ((noinline))
> +test_builtin_calloc_condphi (size_t cnt, size_t sz, int cond)
> +{
> + struct
> + {
> + int a;
> + char b;
> + } bin[cnt];
> +
> + char *ch = __builtin_calloc (cnt, sz);
> + size_t ret = __builtin_dynamic_object_size (cond ? ch : (void *) &bin, 0);
> +
> + __builtin_free (ch);
> + return ret;
> +}
> +int
> +main (int argc, char **argv)
You don't use argc nor argv, just leave those out IMO.
> +{
> + if (test_builtin_malloc_condphi (1) != 32)
> + FAIL ();
> + if (test_builtin_malloc_condphi (0) != 64)
> + FAIL ();
You test the above with both possibilities.
> + if (test_builtin_calloc_condphi (128, 1, 0) == 128)
> + FAIL ();
But not this one, why? Also, it would be better to have
a != ... test rather than ==, if it is the VLA, then 128 * sizeof (struct { int
a; char b; })
?
> +/* Return true if VAL is represents an initial size for OBJECT_SIZE_TYPE. */
s/is //
> +
> +static inline bool
> +size_initval_p (tree val, int object_size_type)
> + phires = TREE_VEC_ELT (size, TREE_VEC_LENGTH (size) - 1);
> + gphi *phi = create_phi_node (phires, gimple_bb (stmt));
> + gphi *obj_phi = as_a <gphi *> (stmt);
Formatting, just one space before as_a.
> + /* Expand all size expressions to put their definitions close to the
> objects
> + for whom size is being computed. */
English is not my primary language, but shouldn't whom be used just
when talking about persons? So for which size instead?
>
> +static void
> +dynamic_object_size (struct object_size_info *osi, tree var,
> + tree *size, tree *wholesize)
Missing function comment.
> + if (i < num_args)
> + sizes = wholesizes = size_unknown (object_size_type);
Perhaps
ggc_free (sizes);
gcc_free (wholesizes);
here before the assignment?
> +
> + /* Point to the same TREE_VEC so that we can avoid emitting two PHI
> + nodes. */
> + if (!wholesize_needed)
and make this else if
> + wholesizes = sizes;
and ggc_free (wholesizes); before the assignment?
When it is very easy and provably correct that it will just be memory
to be GCed later...
Otherwise LGTM.
Jakub
