Re: [PATCH] libcpp: Fix macro expansion for argument of __has_include [PR110558]

[Joseph Myers]

On Tue, 12 Dec 2023, Lewis Hyatt wrote:

> When the file name for a #include directive is the result of stringifying a
> macro argument, libcpp needs to take some care to get the whitespace
> correct; in particular stringify_arg() needs to see a CPP_PADDING token
> between macro tokens so that it can figure out when to output space between
> tokens. The CPP_PADDING tokens are not normally generated when handling a
> preprocessor directive, but for #include-like directives, libcpp sets the
> state variable pfile->state.directive_wants_padding to TRUE so that the
> CPP_PADDING tokens will be output, and then everything works fine for
> computed includes.
> 
> As the PR points out, things do not work fine for __has_include. Fix that by
> setting the state variable the same as is done for #include.
> 
> libcpp/ChangeLog:
> 
>   PR preprocessor/110558
>   * macro.cc (builtin_has_include): Set
>   pfile->state.directive_wants_padding prior to lexing the
>   file name, in case it comes from macro expansion.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR preprocessor/110558
>   * c-c++-common/cpp/has-include-2.c: New test.
>   * c-c++-common/cpp/has-include-2.h: New test.

OK.

-- 
Joseph S. Myers
josmy...@redhat.com

ping: [PATCH] libcpp: Fix macro expansion for argument of __has_include [PR110558]

[Lewis Hyatt]

Hello-

May I please ping this one? Thanks...

https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640386.html

-Lewis

On Tue, Dec 12, 2023 at 6:18 PM Lewis Hyatt  wrote:
>
> Hello-
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110558
>
> This is a small fix for the libcpp issue noted in the PR. Bootstrap +
> regtest all languages on x86-64 Linux. Is it ok for trunk please?
>
> Also, it's not a regression, having never worked since __has_include was
> introduced in GCC 5, but FWIW the fix would backport fine to all branches
> since then... so I think backport to 11,12,13 would make sense assuming the
> patch is OK. Thanks!
>
> -Lewis
>
> -- >8 --
>
> When the file name for a #include directive is the result of stringifying a
> macro argument, libcpp needs to take some care to get the whitespace
> correct; in particular stringify_arg() needs to see a CPP_PADDING token
> between macro tokens so that it can figure out when to output space between
> tokens. The CPP_PADDING tokens are not normally generated when handling a
> preprocessor directive, but for #include-like directives, libcpp sets the
> state variable pfile->state.directive_wants_padding to TRUE so that the
> CPP_PADDING tokens will be output, and then everything works fine for
> computed includes.
>
> As the PR points out, things do not work fine for __has_include. Fix that by
> setting the state variable the same as is done for #include.
>
> libcpp/ChangeLog:
>
> PR preprocessor/110558
> * macro.cc (builtin_has_include): Set
> pfile->state.directive_wants_padding prior to lexing the
> file name, in case it comes from macro expansion.
>
> gcc/testsuite/ChangeLog:
>
> PR preprocessor/110558
> * c-c++-common/cpp/has-include-2.c: New test.
> * c-c++-common/cpp/has-include-2.h: New test.
> ---
>  libcpp/macro.cc|  3 +++
>  gcc/testsuite/c-c++-common/cpp/has-include-2.c | 12 
>  gcc/testsuite/c-c++-common/cpp/has-include-2.h |  1 +
>  3 files changed, 16 insertions(+)
>  create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-2.c
>  create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-2.h
>
> diff --git a/libcpp/macro.cc b/libcpp/macro.cc
> index 6f24a9d6f3a..15140c60023 100644
> --- a/libcpp/macro.cc
> +++ b/libcpp/macro.cc
> @@ -398,6 +398,8 @@ builtin_has_include (cpp_reader *pfile, cpp_hashnode *op, 
> bool has_next)
>NODE_NAME (op));
>
>pfile->state.angled_headers = true;
> +  const auto sav_padding = pfile->state.directive_wants_padding;
> +  pfile->state.directive_wants_padding = true;
>const cpp_token *token = cpp_get_token_no_padding (pfile);
>bool paren = token->type == CPP_OPEN_PAREN;
>if (paren)
> @@ -406,6 +408,7 @@ builtin_has_include (cpp_reader *pfile, cpp_hashnode *op, 
> bool has_next)
>  cpp_error (pfile, CPP_DL_ERROR,
>"missing '(' before \"%s\" operand", NODE_NAME (op));
>pfile->state.angled_headers = false;
> +  pfile->state.directive_wants_padding = sav_padding;
>
>bool bracket = token->type != CPP_STRING;
>char *fname = NULL;
> diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-2.c 
> b/gcc/testsuite/c-c++-common/cpp/has-include-2.c
> new file mode 100644
> index 000..5cd00cb3fb5
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/cpp/has-include-2.c
> @@ -0,0 +1,12 @@
> +/* PR preprocessor/110558 */
> +/* { dg-do preprocess } */
> +#define STRINGIZE(x) #x
> +#define GET_INCLUDE(i) STRINGIZE(has-include-i.h)
> +/* Spaces surrounding the macro args previously caused a problem for 
> __has_include().  */
> +#if __has_include(GET_INCLUDE(2)) && __has_include(GET_INCLUDE( 2)) && 
> __has_include(GET_INCLUDE( 2 ))
> +#include GET_INCLUDE(2)
> +#include GET_INCLUDE( 2)
> +#include GET_INCLUDE( 2 )
> +#else
> +#error "__has_include did not handle padding properly" /* { dg-bogus 
> "__has_include" } */
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-2.h 
> b/gcc/testsuite/c-c++-common/cpp/has-include-2.h
> new file mode 100644
> index 000..57c402b32a8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/cpp/has-include-2.h
> @@ -0,0 +1 @@
> +/* PR preprocessor/110558 */

