public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix array size verification (PR c++/89507)
@ 2019-02-26 20:59 Jakub Jelinek
  2019-02-26 23:12 ` Jason Merrill
  2019-02-27  9:27 ` Richard Biener
  0 siblings, 2 replies; 3+ messages in thread
From: Jakub Jelinek @ 2019-02-26 20:59 UTC (permalink / raw)
  To: Richard Biener, Jeff Law, Jason Merrill; +Cc: gcc-patches

Hi!

Seems valid_constant_size_p has been written with the expectation that only
sizetype/ssizetype constants will be passed to it, otherwise it couldn't
ever just blindly test tree_int_cst_sign_bit (size) for unsigned
INTEGER_CSTs and complain cst_size_too_big.
Unfortunately a recent patch started using this function even on other
types, and the comment explicitly talk about it being done on
pre-conversion to sizetype:
      /* The expression in a noptr-new-declarator is erroneous if it's of
         non-class type and its value before converting to std::size_t is
         less than zero. ... If the expression is a constant expression,
         the program is ill-fomed.  */
      if (TREE_CODE (cst_nelts) == INTEGER_CST
          && !valid_array_size_p (input_location, cst_nelts, NULL_TREE,
                                  complain & tf_error))
        return error_mark_node;
E.g. __int128 negative value could fit just fine after cast to sizetype,
etc.

So, instead of changing the C++ FE to only complain about negative cst_elts
normally and fold_convert everything to sizetype before checking, this patch
attempts to deal with non-{,s}sizetype constants.  Negative (signed)
constants are always rejected as before, newly constants that don't fit into
uhwi are rejected after that check regardless of signedness and anything
larger or equal than SIZE_MAX / 2 is also rejected as too big.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-02-26  Jakub Jelinek  <jakub@redhat.com>

	PR c++/89507
	* tree.c (valid_constant_size_p): Deal with size INTEGER_CSTs
	with types other than sizetype/ssizetype.

	* g++.dg/other/new2.C: New test.

--- gcc/tree.c.jj	2019-02-18 20:48:35.745681423 +0100
+++ gcc/tree.c	2019-02-26 18:22:23.760753681 +0100
@@ -7533,19 +7533,16 @@ valid_constant_size_p (const_tree size,
       return false;
     }
 
-  tree type = TREE_TYPE (size);
-  if (TYPE_UNSIGNED (type))
+  if (tree_int_cst_sgn (size) < 0)
     {
-      if (!tree_fits_uhwi_p (size)
-	  || tree_int_cst_sign_bit (size))
-	{
-	  *perr = cst_size_too_big;
-	  return false;
-	}
+      *perr = cst_size_negative;
+      return false;
     }
-  else if (tree_int_cst_sign_bit (size))
+  if (!tree_fits_uhwi_p (size)
+      || (wi::to_widest (TYPE_MAX_VALUE (sizetype))
+	  < wi::to_widest (size) * 2))
     {
-      *perr = cst_size_negative;
+      *perr = cst_size_too_big;
       return false;
     }
 
--- gcc/testsuite/g++.dg/other/new2.C.jj	2019-02-26 18:24:23.792785651 +0100
+++ gcc/testsuite/g++.dg/other/new2.C	2019-02-26 18:23:26.530724514 +0100
@@ -0,0 +1,5 @@
+// PR c++/89507
+// { dg-do compile }
+
+unsigned char const n = 128;
+int *p = new int[n];	// { dg-bogus "array exceeds maximum object size" }

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix array size verification (PR c++/89507)
  2019-02-26 20:59 [PATCH] Fix array size verification (PR c++/89507) Jakub Jelinek
@ 2019-02-26 23:12 ` Jason Merrill
  2019-02-27  9:27 ` Richard Biener
  1 sibling, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2019-02-26 23:12 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Biener, Jeff Law; +Cc: gcc-patches

