From: Marek Polacek <polacek@redhat.com>
To: "Joseph S. Myers" <joseph@codesourcery.com>
Cc: Jakub Jelinek <jakub@redhat.com>,
GCC Patches <gcc-patches@gcc.gnu.org>,
Richard Biener <rguenther@suse.de>,
Jason Merrill <jason@redhat.com>
Subject: Re: [C PATCH] Disallow subtracting pointers to empty structs (PR c/58346)
Date: Thu, 16 Jan 2014 18:52:00 -0000 [thread overview]
Message-ID: <20140116185242.GG8907@redhat.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1401152117180.24518@digraph.polyomino.org.uk>
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
next prev parent reply other threads:[~2014-01-16 18:52 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-13 16:32 Marek Polacek
2014-01-13 17:21 ` Florian Weimer
2014-01-13 17:22 ` Marek Polacek
2014-01-13 20:48 ` Marek Polacek
2014-01-14 14:39 ` Jason Merrill
2014-01-14 17:52 ` Florian Weimer
2014-01-15 9:12 ` Marek Polacek
2014-01-14 21:42 ` Joseph S. Myers
2014-01-15 10:27 ` Marek Polacek
2014-01-15 10:35 ` Jakub Jelinek
2014-01-15 13:50 ` Marek Polacek
2014-01-15 21:23 ` Joseph S. Myers
2014-01-16 18:52 ` Marek Polacek [this message]
2014-01-23 9:15 ` Marek Polacek
2014-01-23 17:47 ` Joseph S. Myers
2014-01-16 11:50 ` Eric Botcazou
2014-01-16 11:58 ` Marek Polacek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140116185242.GG8907@redhat.com \
--to=polacek@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jason@redhat.com \
--cc=joseph@codesourcery.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).