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

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

[Marek Polacek]

This doesn't really fix the PR, but solves a related issue, where we
have e.g.
struct U {};
static struct U b[6];

int foo (struct U *p, struct U *q)
{
  return q - p;
}
int main()
{
  return foo (&b[0], &b[4]);
}
Such a program SIGFPEs at runtime.  But subtraction of pointers to empty
structures/unions doesn't really make sense and this patch forbids that.
Note that GCC permits a structure/union to have no members, but it's only
an extension, in C11 it's undefined behavior.

Regtested/bootstrapped on x86_64, ok for trunk?

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

	PR c/58346
c/
	* c-typeck.c (pointer_to_empty_aggr_p): New function.
	(pointer_diff): Give an error on arithmetic on pointer to an
	empty aggregate.
testsuite/
	* gcc.dg/pr58346.c: New test.

--- gcc/c/c-typeck.c.mp	2014-01-13 15:47:01.316105676 +0100
+++ gcc/c/c-typeck.c	2014-01-13 16:03:35.513081392 +0100
@@ -3427,6 +3427,18 @@ parser_build_binary_op (location_t locat
   return result;
 }
 \f
+
+/* Return true if T is a pointer to an empty struct/union.  */
+
+static bool
+pointer_to_empty_aggr_p (tree t)
+{
+  t = strip_pointer_operator (t);
+  if (!RECORD_OR_UNION_TYPE_P (t))
+    return false;
+  return TYPE_FIELDS (t) == NULL_TREE;
+}
+
 /* Return a tree for the difference of pointers OP0 and OP1.
    The resulting tree has type int.  */
 
@@ -3536,6 +3548,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_empty_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/gcc.dg/pr58346.c.mp	2014-01-13 15:48:20.011420141 +0100
+++ gcc/testsuite/gcc.dg/pr58346.c	2014-01-13 16:01:41.741713601 +0100
@@ -0,0 +1,21 @@
+/* PR c/58346 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+struct U {};
+static struct U b[6];
+static struct U **s1, **s2;
+
+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 = s1 - s2; /* { dg-error "arithmetic on pointer to an empty aggregate" } */
+  __asm volatile ("" : "+g" (d));
+  foo (&b[0], &b[4]);
+}

	Marek

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

[Florian Weimer]

On 01/13/2014 05:32 PM, Marek Polacek wrote:

> This doesn't really fix the PR, but solves a related issue, where we
> have e.g.
> struct U {};
> static struct U b[6];
>
> int foo (struct U *p, struct U *q)
> {
>    return q - p;
> }
> int main()
> {
>    return foo (&b[0], &b[4]);
> }

> Such a program SIGFPEs at runtime.  But subtraction of pointers to empty
> structures/unions doesn't really make sense and this patch forbids that.
> Note that GCC permits a structure/union to have no members, but it's only

> +  if (pointer_to_empty_aggr_p (TREE_TYPE (orig_op1)))
> +    error_at (loc, "arithmetic on pointer to an empty aggregate");

You need to check the size of the aggregate, not if it has no members. 
With your patch applied, if the struct definition in your test case is 
changed to this:

struct U { char empty[0]; };

it still compiles and fails at run time.

Empty structs have size 1 in C++, but structs with a zero-length array 
have size 0, so the C++ compiler should be changed as well.

-- 
Florian Weimer / Red Hat Product Security Team

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

[Marek Polacek]

On Mon, Jan 13, 2014 at 05:32:26PM +0100, Marek Polacek wrote:
> This doesn't really fix the PR, but solves a related issue, where we
> have e.g.
> struct U {};
> static struct U b[6];
> 
> int foo (struct U *p, struct U *q)
> {
>   return q - p;
> }
> int main()
> {
>   return foo (&b[0], &b[4]);
> }
> Such a program SIGFPEs at runtime.  But subtraction of pointers to empty
> structures/unions doesn't really make sense and this patch forbids that.
> Note that GCC permits a structure/union to have no members, but it's only
> an extension, in C11 it's undefined behavior.
> 
> Regtested/bootstrapped on x86_64, ok for trunk?

The patch will need some tweaking, I realized that e.g. for struct S {
union {}; }; it doesn't do the right thing...

	Marek

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

[Marek Polacek]