[PATCH] libcpp: Fix macro expansion for argument of __has_include [PR110558]

[Lewis Hyatt]

Hello-

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110558

This is a small fix for the libcpp issue noted in the PR. Bootstrap +
regtest all languages on x86-64 Linux. Is it ok for trunk please?

Also, it's not a regression, having never worked since __has_include was
introduced in GCC 5, but FWIW the fix would backport fine to all branches
since then... so I think backport to 11,12,13 would make sense assuming the
patch is OK. Thanks!

-Lewis

-- >8 --

When the file name for a #include directive is the result of stringifying a
macro argument, libcpp needs to take some care to get the whitespace
correct; in particular stringify_arg() needs to see a CPP_PADDING token
between macro tokens so that it can figure out when to output space between
tokens. The CPP_PADDING tokens are not normally generated when handling a
preprocessor directive, but for #include-like directives, libcpp sets the
state variable pfile->state.directive_wants_padding to TRUE so that the
CPP_PADDING tokens will be output, and then everything works fine for
computed includes.

As the PR points out, things do not work fine for __has_include. Fix that by
setting the state variable the same as is done for #include.

libcpp/ChangeLog:

PR preprocessor/110558
* macro.cc (builtin_has_include): Set
pfile->state.directive_wants_padding prior to lexing the
file name, in case it comes from macro expansion.

gcc/testsuite/ChangeLog:

PR preprocessor/110558
* c-c++-common/cpp/has-include-2.c: New test.
* c-c++-common/cpp/has-include-2.h: New test.
---
 libcpp/macro.cc|  3 +++
 gcc/testsuite/c-c++-common/cpp/has-include-2.c | 12 
 gcc/testsuite/c-c++-common/cpp/has-include-2.h |  1 +
 3 files changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-2.c
 create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-2.h

diff --git a/libcpp/macro.cc b/libcpp/macro.cc
index 6f24a9d6f3a..15140c60023 100644
--- a/libcpp/macro.cc
+++ b/libcpp/macro.cc
@@ -398,6 +398,8 @@ builtin_has_include (cpp_reader *pfile, cpp_hashnode *op, 
bool has_next)
   NODE_NAME (op));
 
   pfile->state.angled_headers = true;
+  const auto sav_padding = pfile->state.directive_wants_padding;
+  pfile->state.directive_wants_padding = true;
   const cpp_token *token = cpp_get_token_no_padding (pfile);
   bool paren = token->type == CPP_OPEN_PAREN;
   if (paren)
@@ -406,6 +408,7 @@ builtin_has_include (cpp_reader *pfile, cpp_hashnode *op, 
bool has_next)
 cpp_error (pfile, CPP_DL_ERROR,
   "missing '(' before \"%s\" operand", NODE_NAME (op));
   pfile->state.angled_headers = false;
+  pfile->state.directive_wants_padding = sav_padding;
 
   bool bracket = token->type != CPP_STRING;
   char *fname = NULL;
diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-2.c 
b/gcc/testsuite/c-c++-common/cpp/has-include-2.c
new file mode 100644
index 000..5cd00cb3fb5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/has-include-2.c
@@ -0,0 +1,12 @@
+/* PR preprocessor/110558 */
+/* { dg-do preprocess } */
+#define STRINGIZE(x) #x
+#define GET_INCLUDE(i) STRINGIZE(has-include-i.h)
+/* Spaces surrounding the macro args previously caused a problem for 
__has_include().  */
+#if __has_include(GET_INCLUDE(2)) && __has_include(GET_INCLUDE( 2)) && 
__has_include(GET_INCLUDE( 2 ))
+#include GET_INCLUDE(2)
+#include GET_INCLUDE( 2)
+#include GET_INCLUDE( 2 )
+#else
+#error "__has_include did not handle padding properly" /* { dg-bogus 
"__has_include" } */
+#endif
diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-2.h 
b/gcc/testsuite/c-c++-common/cpp/has-include-2.h
new file mode 100644
index 000..57c402b32a8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/has-include-2.h
@@ -0,0 +1 @@
+/* PR preprocessor/110558 */