On 2/26/19 3:44 PM, Jakub Jelinek wrote:
> Hi!
> 
> Seems valid_constant_size_p has been written with the expectation that only
> sizetype/ssizetype constants will be passed to it, otherwise it couldn't
> ever just blindly test tree_int_cst_sign_bit (size) for unsigned
> INTEGER_CSTs and complain cst_size_too_big.
> Unfortunately a recent patch started using this function even on other
> types, and the comment explicitly talk about it being done on
> pre-conversion to sizetype:
>        /* The expression in a noptr-new-declarator is erroneous if it's of
>           non-class type and its value before converting to std::size_t is
>           less than zero. ... If the expression is a constant expression,
>           the program is ill-fomed.  */
>        if (TREE_CODE (cst_nelts) == INTEGER_CST
>            && !valid_array_size_p (input_location, cst_nelts, NULL_TREE,
>                                    complain & tf_error))
>          return error_mark_node;
> E.g. __int128 negative value could fit just fine after cast to sizetype,
> etc.
> 
> So, instead of changing the C++ FE to only complain about negative cst_elts
> normally and fold_convert everything to sizetype before checking, this patch
> attempts to deal with non-{,s}sizetype constants.  Negative (signed)
> constants are always rejected as before, newly constants that don't fit into
> uhwi are rejected after that check regardless of signedness and anything
> larger or equal than SIZE_MAX / 2 is also rejected as too big.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-02-26  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89507
> 	* tree.c (valid_constant_size_p): Deal with size INTEGER_CSTs
> 	with types other than sizetype/ssizetype.

Looks good to me.

Jason

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix array size verification (PR c++/89507)
  2019-02-26 20:59 [PATCH] Fix array size verification (PR c++/89507) Jakub Jelinek
  2019-02-26 23:12 ` Jason Merrill
@ 2019-02-27  9:27 ` Richard Biener
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2019-02-27  9:27 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jeff Law, Jason Merrill, gcc-patches

On Tue, 26 Feb 2019, Jakub Jelinek wrote:

> Hi!
> 
> Seems valid_constant_size_p has been written with the expectation that only
> sizetype/ssizetype constants will be passed to it, otherwise it couldn't
> ever just blindly test tree_int_cst_sign_bit (size) for unsigned
> INTEGER_CSTs and complain cst_size_too_big.
> Unfortunately a recent patch started using this function even on other
> types, and the comment explicitly talk about it being done on
> pre-conversion to sizetype:
>       /* The expression in a noptr-new-declarator is erroneous if it's of
>          non-class type and its value before converting to std::size_t is
>          less than zero. ... If the expression is a constant expression,
>          the program is ill-fomed.  */
>       if (TREE_CODE (cst_nelts) == INTEGER_CST
>           && !valid_array_size_p (input_location, cst_nelts, NULL_TREE,
>                                   complain & tf_error))
>         return error_mark_node;
> E.g. __int128 negative value could fit just fine after cast to sizetype,
> etc.
> 
> So, instead of changing the C++ FE to only complain about negative cst_elts
> normally and fold_convert everything to sizetype before checking, this patch
> attempts to deal with non-{,s}sizetype constants.  Negative (signed)
> constants are always rejected as before, newly constants that don't fit into
> uhwi are rejected after that check regardless of signedness and anything
> larger or equal than SIZE_MAX / 2 is also rejected as too big.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-02-26  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89507
> 	* tree.c (valid_constant_size_p): Deal with size INTEGER_CSTs
> 	with types other than sizetype/ssizetype.
> 
> 	* g++.dg/other/new2.C: New test.
> 
> --- gcc/tree.c.jj	2019-02-18 20:48:35.745681423 +0100
> +++ gcc/tree.c	2019-02-26 18:22:23.760753681 +0100
> @@ -7533,19 +7533,16 @@ valid_constant_size_p (const_tree size,
>        return false;
>      }
>  
> -  tree type = TREE_TYPE (size);
> -  if (TYPE_UNSIGNED (type))
> +  if (tree_int_cst_sgn (size) < 0)
>      {
> -      if (!tree_fits_uhwi_p (size)
> -	  || tree_int_cst_sign_bit (size))
> -	{
> -	  *perr = cst_size_too_big;
> -	  return false;
> -	}
> +      *perr = cst_size_negative;
> +      return false;
>      }
> -  else if (tree_int_cst_sign_bit (size))
> +  if (!tree_fits_uhwi_p (size)
> +      || (wi::to_widest (TYPE_MAX_VALUE (sizetype))
> +	  < wi::to_widest (size) * 2))
>      {
> -      *perr = cst_size_negative;
> +      *perr = cst_size_too_big;
>        return false;
>      }
>  
> --- gcc/testsuite/g++.dg/other/new2.C.jj	2019-02-26 18:24:23.792785651 +0100
> +++ gcc/testsuite/g++.dg/other/new2.C	2019-02-26 18:23:26.530724514 +0100
> @@ -0,0 +1,5 @@
> +// PR c++/89507
> +// { dg-do compile }
> +
> +unsigned char const n = 128;
> +int *p = new int[n];	// { dg-bogus "array exceeds maximum object size" }
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-02-27  8:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26 20:59 [PATCH] Fix array size verification (PR c++/89507) Jakub Jelinek
2019-02-26 23:12 ` Jason Merrill
2019-02-27  9:27 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).