On Mon, Jan 13, 2014 at 05:48:59PM +0100, Marek Polacek wrote:
> The patch will need some tweaking, I realized that e.g. for struct S {
> union {}; }; it doesn't do the right thing...

Done in the patch below.  CCing Jason for the C++ part.  Does this
look sane now?

Regtested/bootstrapped on x86_64.

2014-01-13  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.c: New test.

--- gcc/c-family/c-common.h.mp	2014-01-13 19:02:22.249870601 +0100
+++ gcc/c-family/c-common.h	2014-01-13 19:04:15.068294390 +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-13 19:01:20.503637616 +0100
+++ gcc/c-family/c-common.c	2014-01-13 19:42:32.805135382 +0100
@@ -11829,4 +11829,17 @@ cxx_fundamental_alignment_p  (unsigned a
 			 TYPE_ALIGN (long_double_type_node)));
 }
 
+/* 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);
+  if (RECORD_OR_UNION_TYPE_P (t)
+      && TYPE_SIZE (t)
+      && integer_zerop (TYPE_SIZE (t)))
+    return true;
+  return false;
+}
+
 #include "gt-c-family-c-common.h"
--- gcc/cp/typeck.c.mp	2014-01-13 19:08:12.237244663 +0100
+++ gcc/cp/typeck.c	2014-01-13 19:10:23.350742070 +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-13 15:47:01.316105676 +0100
+++ gcc/c/c-typeck.c	2014-01-13 19:58:19.237271626 +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.c.mp	2014-01-13 15:48:20.011420141 +0100
+++ gcc/testsuite/c-c++-common/pr58346.c	2014-01-13 20:25:17.544582444 +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]);
+}

	Marek

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

[Jason Merrill]

The C++ part is OK.

Jason

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

[Florian Weimer]

On 01/13/2014 09:48 PM, Marek Polacek wrote:
> +bool
> +pointer_to_zero_sized_aggr_p (tree t)
> +{
> +  t = strip_pointer_operator (t);
> +  if (RECORD_OR_UNION_TYPE_P (t)
> +      && TYPE_SIZE (t)
> +      && integer_zerop (TYPE_SIZE (t)))
> +    return true;
> +  return false;
> +}

I think you can just return the value of the condition, there's no need 
for the if statement.

-- 
Florian Weimer / Red Hat Product Security Team

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

[Joseph S. Myers]

On Mon, 13 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);
> +  if (RECORD_OR_UNION_TYPE_P (t)
> +      && TYPE_SIZE (t)
> +      && integer_zerop (TYPE_SIZE (t)))
> +    return true;
> +  return false;

Given that GNU C also allows arrays of constant size 0, shouldn't the 
errors also apply in that case?  (I don't know whether the original bug 
can appear for such arrays, but I'd think the errors should apply to 
anything with constant size 0 - not of course for VLAs where it just so 
happens that the compiler can tell at compile time that the size is always 
0.)

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Marek Polacek]

On Tue, Jan 14, 2014 at 06:52:00PM +0100, Florian Weimer wrote:
> On 01/13/2014 09:48 PM, Marek Polacek wrote:
> >+bool
> >+pointer_to_zero_sized_aggr_p (tree t)
> >+{
> >+  t = strip_pointer_operator (t);
> >+  if (RECORD_OR_UNION_TYPE_P (t)
> >+      && TYPE_SIZE (t)
> >+      && integer_zerop (TYPE_SIZE (t)))
> >+    return true;
> >+  return false;
> >+}
> 
> I think you can just return the value of the condition, there's no
> need for the if statement.

Oops, yeah, forgot to change that.  Will do it in next patch.

	Marek

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

[Marek Polacek]

On Tue, Jan 14, 2014 at 09:42:37PM +0000, Joseph S. Myers wrote:
> On Mon, 13 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);
> > +  if (RECORD_OR_UNION_TYPE_P (t)
> > +      && TYPE_SIZE (t)
> > +      && integer_zerop (TYPE_SIZE (t)))
> > +    return true;
> > +  return false;
> 
> Given that GNU C also allows arrays of constant size 0, shouldn't the 
> errors also apply in that case?  (I don't know whether the original bug 
> can appear for such arrays, but I'd think the errors should apply to 
> anything with constant size 0 - not of course for VLAs where it just so 
> happens that the compiler can tell at compile time that the size is always 
> 0.)

Perhaps, but I don't think we can do it easily.  Consider

int
foo (int *p, int *q)
{
  return p - q;
}

int
bar (void)
{
  int a[0], b[0];
  return foo (a, b);
}

pointer_diff doesn't know that what p/q point to are zero-sized
arrays.  We could do something about

int
foo (void)
{
  int a[0], b[0];
  return a - b;
}

but in the light of the beforementioned issue, I'm not sanguine about
that.

And no, the original bug doesn't trigger: we have 
(((long int) p - (long int) q) /[ex] 4)
here and not
(((long int) p - (long int) q) /[ex] 0)

Here's patch with a change Florian suggested.

2014-01-15  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.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-15 11:24:57.724675857 +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-15 11:24:43.117615687 +0100
+++ gcc/c-family/c-common.c	2014-01-15 11:24:57.723675852 +0100
@@ -11829,4 +11829,15 @@ cxx_fundamental_alignment_p  (unsigned a
 			 TYPE_ALIGN (long_double_type_node)));
 }
 
+/* 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)
+	  && 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-15 11:24:57.740675935 +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-15 11:24:57.732675897 +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.c.mp	2014-01-13 15:48:20.011420141 +0100
+++ gcc/testsuite/c-c++-common/pr58346.c	2014-01-13 20:25:17.544582444 +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]);
+}

	Marek

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

[Jakub Jelinek]

On Wed, Jan 15, 2014 at 11:27:37AM +0100, Marek Polacek wrote:
> Perhaps, but I don't think we can do it easily.  Consider
> 
> int
> foo (int *p, int *q)
> {
>   return p - q;
> }

That is not a difference of pointers to zero sized arrays though,
that is pointers to int, and there is no division by zero involved, you can
get 0 as the result of course.

I think Joseph meant say

__PTRDIFF_TYPE__
foo (int p[3][0], int q[3][0])
{
  return p - q;
}

which is also (((long int) p - (long int) q) /[ex] 0).

	Jakub

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

[Marek Polacek]

On Wed, Jan 15, 2014 at 11:35:40AM +0100, Jakub Jelinek wrote:
> On Wed, Jan 15, 2014 at 11:27:37AM +0100, Marek Polacek wrote:
> > Perhaps, but I don't think we can do it easily.  Consider
> > 
> > int
> > foo (int *p, int *q)
> > {
> >   return p - q;
> > }
> 
> That is not a difference of pointers to zero sized arrays though,
> that is pointers to int, and there is no division by zero involved, you can
> get 0 as the result of course.
> 
> I think Joseph meant say
> 
> __PTRDIFF_TYPE__
> foo (int p[3][0], int q[3][0])
> {
>   return p - q;
> }
> 
> which is also (((long int) p - (long int) q) /[ex] 0).

Ah, of course.  The following should take into account such arrays.

Regtested/bootstrapped on x86_64-linux, ok for trunk now?

2014-01-15  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.

--- gcc/c-family/c-common.h.mp	2014-01-15 11:24:41.438608787 +0100
+++ gcc/c-family/c-common.h	2014-01-15 11:24:57.724675857 +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-15 11:24:43.117615687 +0100
+++ gcc/c-family/c-common.c	2014-01-15 11:48:02.109071662 +0100
@@ -11829,4 +11829,15 @@ cxx_fundamental_alignment_p  (unsigned a
 			 TYPE_ALIGN (long_double_type_node)));
 }
 
+/* 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)));
+}
+
 #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-15 11:24:57.740675935 +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-15 11:24:57.732675897 +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-1.c.mp	2014-01-15 14:04:27.159428834 +0100
+++ gcc/testsuite/c-c++-common/pr58346-1.c	2014-01-13 20:25:17.544582444 +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

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

[Joseph S. Myers]

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.

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).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Eric Botcazou]

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

That's AGGREGATE_TYPE_P (and the comment seems to be out-of-sync).

-- 
Eric Botcazou

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

[Marek Polacek]

On Thu, Jan 16, 2014 at 12:50:10PM +0100, Eric Botcazou 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)));
> > +}
> 
> That's AGGREGATE_TYPE_P (and the comment seems to be out-of-sync).

Right.  However, I need to drop the check altogether anyway.  I'll
update the comment.  Thanks.

	Marek

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

[Marek Polacek]

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

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

[Marek Polacek]

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

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

[Joseph S. Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com