[C] Disallow subtracting pointers to empty structs (PR c/58346)

On Wed, Jan 15, 2014 at 09:23:06PM +0000, Joseph S. Myers wrote:
> On Wed, 15 Jan 2014, Marek Polacek wrote:
> 
> > +/* Return true if T is a pointer to a zero-sized struct/union.  */
> > +
> > +bool
> > +pointer_to_zero_sized_aggr_p (tree t)
> > +{
> > +  t = strip_pointer_operator (t);
> > +  return ((RECORD_OR_UNION_TYPE_P (t) || TREE_CODE (t) == ARRAY_TYPE)
> > +	  && TYPE_SIZE (t)
> > +	  && integer_zerop (TYPE_SIZE (t)));
> 
> Why have the (RECORD_OR_UNION_TYPE_P (t) || TREE_CODE (t) == ARRAY_TYPE) 
> check at all?  It may well be the case that those are the only kinds of 
> types that can have zero size here, but the principle of this error 
> applies to anything with zero size so it would seem best not to have that 
> part of the check at all.

Ok, changed.
 
> strip_pointer_operator seems wrong here.  It recursively removes an 
> arbitrary number of pointer type derivations - but where the types are 
> pointer to pointer to zero-size, arithmetic is perfectly valid (so you 
> should have a test that such cases are still accepted, where this patch 
> version would have rejected them).  I believe this function should return 
> true if the argument is a pointer (to anything) and after removal of 
> exactly one level of pointer type derivation, the result has zero size 
> (constant zero - also add a test that the array case where the size is a 
> const int initialized to 0 is not, for C, rejected, as those are VLAs in C 
> terms).

Ah, right, sorry about that.  Hopefully all done in the following.
The if (!POINTER_TYPE_P (t)) check in pointer_to_zero_sized_aggr_p
could perhaps go; I added it mostly for paranoia sake.

2014-01-16  Marek Polacek  <polacek@redhat.com>

	PR c/58346
c-family/
	* c-common.c (pointer_to_zero_sized_aggr_p): New function.
	* c-common.h: Declare it.
cp/
	* typeck.c (pointer_diff): Give an error on arithmetic on pointer to
	an empty aggregate.
c/
	* c-typeck.c (pointer_diff): Give an error on arithmetic on pointer to
	an empty aggregate.
testsuite/
	* c-c++-common/pr58346-1.c: New test.
	* c-c++-common/pr58346-2.c: New test.
	* c-c++-common/pr58346-3.c: New test.


	Marek

Ping.

On Thu, Jan 16, 2014 at 07:52:43PM +0100, Marek Polacek wrote:
> On Wed, Jan 15, 2014 at 09:23:06PM +0000, Joseph S. Myers wrote:
> > On Wed, 15 Jan 2014, Marek Polacek wrote:
> > 
> > > +/* Return true if T is a pointer to a zero-sized struct/union.  */
> > > +
> > > +bool
> > > +pointer_to_zero_sized_aggr_p (tree t)
> > > +{
> > > +  t = strip_pointer_operator (t);
> > > +  return ((RECORD_OR_UNION_TYPE_P (t) || TREE_CODE (t) == ARRAY_TYPE)
> > > +	  && TYPE_SIZE (t)
> > > +	  && integer_zerop (TYPE_SIZE (t)));
> > 
> > Why have the (RECORD_OR_UNION_TYPE_P (t) || TREE_CODE (t) == ARRAY_TYPE) 
> > check at all?  It may well be the case that those are the only kinds of 
> > types that can have zero size here, but the principle of this error 
> > applies to anything with zero size so it would seem best not to have that 
> > part of the check at all.
> 
> Ok, changed.
>  
> > strip_pointer_operator seems wrong here.  It recursively removes an 
> > arbitrary number of pointer type derivations - but where the types are 
> > pointer to pointer to zero-size, arithmetic is perfectly valid (so you 
> > should have a test that such cases are still accepted, where this patch 
> > version would have rejected them).  I believe this function should return 
> > true if the argument is a pointer (to anything) and after removal of 
> > exactly one level of pointer type derivation, the result has zero size 
> > (constant zero - also add a test that the array case where the size is a 
> > const int initialized to 0 is not, for C, rejected, as those are VLAs in C 
> > terms).
> 
> Ah, right, sorry about that.  Hopefully all done in the following.
> The if (!POINTER_TYPE_P (t)) check in pointer_to_zero_sized_aggr_p
> could perhaps go; I added it mostly for paranoia sake.
> 
> 2014-01-16  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c/58346
> c-family/
> 	* c-common.c (pointer_to_zero_sized_aggr_p): New function.
> 	* c-common.h: Declare it.
> cp/
> 	* typeck.c (pointer_diff): Give an error on arithmetic on pointer to
> 	an empty aggregate.
> c/
> 	* c-typeck.c (pointer_diff): Give an error on arithmetic on pointer to
> 	an empty aggregate.
> testsuite/
> 	* c-c++-common/pr58346-1.c: New test.
> 	* c-c++-common/pr58346-2.c: New test.
> 	* c-c++-common/pr58346-3.c: New test.
> 
> --- gcc/c-family/c-common.h.mp	2014-01-15 11:24:41.438608787 +0100
> +++ gcc/c-family/c-common.h	2014-01-16 16:40:06.105688048 +0100
> @@ -789,6 +789,7 @@ extern bool keyword_is_storage_class_spe
>  extern bool keyword_is_type_qualifier (enum rid);
>  extern bool keyword_is_decl_specifier (enum rid);
>  extern bool cxx_fundamental_alignment_p (unsigned);
> +extern bool pointer_to_zero_sized_aggr_p (tree);
>  
>  #define c_sizeof(LOC, T)  c_sizeof_or_alignof_type (LOC, T, true, false, 1)
>  #define c_alignof(LOC, T) c_sizeof_or_alignof_type (LOC, T, false, false, 1)
> --- gcc/c-family/c-common.c.mp	2014-01-16 16:39:17.093487218 +0100
> +++ gcc/c-family/c-common.c	2014-01-16 17:15:37.466403444 +0100
> @@ -11823,4 +11823,15 @@ cxx_fundamental_alignment_p  (unsigned a
>  			 TYPE_ALIGN (long_double_type_node)));
>  }
>  
> +/* Return true if T is a pointer to a zero-sized aggregate.  */
> +
> +bool
> +pointer_to_zero_sized_aggr_p (tree t)
> +{
> +  if (!POINTER_TYPE_P (t))
> +    return false;
> +  t = TREE_TYPE (t);
> +  return (TYPE_SIZE (t) && integer_zerop (TYPE_SIZE (t)));
> +}
> +
>  #include "gt-c-family-c-common.h"
> --- gcc/cp/typeck.c.mp	2014-01-15 11:24:48.940639724 +0100
> +++ gcc/cp/typeck.c	2014-01-16 16:40:06.120688120 +0100
> @@ -5043,6 +5043,14 @@ pointer_diff (tree op0, tree op1, tree p
>  	return error_mark_node;
>      }
>  
> +  if (pointer_to_zero_sized_aggr_p (TREE_TYPE (op1)))
> +    {
> +      if (complain & tf_error)
> +	error ("arithmetic on pointer to an empty aggregate");
> +      else
> +	return error_mark_node;
> +    }
> +
>    op1 = (TYPE_PTROB_P (ptrtype)
>  	 ? size_in_bytes (target_type)
>  	 : integer_one_node);
> --- gcc/c/c-typeck.c.mp	2014-01-15 11:24:53.699659333 +0100
> +++ gcc/c/c-typeck.c	2014-01-16 16:40:06.130688170 +0100
> @@ -3536,6 +3536,9 @@ pointer_diff (location_t loc, tree op0,
>    /* This generates an error if op0 is pointer to incomplete type.  */
>    op1 = c_size_in_bytes (target_type);
>  
> +  if (pointer_to_zero_sized_aggr_p (TREE_TYPE (orig_op1)))
> +    error_at (loc, "arithmetic on pointer to an empty aggregate");
> +
>    /* Divide by the size, in easiest possible way.  */
>    result = fold_build2_loc (loc, EXACT_DIV_EXPR, inttype,
>  			    op0, convert (inttype, op1));
> --- gcc/testsuite/c-c++-common/pr58346-3.c.mp	2014-01-16 17:13:26.184800572 +0100
> +++ gcc/testsuite/c-c++-common/pr58346-3.c	2014-01-16 17:30:15.503506328 +0100
> @@ -0,0 +1,16 @@
> +/* PR c/58346 */
> +/* { dg-do compile } */
> +
> +void
> +foo (void)
> +{
> +  __PTRDIFF_TYPE__ d;
> +  const int i = 0;
> +  int a1[2][0], a2[2][0];
> +  int b1[3][i], b2[4][i];
> +  d = a1 - a2; /* { dg-error "arithmetic on pointer to an empty aggregate" } */
> +  __asm volatile ("" : "+g" (d));
> +  /* No error here for C.  */
> +  d = b1 - b2; /* { dg-error "arithmetic on pointer to an empty aggregate" "" { target c++ } } */
> +  __asm volatile ("" : "+g" (d));
> +}
> --- gcc/testsuite/c-c++-common/pr58346-1.c.mp	2014-01-15 14:04:27.159428834 +0100
> +++ gcc/testsuite/c-c++-common/pr58346-1.c	2014-01-16 16:59:13.428236540 +0100
> @@ -0,0 +1,24 @@
> +/* PR c/58346 */
> +/* { dg-do compile } */
> +
> +struct U {
> +#ifdef __cplusplus
> +  char a[0];
> +#endif
> +};
> +static struct U b[6];
> +static struct U *u1, *u2;
> +
> +int
> +foo (struct U *p, struct U *q)
> +{
> +  return q - p; /* { dg-error "arithmetic on pointer to an empty aggregate" } */
> +}
> +
> +void
> +bar (void)
> +{
> +  __PTRDIFF_TYPE__ d = u1 - u2; /* { dg-error "arithmetic on pointer to an empty aggregate" } */
> +  __asm volatile ("" : "+g" (d));
> +  foo (&b[0], &b[4]);
> +}
> --- gcc/testsuite/c-c++-common/pr58346-2.c.mp	2014-01-13 15:48:20.011420141 +0100
> +++ gcc/testsuite/c-c++-common/pr58346-2.c	2014-01-15 14:05:57.042817401 +0100
> @@ -0,0 +1,8 @@
> +/* PR c/58346 */
> +/* { dg-do compile } */
> +
> +__PTRDIFF_TYPE__
> +foo (int p[3][0], int q[3][0])
> +{
> +  return p - q; /* { dg-error "arithmetic on pointer to an empty aggregate" } */
> +}
> 
> 	Marek

	Marek

On Thu, 23 Jan 2014, Marek Polacek wrote:

> > 2014-01-16  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR c/58346
> > c-family/
> > 	* c-common.c (pointer_to_zero_sized_aggr_p): New function.
> > 	* c-common.h: Declare it.
> > cp/
> > 	* typeck.c (pointer_diff): Give an error on arithmetic on pointer to
> > 	an empty aggregate.
> > c/
> > 	* c-typeck.c (pointer_diff): Give an error on arithmetic on pointer to
> > 	an empty aggregate.
> > testsuite/
> > 	* c-c++-common/pr58346-1.c: New test.
> > 	* c-c++-common/pr58346-2.c: New test.
> > 	* c-c++-common/pr58346-3.c: New test.

OK.