diff mbox series

tree-object-size: Robustify alloc_size attribute handling [PR113013]

Message ID ZYAEH+Oi9+15lmzw@tucnak
State Unresolved
Headers
Series tree-object-size: Robustify alloc_size attribute handling [PR113013] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Dec. 18, 2023, 8:34 a.m. UTC 
  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?

2023-12-18  Jakub Jelinek  <jakub@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.


	Jakub

Comments

Richard Biener Dec. 18, 2023, 8:53 a.m. UTC | #1 
On Mon, Dec 18, 2023 at 9:35 AM Jakub Jelinek <jakub@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  <jakub@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
>
diff mbox series

Patch

--- 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);
+}