On Mon, Dec 18, 2023 at 9:35 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> The following testcase ICEs because we aren't careful enough with
> alloc_size attribute. We do check that such an argument exists
> (although wouldn't handle correctly functions with more than INT_MAX
> arguments), but didn't check that it is scalar integer, the ICE is
> trying to fold_convert a structure to sizetype.
>
> Given that the attribute can also appear on non-prototyped functions
> where the arguments aren't known, I don't see how the FE could diagnose
> that and because we already handle the case where argument doesn't exist,
> I think we should also verify the argument is scalar integer convertible
> to sizetype. Furthermore, given this is not just in diagnostics but
> used for code generation, I think it is better to punt on arguments with
> larger precision then sizetype, the upper bits are then truncated.
>
> The patch also fixes some formatting issues and avoids duplication of the
> fold_convert, plus removes unnecessary check for if (arg1 >= 0), that is
> always the case after if (arg1 < 0) return ...;
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2023-12-18 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/113013
> * tree-object-size.cc (alloc_object_size): Return size_unknown if
> corresponding argument(s) don't have integral type or have integral
> type with higher precision than sizetype. Don't check arg1 >= 0
> uselessly. Compare argument indexes against gimple_call_num_args
> in unsigned type rather than int. Formatting fixes.
>
> * gcc.dg/pr113013.c: New test.
>
> --- gcc/tree-object-size.cc.jj 2023-11-02 07:49:20.538817331 +0100
> +++ gcc/tree-object-size.cc 2023-12-15 14:18:13.229417305 +0100
> @@ -794,21 +794,33 @@ alloc_object_size (const gcall *call, in
> arg2 = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN (p)))-1;
> }
> else if (gimple_call_builtin_p (call, BUILT_IN_NORMAL)
> - && callfn && ALLOCA_FUNCTION_CODE_P (DECL_FUNCTION_CODE (callfn)))
> - arg1 = 0;
> + && callfn
> + && ALLOCA_FUNCTION_CODE_P (DECL_FUNCTION_CODE (callfn)))
> + arg1 = 0;
>
> /* Non-const arguments are OK here, let the caller handle constness. */
> - if (arg1 < 0 || arg1 >= (int) gimple_call_num_args (call)
> - || arg2 >= (int) gimple_call_num_args (call))
> + if (arg1 < 0
> + || (unsigned) arg1 >= gimple_call_num_args (call)
> + || (arg2 >= 0 && (unsigned) arg2 >= gimple_call_num_args (call)))
> return size_unknown (object_size_type);
>
> + tree targ1 = gimple_call_arg (call, arg1);
> + if (!INTEGRAL_TYPE_P (TREE_TYPE (targ1))
> + || TYPE_PRECISION (TREE_TYPE (targ1)) > TYPE_PRECISION (sizetype))
> + return size_unknown (object_size_type);
> + targ1 = fold_convert (sizetype, targ1);
> tree bytes = NULL_TREE;
> if (arg2 >= 0)
> - bytes = size_binop (MULT_EXPR,
> - fold_convert (sizetype, gimple_call_arg (call, arg1)),
> - fold_convert (sizetype, gimple_call_arg (call, arg2)));
> - else if (arg1 >= 0)
> - bytes = fold_convert (sizetype, gimple_call_arg (call, arg1));
> + {
> + tree targ2 = gimple_call_arg (call, arg2);
> + if (!INTEGRAL_TYPE_P (TREE_TYPE (targ2))
> + || TYPE_PRECISION (TREE_TYPE (targ2)) > TYPE_PRECISION (sizetype))
> + return size_unknown (object_size_type);
> + targ2 = fold_convert (sizetype, targ2);
> + bytes = size_binop (MULT_EXPR, targ1, targ2);
> + }
> + else
> + bytes = targ1;
>
> return bytes ? bytes : size_unknown (object_size_type);
> }
> --- gcc/testsuite/gcc.dg/pr113013.c.jj 2023-12-15 14:20:19.889631653 +0100
> +++ gcc/testsuite/gcc.dg/pr113013.c 2023-12-15 14:19:19.122488347 +0100
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/113013 */
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu99 -O2" } */
> +
> +struct S { short x; } s;
> +void *foo () __attribute__((__alloc_size__(1)));
> +struct S *p;
> +
> +__SIZE_TYPE__
> +bar (void)
> +{
> + p = foo (s);
> + return __builtin_dynamic_object_size (p, 0);
> +}
>
> Jakub
>
