public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: Marek Polacek <polacek@redhat.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] c++: Reject UDLs in certain contexts [PR105300]
Date: Tue, 15 Nov 2022 18:58:39 -0500	[thread overview]
Message-ID: <f532848a-bb39-0032-a625-4c4365de610f@redhat.com> (raw)
In-Reply-To: <20221112165331.349041-1-polacek@redhat.com>

On 11/12/22 06:53, Marek Polacek wrote:
> In this PR, we are crashing because we've encountered a UDL where a
> string-literal is expected.  This patch makes the parser reject string
> and character UDLs in all places where the grammar requires a
> string-literal and not a user-defined-string-literal.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

Since the grammar has

user-defined-string-literal :
	string-literal ud-suffix

maybe we want to move the UDL handling out to a 
cp_parser_udl_string_literal that calls cp_parser_string_literal?

> 	PR c++/105300
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-pragma.cc (handle_pragma_message): Warn for CPP_STRING_USERDEF.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_string_literal): Add a bool parameter.
> 	Give an error when UDLs are not permitted.
> 	(cp_parser_primary_expression): Adjust the call to
> 	cp_parser_string_literal.
> 	(cp_parser_linkage_specification): Likewise.
> 	(cp_parser_static_assert): Likewise.
> 	(cp_parser_operator): Likewise.
> 	(cp_parser_asm_definition): Likewise.
> 	(cp_parser_asm_specification_opt): Likewise.
> 	(cp_parser_asm_operand_list): Likewise.
> 	(cp_parser_asm_clobber_list): Likewise.
> 	(cp_parser_omp_context_selector): Likewise.
> 	(pragma_lex): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/udlit-error1.C: New test.
> ---
>   gcc/c-family/c-pragma.cc                  |  3 +
>   gcc/cp/parser.cc                          | 69 ++++++++++++++---------
>   gcc/testsuite/g++.dg/cpp0x/udlit-error1.C | 21 +++++++
>   3 files changed, 65 insertions(+), 28 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/udlit-error1.C
> 
> diff --git a/gcc/c-family/c-pragma.cc b/gcc/c-family/c-pragma.cc
> index 142a46441ac..49f405b605b 100644
> --- a/gcc/c-family/c-pragma.cc
> +++ b/gcc/c-family/c-pragma.cc
> @@ -1390,6 +1390,9 @@ handle_pragma_message (cpp_reader *)
>       }
>     else if (token == CPP_STRING)
>       message = x;
> +  else if (token == CPP_STRING_USERDEF)
> +    GCC_BAD ("string literal with user-defined suffix is invalid in this "
> +	     "context");
>     else
>       GCC_BAD ("expected a string after %<#pragma message%>");
>   
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index e4021835ed5..ae2798e2a33 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -2226,7 +2226,7 @@ pop_unparsed_function_queues (cp_parser *parser)
>   static cp_expr cp_parser_identifier
>     (cp_parser *);
>   static cp_expr cp_parser_string_literal
> -  (cp_parser *, bool, bool, bool);
> +  (cp_parser *, bool, bool, bool, bool);
>   static cp_expr cp_parser_userdef_char_literal
>     (cp_parser *);
>   static tree cp_parser_userdef_string_literal
> @@ -4402,7 +4402,8 @@ cp_parser_identifier (cp_parser* parser)
>      TREE_STRING representing the combined, nul-terminated string
>      constant.  If TRANSLATE is true, translate the string to the
>      execution character set.  If WIDE_OK is true, a wide string is
> -   invalid here.
> +   valid here.  If UDL_OK is true, a string literal with user-defined
> +   suffix can be used in this context.
>   
>      C++98 [lex.string] says that if a narrow string literal token is
>      adjacent to a wide string literal token, the behavior is undefined.
> @@ -4414,7 +4415,7 @@ cp_parser_identifier (cp_parser* parser)
>      FUTURE: ObjC++ will need to handle @-strings here.  */
>   static cp_expr
>   cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
> -			  bool lookup_udlit = true)
> +			  bool udl_ok, bool lookup_udlit = true)
>   {
>     tree value;
>     size_t count;
> @@ -4439,6 +4440,12 @@ cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
>   
>     if (cpp_userdef_string_p (tok->type))
>       {
> +      if (!udl_ok)
> +	{
> +	  error_at (loc, "string literal with user-defined suffix "
> +		    "is invalid in this context");
> +	  return error_mark_node;
> +	}
>         string_tree = USERDEF_LITERAL_VALUE (tok->u.value);
>         curr_type = cpp_userdef_string_remove_type (tok->type);
>         curr_tok_is_userdef_p = true;
> @@ -5655,7 +5662,7 @@ cp_parser_primary_expression (cp_parser *parser,
>   	 argument to cp_parser_string_literal.  */
>         return (cp_parser_string_literal (parser,
>   					parser->translate_strings_p,
> -					true)
> +					/*wide_ok=*/true, /*udl_ok=*/true)
>   	      .maybe_add_location_wrapper ());
>   
>       case CPP_OPEN_PAREN:
> @@ -16161,15 +16168,14 @@ cp_parser_function_specifier_opt (cp_parser* parser,
>   static void
>   cp_parser_linkage_specification (cp_parser* parser, tree prefix_attr)
>   {
> -  tree linkage;
> -
>     /* Look for the `extern' keyword.  */
>     cp_token *extern_token
>       = cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN);
>   
>     /* Look for the string-literal.  */
>     cp_token *string_token = cp_lexer_peek_token (parser->lexer);
> -  linkage = cp_parser_string_literal (parser, false, false);
> +  tree linkage = cp_parser_string_literal (parser, /*translate=*/false,
> +					   /*wide_ok=*/false, /*udl_ok=*/false);
>   
>     /* Transform the literal into an identifier.  If the literal is a
>        wide-character string, or contains embedded NULs, then we can't
> @@ -16300,8 +16306,8 @@ cp_parser_static_assert(cp_parser *parser, bool member_p)
>   
>         /* Parse the string-literal message.  */
>         message = cp_parser_string_literal (parser,
> -                                	  /*translate=*/false,
> -                                	  /*wide_ok=*/true);
> +					  /*translate=*/false, /*wide_ok=*/true,
> +					  /*udl_ok=*/false);
>   
>         /* A `)' completes the static assertion.  */
>         if (!parens.require_close (parser))
> @@ -17349,7 +17355,6 @@ cp_parser_operator (cp_parser* parser, location_t start_loc)
>       case CPP_STRING16_USERDEF:
>       case CPP_STRING32_USERDEF:
>         {
> -	cp_expr str;
>   	tree string_tree;
>   	int sz, len;
>   
> @@ -17357,8 +17362,10 @@ cp_parser_operator (cp_parser* parser, location_t start_loc)
>   	  maybe_warn_cpp0x (CPP0X_USER_DEFINED_LITERALS);
>   
>   	/* Consume the string.  */
> -	str = cp_parser_string_literal (parser, /*translate=*/true,
> -				      /*wide_ok=*/true, /*lookup_udlit=*/false);
> +	cp_expr str = cp_parser_string_literal (parser, /*translate=*/true,
> +						/*wide_ok=*/true,
> +						/*udl_ok=*/true,
> +						/*lookup_udlit=*/false);
>   	if (str == error_mark_node)
>   	  return error_mark_node;
>   	else if (TREE_CODE (str) == USERDEF_LITERAL)
> @@ -21975,7 +21982,6 @@ cp_parser_using_directive (cp_parser* parser)
>   static void
>   cp_parser_asm_definition (cp_parser* parser)
>   {
> -  tree string;
>     tree outputs = NULL_TREE;
>     tree inputs = NULL_TREE;
>     tree clobbers = NULL_TREE;
> @@ -22083,7 +22089,8 @@ cp_parser_asm_definition (cp_parser* parser)
>     if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
>       return;
>     /* Look for the string.  */
> -  string = cp_parser_string_literal (parser, false, false);
> +  tree string = cp_parser_string_literal (parser, /*translate=*/false,
> +					  /*wide_ok=*/false, /*udl_ok=*/false);
>     if (string == error_mark_node)
>       {
>         cp_parser_skip_to_closing_parenthesis (parser, true, false,
> @@ -28485,11 +28492,8 @@ cp_parser_yield_expression (cp_parser* parser)
>   static tree
>   cp_parser_asm_specification_opt (cp_parser* parser)
>   {
> -  cp_token *token;
> -  tree asm_specification;
> -
>     /* Peek at the next token.  */
> -  token = cp_lexer_peek_token (parser->lexer);
> +  cp_token *token = cp_lexer_peek_token (parser->lexer);
>     /* If the next token isn't the `asm' keyword, then there's no
>        asm-specification.  */
>     if (!cp_parser_is_keyword (token, RID_ASM))
> @@ -28502,7 +28506,10 @@ cp_parser_asm_specification_opt (cp_parser* parser)
>     parens.require_open (parser);
>   
>     /* Look for the string-literal.  */
> -  asm_specification = cp_parser_string_literal (parser, false, false);
> +  tree asm_specification = cp_parser_string_literal (parser,
> +						     /*translate=*/false,
> +						     /*wide_ok=*/false,
> +						     /*udl_ok=*/false);
>   
>     /* Look for the `)'.  */
>     parens.require_close (parser);
> @@ -28535,8 +28542,6 @@ cp_parser_asm_operand_list (cp_parser* parser)
>   
>     while (true)
>       {
> -      tree string_literal;
> -      tree expression;
>         tree name;
>   
>         if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_SQUARE))
> @@ -28554,13 +28559,16 @@ cp_parser_asm_operand_list (cp_parser* parser)
>         else
>   	name = NULL_TREE;
>         /* Look for the string-literal.  */
> -      string_literal = cp_parser_string_literal (parser, false, false);
> +      tree string_literal = cp_parser_string_literal (parser,
> +						      /*translate=*/false,
> +						      /*wide_ok=*/false,
> +						      /*udl_ok=*/false);
>   
>         /* Look for the `('.  */
>         matching_parens parens;
>         parens.require_open (parser);
>         /* Parse the expression.  */
> -      expression = cp_parser_expression (parser);
> +      tree expression = cp_parser_expression (parser);
>         /* Look for the `)'.  */
>         parens.require_close (parser);
>   
> @@ -28600,10 +28608,11 @@ cp_parser_asm_clobber_list (cp_parser* parser)
>   
>     while (true)
>       {
> -      tree string_literal;
> -
>         /* Look for the string literal.  */
> -      string_literal = cp_parser_string_literal (parser, false, false);
> +      tree string_literal = cp_parser_string_literal (parser,
> +						      /*translate=*/false,
> +						      /*wide_ok=*/false,
> +						      /*udl_ok=*/false);
>         /* Add it to the list.  */
>         clobbers = tree_cons (NULL_TREE, string_literal, clobbers);
>         /* If the next token is not a `,', then the list is
> @@ -45761,7 +45770,10 @@ cp_parser_omp_context_selector (cp_parser *parser, tree set, bool has_parms_p)
>   		      cp_lexer_consume_token (parser->lexer);
>   		    }
>   		  else if (cp_lexer_next_token_is (parser->lexer, CPP_STRING))
> -		    value = cp_parser_string_literal (parser, false, false);
> +		    value = cp_parser_string_literal (parser,
> +						      /*translate=*/false,
> +						      /*wide_ok=*/false,
> +						      /*udl_ok=*/false);
>   		  else
>   		    {
>   		      cp_parser_error (parser, "expected identifier or "
> @@ -48783,7 +48795,8 @@ pragma_lex (tree *value, location_t *loc)
>     if (ret == CPP_PRAGMA_EOL)
>       ret = CPP_EOF;
>     else if (ret == CPP_STRING)
> -    *value = cp_parser_string_literal (the_parser, false, false);
> +    *value = cp_parser_string_literal (the_parser, /*translate=*/false,
> +				       /*wide_ok=*/false, /*udl_ok=*/false);
>     else
>       {
>         if (ret == CPP_KEYWORD)
> diff --git a/gcc/testsuite/g++.dg/cpp0x/udlit-error1.C b/gcc/testsuite/g++.dg/cpp0x/udlit-error1.C
> new file mode 100644
> index 00000000000..66e300e350f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/udlit-error1.C
> @@ -0,0 +1,21 @@
> +// PR c++/105300
> +// { dg-do compile { target c++11 } }
> +
> +void operator""_x(const char *, decltype(sizeof(0)));
> +
> +#include ""_x		  // { dg-error "include expects" }
> +#line ""_x		  // { dg-error "not a positive integer" }
> +#if __has_include(""_x)	  // { dg-error "requires a header-name" }
> +#endif
> +
> +#pragma message "hi"_x	  // { dg-warning "string literal with user-defined suffix is invalid in this context" }
> +
> +extern "C"_x { void g(); } // { dg-error "before user-defined string literal" }
> +static_assert(true, "foo"_x); // { dg-error "string literal with user-defined suffix is invalid in this context|expected" }
> +
> +[[deprecated("oof"_x)]]
> +void
> +lol () // { dg-error "not a string" }
> +{
> +  asm (""_x); // { dg-error "string literal with user-defined suffix is invalid in this context" }
> +}
> 
> base-commit: f232715d15618e91c90eb210e23de10909590944


  reply	other threads:[~2022-11-15 23:58 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-12 16:53 Marek Polacek
2022-11-15 23:58 ` Jason Merrill [this message]
2022-11-16  0:35   ` Marek Polacek
2022-11-16 13:22     ` Jason Merrill
2022-11-17  1:12       ` Marek Polacek
2022-11-18  0:06         ` Jason Merrill
2022-11-18 23:52           ` [PATCH v2] " Marek Polacek
2022-11-19  1:39             ` Jason Merrill
2022-12-02 23:58               ` [PATCH v3] " Marek Polacek
2022-12-03 19:58                 ` Jason Merrill
2023-01-13 23:22                   ` [PATCH v4] " Marek Polacek
2023-01-24 22:49                     ` Marek Polacek
2023-01-25 19:36                     ` Jason Merrill

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=f532848a-bb39-0032-a625-4c4365de610f@redhat.com \
    --to=jason@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=polacek@redhat.com \
    /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).