From: Jakub Jelinek <jakub@redhat.com>
To: "Joseph S. Myers" <joseph@codesourcery.com>,
Marek Polacek <polacek@redhat.com>,
Jason Merrill <jason@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] libcpp: Fix up padding handling in funlike_invocation_p [PR104147]
Date: Mon, 31 Jan 2022 20:03:49 +0100 [thread overview]
Message-ID: <20220131190349.GM2646553@tucnak> (raw)
Hi!
As mentioned in the PR, in some cases we preprocess incorrectly when we
encounter an identifier which is defined as function-like macro, followed
by at least 2 CPP_PADDING tokens and then some other identifier.
On the following testcase, the problem is in the 3rd funlike_invocation_p,
the tokens are CPP_NAME Y, CPP_PADDING (the pfile->avoid_paste shared token),
CPP_PADDING (one created with padding_token, val.source is non-NULL and
val.source->flags & PREV_WHITE is non-zero) and then another CPP_NAME.
funlike_invocation_p remembers there was a padding token, but remembers the
first one because of its condition, then the next token is the CPP_NAME,
which is not CPP_OPEN_PAREN, so the CPP_NAME token is backed up, but as we
can't easily backup more tokens, it pushes into a new context the padding
token (the pfile->avoid_paste one). The net effect is that when Y is not
defined as fun-like macro, we read Y, avoid_paste, padding_token, Y,
while if Y is fun-like macro, we read Y, avoid_paste, avoid_paste, Y
(the second avoid_paste is because that is how we handle end of a context).
Now, for stringify_arg that is unfortunately a significant difference,
which handles CPP_PADDING tokens with:
if (token->type == CPP_PADDING)
{
if (source == NULL
|| (!(source->flags & PREV_WHITE)
&& token->val.source == NULL))
source = token->val.source;
continue;
}
and later on
/* Leading white space? */
if (dest - 1 != BUFF_FRONT (pfile->u_buff))
{
if (source == NULL)
source = token;
if (source->flags & PREV_WHITE)
*dest++ = ' ';
}
source = NULL;
(and c-ppoutput.cc has similar code).
So, when Y is not fun-like macro, ' ' is added because padding_token's
val.source->flags & PREV_WHITE is non-zero, while when it is fun-like
macro, we don't add ' ' in between, because source is NULL and so
used from the next token (CPP_NAME Y), which doesn't have PREV_WHITE set.
Now, the funlike_invocation_p condition
if (padding == NULL
|| (!(padding->flags & PREV_WHITE) && token->val.source == NULL))
padding = token;
looks very similar to that in stringify_arg/c-ppoutput.cc, so I assume
the intent was to prefer do the same thing and pick the right padding.
But there are significant differences. Both stringify_arg and c-ppoutput.cc
don't remember the CPP_PADDING token, but its val.source instead, while
in funlike_invocation_p we want to remember the padding token that has the
significant information for stringify_arg/c-ppoutput.cc.
So, IMHO we want to overwrite padding if:
1) padding == NULL (remember that there was any padding at all)
2) padding->val.source == NULL (this matches the source == NULL
case in stringify_arg)
3) !(padding->val.source->flags & PREV_WHITE) && token->val.source == NULL
(this matches the !(source->flags & PREV_WHITE) && token->val.source == NULL
case in stringify_arg)
Bootstrapped/regtested on powerpc64le-linux, ok for trunk
(and after a while for some release branches)?
2022-01-31 Jakub Jelinek <jakub@redhat.com>
PR preprocessor/104147
* macro.cc (funlike_invocation_p): For padding prefer a token
with val.source non-NULL especially if it has PREV_WHITE set
on val.source->flags.
* c-c++-common/cpp/pr104147.c: New test.
--- libcpp/macro.cc.jj 2022-01-18 11:59:00.277972128 +0100
+++ libcpp/macro.cc 2022-01-31 15:44:49.208847524 +0100
@@ -1374,7 +1374,9 @@ funlike_invocation_p (cpp_reader *pfile,
if (token->type != CPP_PADDING)
break;
if (padding == NULL
- || (!(padding->flags & PREV_WHITE) && token->val.source == NULL))
+ || padding->val.source == NULL
+ || (!(padding->val.source->flags & PREV_WHITE)
+ && token->val.source == NULL))
padding = token;
}
--- gcc/testsuite/c-c++-common/cpp/pr104147.c.jj 2022-01-31 15:54:18.113847934 +0100
+++ gcc/testsuite/c-c++-common/cpp/pr104147.c 2022-01-31 15:54:04.530038941 +0100
@@ -0,0 +1,27 @@
+/* PR preprocessor/104147 */
+/* { dg-do run } */
+
+#define X(x,y) x y
+#define STR_(x) #x
+#define STR(x) STR_(x)
+const char *str =
+STR(X(Y,Y))
+#define Y()
+STR(X(Y,Y))
+#undef Y
+STR(X(Y,Y))
+#define Y()
+STR(X(Y,Y))
+STR(X(Y,
+Y))
+STR(X(Y
+,Y))
+;
+
+int
+main ()
+{
+ if (__builtin_strcmp (str, "Y YY YY YY YY YY Y") != 0)
+ __builtin_abort ();
+ return 0;
+}
Jakub
next reply other threads:[~2022-01-31 19:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-31 19:03 Jakub Jelinek [this message]
2022-01-31 22:12 ` Jason Merrill
2022-01-31 23:28 ` Jakub Jelinek
2022-02-01 9:03 ` Jakub Jelinek
2022-02-01 15:31 ` [PATCH] libcpp: Avoid PREV_WHITE and other random content on CPP_PADDING tokens Jakub Jelinek
2022-02-01 19:26 ` 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=20220131190349.GM2646553@tucnak \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=joseph@codesourcery.com \
--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).