Re: [PATCH] [PR c++/85039] no type definitions in builtin offsetof

[Jason Merrill <jason at redhat dot com>]

On Fri, Apr 13, 2018, 6:09 PM Alexandre Oliva <aoliva@redhat.com> wrote:

> On Apr 11, 2018, Jason Merrill <jason@redhat.com> wrote:
>
> > I raised this issue with the C++ committee, and it seems that nobody
> > expects defining a type here to work.  So let's go back to your first
> > patch, removing the offending part of semicolon3.C.
>
> All right, here's the adjusted patch.  Retested on i686- and
> x86_64-linux-gnu.  Ok to install?
>

Ok.

Jason

[PR c++/85039] no type definitions in builtin offsetof
>
> Types defined within a __builtin_offsetof argument don't always get
> properly recorded as members of their context types, so if they're
> anonymous, we may fail to assign them an anon type index for mangling
> and ICE.
>
> We shouldn't allow types to be introduced in __builtin_offsetof, I
> think, and Jason says the std committee agrees, so I've arranged for
> us to reject them.
>
> Even then, we still parse the definitions and attempt to assign
> mangled names to its member functions, so the ICE remains.  Since
> we've already reported an error, we might as well complete the name
> assignment with an arbitrary index, thus avoiding the ICE.
>
> We used to have a test that expected to be able to define types in
> __builtin_offsetof; this patch removes that specific test.
>
>
> for  gcc/cp/ChangeLog
>
>         PR c++/85039
>         * parser.c (cp_parser_builtin_offset): Reject type definitions.
>         * mangle.c (nested_anon_class_index): Avoid crash returning -1
>         if we've seen errors.
>
> for  gcc/testsuite/ChangeLog
>
>         PR c++/85039
>         * g++.dg/pr85039-1.C: New.
>         * g++.dg/pr85039-2.C: New.
>         * g++.dg/parse/semicolon3.C: Remove test_offsetof.
> ---
>  gcc/cp/mangle.c                         |    3 +++
>  gcc/cp/parser.c                         |    8 +++++++-
>  gcc/testsuite/g++.dg/parse/semicolon3.C |    7 -------
>  gcc/testsuite/g++.dg/pr85039-1.C        |   17 +++++++++++++++++
>  gcc/testsuite/g++.dg/pr85039-2.C        |   10 ++++++++++
>  5 files changed, 37 insertions(+), 8 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/pr85039-1.C
>  create mode 100644 gcc/testsuite/g++.dg/pr85039-2.C
>
> diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
> index 94c4bed28486..a7f9d686345d 100644
> --- a/gcc/cp/mangle.c
> +++ b/gcc/cp/mangle.c
> @@ -1623,6 +1623,9 @@ nested_anon_class_index (tree type)
>           ++index;
>        }
>
> +  if (seen_error ())
> +    return -1;
> +
>    gcc_unreachable ();
>  }
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 8b1b271b53d1..bf46165f5ae1 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -9823,7 +9823,13 @@ cp_parser_builtin_offsetof (cp_parser *parser)
>    parens.require_open (parser);
>    /* Parse the type-id.  */
>    location_t loc = cp_lexer_peek_token (parser->lexer)->location;
> -  type = cp_parser_type_id (parser);
> +  {
> +    const char *saved_message = parser->type_definition_forbidden_message;
> +    parser->type_definition_forbidden_message
> +      = G_("types may not be defined within __builtin_offsetof");
> +    type = cp_parser_type_id (parser);
> +    parser->type_definition_forbidden_message = saved_message;
> +  }
>    /* Look for the `,'.  */
>    cp_parser_require (parser, CPP_COMMA, RT_COMMA);
>    token = cp_lexer_peek_token (parser->lexer);
> diff --git a/gcc/testsuite/g++.dg/parse/semicolon3.C
> b/gcc/testsuite/g++.dg/parse/semicolon3.C
> index 8a2b1ac46301..0d46be9ed654 100644
> --- a/gcc/testsuite/g++.dg/parse/semicolon3.C
> +++ b/gcc/testsuite/g++.dg/parse/semicolon3.C
> @@ -20,13 +20,6 @@ struct OK3
>  } // no complaints
>    (s7);
>
> -__SIZE_TYPE__
> -test_offsetof (void)
> -{
> -  // no complaints about a missing semicolon
> -  return __builtin_offsetof (struct OK4 { int a; int b; }, b);
> -}
> -
>  struct OK5
>  {
>    int a;
> diff --git a/gcc/testsuite/g++.dg/pr85039-1.C
> b/gcc/testsuite/g++.dg/pr85039-1.C
> new file mode 100644
> index 000000000000..f57c8a261dee
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr85039-1.C
> @@ -0,0 +1,17 @@
> +// { dg-do compile { target c++14 } }
> +
> +constexpr int a() {
> + return
> +  __builtin_offsetof(struct { // { dg-error "types may not be defined" }
> +    int i;
> +    short b {
> +      __builtin_offsetof(struct { // { dg-error "types may not be
> defined" }
> +       int j;
> +        struct c { // { dg-error "types may not be defined" }
> +          void d() {
> +          }
> +        };
> +      }, j)
> +    };
> +  }, i);
> +}
> diff --git a/gcc/testsuite/g++.dg/pr85039-2.C
> b/gcc/testsuite/g++.dg/pr85039-2.C
> new file mode 100644
> index 000000000000..e6d16325105b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr85039-2.C
> @@ -0,0 +1,10 @@
> +// { dg-do compile }
> +
> +struct d {
> +  static d *b;
> +} * d::b(__builtin_offsetof(struct { // { dg-error "types may not be
> defined" }
> +  int i;
> +  struct a { // { dg-error "types may not be defined" }
> +    int c() { return .1f; }
> +  };
> +}, i));
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
>