diff mbox

[1/2] Add a STATIC_ASSERT on sizeof (struct cp_token)

Message ID 1469575179-53321-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm July 26, 2016, 11:19 p.m. UTC 
On Mon, 2016-07-25 at 10:36 +0200, Richard Biener wrote:
> On Fri, Jul 22, 2016 at 4:11 PM, Jakub Jelinek <jakub@redhat.com>
> wrote:
> > On Fri, Jul 22, 2016 at 10:33:50AM -0400, David Malcolm wrote:
> > > gcc/cp/ChangeLog:
> > >       * parser.h (struct cp_token): Add a STATIC_ASSERT on the
> > >       size of the struct.
> > > ---
> > >  gcc/cp/parser.h | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
> > > index ccbace9..8c1de57 100644
> > > --- a/gcc/cp/parser.h
> > > +++ b/gcc/cp/parser.h
> > > @@ -71,6 +71,15 @@ struct GTY (()) cp_token {
> > >              "|| (%1.type == CPP_DECLTYPE)"))) u;
> > >  };
> > > 
> > > +/* The C++ frontend lexes everything first, and keeps the tokens
> > > +   in memory, so there are possibly millions of tokens in
> > > memory.
> > > +   Ensure that we don't accidentally grow the structure.  */
> > > +STATIC_ASSERT (sizeof (cp_token) ==
> > > +            (2 // "type" and "keyword"
> > > +             + 1 // "flags"
> > > +             + 1 // bitfields
> > > +             + 4 // location_t
> > > +             + sizeof (void *))); // union
> > 
> > Doesn't that assume way too much on the host data layout?
> > This can be compiled with weirdo system compilers or on weirdo
> > hosts,
> > I bet we don't really support non-8-bit char hosts, but still this
> > might
> > break somewhere...
> > 
> > The formatting is wrong BTW, == shouldn't appear at the end of
> > line.
> 
> Maybe restrict it to __GNUC__ and __x86_64__ or so.
> 
> Richard.
> 
> >         Jakub

Bootstrapped&regrtested on x86_64-pc-linux-gnu (in conjunction
with the followup patch)

OK for trunk if it passes testing by itself?

gcc/cp/ChangeLog:
	* parser.h (struct cp_token): Add a STATIC_ASSERT on the
	size of the struct.
---
 gcc/cp/parser.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Andreas Schwab July 27, 2016, 6:59 a.m. UTC | #1 
On Mi, Jul 27 2016, David Malcolm <dmalcolm@redhat.com> wrote:

> +/* The C++ frontend lexes everything first, and keeps the tokens
> +   in memory, so there are possibly millions of tokens in memory.
> +
> +   Use a STATIC_ASSERT to ensure that we don't accidentally grow
> +   the structure.
> +
> +   To avoid introducing too many assumptions on the host data layout,
> +   only enable the assertion when compiling with GCC for a
> +   known-good host.  */
> +#if defined (__GNUC__) && defined (__x86_64__)
> +STATIC_ASSERT (sizeof (cp_token) ==

If you make that <= then you can enable it on more hosts.

Andreas.
diff mbox

Patch

diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
index 2923378..bc961c5 100644
--- a/gcc/cp/parser.h
+++ b/gcc/cp/parser.h
@@ -71,6 +71,23 @@  struct GTY (()) cp_token {
 	       "|| (%1.type == CPP_DECLTYPE)"))) u;
 };
 
+/* The C++ frontend lexes everything first, and keeps the tokens
+   in memory, so there are possibly millions of tokens in memory.
+
+   Use a STATIC_ASSERT to ensure that we don't accidentally grow
+   the structure.
+
+   To avoid introducing too many assumptions on the host data layout,
+   only enable the assertion when compiling with GCC for a
+   known-good host.  */
+#if defined (__GNUC__) && defined (__x86_64__)
+STATIC_ASSERT (sizeof (cp_token) ==
+	       (2 // "type" and "keyword"
+		+ 1 // "flags"
+		+ 1 // bitfields
+		+ 4 // location_t
+		+ sizeof (void *))); // union
+#endif /* #if defined (__GNUC__) && defined (__x86_64__) */
 
 /* We use a stack of token pointer for saving token sets.  */
 typedef struct cp_token *cp_token_position;