diff mbox series

[v2,4/4] tree-object-size: Fall back to wholesize for non-const offset

Message ID 20240920164029.63843-5-siddhesh@gotplt.org
State New
Headers
Series tree-object-size: Improved offset handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Siddhesh Poyarekar Sept. 20, 2024, 4:40 p.m. UTC 
  Don't bail out early if the offset to a pointer in __builtin_object_size
is a variable, return the wholesize instead since that is a better
fallback for maximum estimate.  This should keep checks in place for
fortified functions to constrain overflows to at lesat some extent.

gcc/ChangeLog:

	PR middle-end/77608
	* tree-object-size.cc (plus_stmt_object_size): Drop check for
	constant offset.
	* testsuite/gcc.dg/builtin-object-size-1.c (test14): New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 gcc/testsuite/gcc.dg/builtin-object-size-1.c | 19 +++++++++++++++++++
 gcc/tree-object-size.cc                      |  7 ++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

Comments

Jakub Jelinek Sept. 27, 2024, 11:27 a.m. UTC | #1 
On Fri, Sep 20, 2024 at 12:40:29PM -0400, Siddhesh Poyarekar wrote:
> Don't bail out early if the offset to a pointer in __builtin_object_size
> is a variable, return the wholesize instead since that is a better
> fallback for maximum estimate.  This should keep checks in place for
> fortified functions to constrain overflows to at lesat some extent.
> 
> gcc/ChangeLog:
> 
> 	PR middle-end/77608
> 	* tree-object-size.cc (plus_stmt_object_size): Drop check for
> 	constant offset.
> 	* testsuite/gcc.dg/builtin-object-size-1.c (test14): New test.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
>  gcc/testsuite/gcc.dg/builtin-object-size-1.c | 19 +++++++++++++++++++
>  gcc/tree-object-size.cc                      |  7 ++++---
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> index 6ffe12be683..5a24087ae1e 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> @@ -791,6 +791,25 @@ test13 (unsigned cond)
>  #endif
>  }
>  
> +void
> +__attribute__ ((noinline))
> +test14 (unsigned off)
> +{
> +  char *buf2 = malloc (10);
> +  char *p;
> +  size_t t;
> +
> +  p = &buf2[off];
> +
> +#ifdef __builtin_object_size
> +  if (__builtin_object_size (p, 0) != 10 - off)
> +    FAIL ();
> +#else
> +  if (__builtin_object_size (p, 0) != 10)
> +    FAIL ();
> +#endif
> +}
> +
>  int
>  main (void)
>  {
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 1b569c3d12b..ebd2a4650aa 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -1595,8 +1595,7 @@ plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
>      op1 = try_collapsing_offset (op1, NULL_TREE, NOP_EXPR, object_size_type);
>  
>    /* Handle PTR + OFFSET here.  */
> -  if (size_valid_p (op1, object_size_type)
> -      && (TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))
> +  if ((TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))
>      {
>        if (TREE_CODE (op0) == SSA_NAME)
>  	{
> @@ -1621,7 +1620,9 @@ plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
>        if (size_unknown_p (bytes, 0))
>  	;
>        else if ((object_size_type & OST_DYNAMIC)
> -	       || bytes != wholesize || compare_tree_int (op1, offset_limit) <= 0)
> +	       || bytes != wholesize
> +	       || (size_valid_p (op1, object_size_type)
> +		   && compare_tree_int (op1, offset_limit) <= 0))
>  	bytes = size_for_offset (bytes, op1, wholesize);
>        /* In the static case, with a negative offset, the best estimate for
>  	 minimum size is size_unknown but for maximum size, the wholesize is a

LGTM.

	Jakub
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
index 6ffe12be683..5a24087ae1e 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
@@ -791,6 +791,25 @@  test13 (unsigned cond)
 #endif
 }
 
+void
+__attribute__ ((noinline))
+test14 (unsigned off)
+{
+  char *buf2 = malloc (10);
+  char *p;
+  size_t t;
+
+  p = &buf2[off];
+
+#ifdef __builtin_object_size
+  if (__builtin_object_size (p, 0) != 10 - off)
+    FAIL ();
+#else
+  if (__builtin_object_size (p, 0) != 10)
+    FAIL ();
+#endif
+}
+
 int
 main (void)
 {
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 1b569c3d12b..ebd2a4650aa 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -1595,8 +1595,7 @@  plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
     op1 = try_collapsing_offset (op1, NULL_TREE, NOP_EXPR, object_size_type);
 
   /* Handle PTR + OFFSET here.  */
-  if (size_valid_p (op1, object_size_type)
-      && (TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))
+  if ((TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))
     {
       if (TREE_CODE (op0) == SSA_NAME)
 	{
@@ -1621,7 +1620,9 @@  plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
       if (size_unknown_p (bytes, 0))
 	;
       else if ((object_size_type & OST_DYNAMIC)
-	       || bytes != wholesize || compare_tree_int (op1, offset_limit) <= 0)
+	       || bytes != wholesize
+	       || (size_valid_p (op1, object_size_type)
+		   && compare_tree_int (op1, offset_limit) <= 0))
 	bytes = size_for_offset (bytes, op1, wholesize);
       /* In the static case, with a negative offset, the best estimate for
 	 minimum size is size_unknown but for maximum size, the wholesize is a