libcpp: Fix __has_include_next ICE in the last directory of the path [PR80755]

Message ID 20231221123750.2303405-1-lhyatt@gmail.com
State Accepted
Headers
Series libcpp: Fix __has_include_next ICE in the last directory of the path [PR80755] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Lewis Hyatt Dec. 21, 2023, 12:37 p.m. UTC
  Hello-

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

Here is a short fix for the ICE in libcpp noted in the PR. Bootstrap +
regtest all languages on x86-64 Linux. Is it OK please? Thanks!

-Lewis

-- >8 --

In libcpp/files.cc, the function _cpp_has_header(), which implements
__has_include and __has_include_next, does not check for a NULL return value
from search_path_head(), leading to an ICE tripping an assert when
_cpp_find_file() tries to use it. Fix it by checking for that case and
silently returning false instead.

As suggested by the PR author, it is easiest to make a testcase by using
the -idirafter option. To enable that, also modify the dg-additional-options
testsuite procedure to make the global $srcdir available, since -idirafter
requires the full path.

libcpp/ChangeLog:

	PR preprocessor/80755
	* files.cc (search_path_head): Add SUPPRESS_DIAGNOSTIC argument
	defaulting to false.
	(_cpp_has_header): Silently return false if the search path has been
	exhausted, rather than issuing a diagnostic and then hitting an
	assert.

gcc/testsuite/ChangeLog:

	* lib/gcc-defs.exp (dg-additional-options): Make $srcdir usable in a
	dg-additional-options directive.
	* c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h: New test.
	* c-c++-common/cpp/has-include-next-2.c: New test.
---
 libcpp/files.cc                                      | 12 ++++++++----
 .../cpp/has-include-next-2-dir/has-include-next-2.h  |  3 +++
 gcc/testsuite/c-c++-common/cpp/has-include-next-2.c  |  4 ++++
 gcc/testsuite/lib/gcc-defs.exp                       |  1 +
 4 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
 create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
  

Comments

Lewis Hyatt Jan. 11, 2024, 12:34 p.m. UTC | #1
Can I please ping this one? Thanks...
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/641247.html

-Lewis

On Thu, Dec 21, 2023 at 7:37 AM Lewis Hyatt <lhyatt@gmail.com> wrote:
>
> Hello-
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80755
>
> Here is a short fix for the ICE in libcpp noted in the PR. Bootstrap +
> regtest all languages on x86-64 Linux. Is it OK please? Thanks!
>
> -Lewis
>
> -- >8 --
>
> In libcpp/files.cc, the function _cpp_has_header(), which implements
> __has_include and __has_include_next, does not check for a NULL return value
> from search_path_head(), leading to an ICE tripping an assert when
> _cpp_find_file() tries to use it. Fix it by checking for that case and
> silently returning false instead.
>
> As suggested by the PR author, it is easiest to make a testcase by using
> the -idirafter option. To enable that, also modify the dg-additional-options
> testsuite procedure to make the global $srcdir available, since -idirafter
> requires the full path.
>
> libcpp/ChangeLog:
>
>         PR preprocessor/80755
>         * files.cc (search_path_head): Add SUPPRESS_DIAGNOSTIC argument
>         defaulting to false.
>         (_cpp_has_header): Silently return false if the search path has been
>         exhausted, rather than issuing a diagnostic and then hitting an
>         assert.
>
> gcc/testsuite/ChangeLog:
>
>         * lib/gcc-defs.exp (dg-additional-options): Make $srcdir usable in a
>         dg-additional-options directive.
>         * c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h: New test.
>         * c-c++-common/cpp/has-include-next-2.c: New test.
> ---
>  libcpp/files.cc                                      | 12 ++++++++----
>  .../cpp/has-include-next-2-dir/has-include-next-2.h  |  3 +++
>  gcc/testsuite/c-c++-common/cpp/has-include-next-2.c  |  4 ++++
>  gcc/testsuite/lib/gcc-defs.exp                       |  1 +
>  4 files changed, 16 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
>  create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
>
> diff --git a/libcpp/files.cc b/libcpp/files.cc
> index 27301d79fa4..aaab4b13a6a 100644
> --- a/libcpp/files.cc
> +++ b/libcpp/files.cc
> @@ -181,7 +181,8 @@ static bool read_file_guts (cpp_reader *pfile, _cpp_file *file,
>  static bool read_file (cpp_reader *pfile, _cpp_file *file,
>                        location_t loc);
>  static struct cpp_dir *search_path_head (cpp_reader *, const char *fname,
> -                                int angle_brackets, enum include_type);
> +                                        int angle_brackets, enum include_type,
> +                                        bool suppress_diagnostic = false);
>  static const char *dir_name_of_file (_cpp_file *file);
>  static void open_file_failed (cpp_reader *pfile, _cpp_file *file, int,
>                               location_t);
> @@ -1041,7 +1042,7 @@ _cpp_mark_file_once_only (cpp_reader *pfile, _cpp_file *file)
>     nothing left in the path, returns NULL.  */
>  static struct cpp_dir *
>  search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
> -                 enum include_type type)
> +                 enum include_type type, bool suppress_diagnostic)
>  {
>    cpp_dir *dir;
>    _cpp_file *file;
> @@ -1070,7 +1071,7 @@ search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
>      return make_cpp_dir (pfile, dir_name_of_file (file),
>                          pfile->buffer ? pfile->buffer->sysp : 0);
>
> -  if (dir == NULL)
> +  if (dir == NULL && !suppress_diagnostic)
>      cpp_error (pfile, CPP_DL_ERROR,
>                "no include path in which to search for %s", fname);
>
> @@ -2164,7 +2165,10 @@ bool
>  _cpp_has_header (cpp_reader *pfile, const char *fname, int angle_brackets,
>                  enum include_type type)
>  {
> -  cpp_dir *start_dir = search_path_head (pfile, fname, angle_brackets, type);
> +  cpp_dir *start_dir = search_path_head (pfile, fname, angle_brackets, type,
> +                                        /* suppress_diagnostic = */ true);
> +  if (!start_dir)
> +    return false;
>    _cpp_file *file = _cpp_find_file (pfile, fname, start_dir, angle_brackets,
>                                     _cpp_FFK_HAS_INCLUDE, 0);
>    return file->err_no != ENOENT;
> diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h b/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
> new file mode 100644
> index 00000000000..1e4be6ce7a3
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
> @@ -0,0 +1,3 @@
> +#if __has_include_next(<whatever>)
> +/* This formerly led to an ICE when the current directory was the last one in the path.  */
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c b/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
> new file mode 100644
> index 00000000000..4928d3e992c
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
> @@ -0,0 +1,4 @@
> +/* PR preprocessor/80755 */
> +/* { dg-do preprocess } */
> +/* { dg-additional-options "-idirafter $srcdir/c-c++-common/cpp/has-include-next-2-dir" } */
> +#include <has-include-next-2.h>
> diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp
> index fc569c18ad5..091520ff69e 100644
> --- a/gcc/testsuite/lib/gcc-defs.exp
> +++ b/gcc/testsuite/lib/gcc-defs.exp
> @@ -280,6 +280,7 @@ if { [info exists env(GCC_RUNTEST_PARALLELIZE_DIR)] \
>
>  proc dg-additional-options { args } {
>      upvar dg-extra-tool-flags extra-tool-flags
> +    global srcdir
>
>      if { [llength $args] > 3 } {
>         error "[lindex $args 0]: too many arguments"
  
Lewis Hyatt Feb. 28, 2024, 12:03 a.m. UTC | #2
Hello-

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

There was a request on the PR
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80755#c5) for me to ping
this again, so I am complying :). Might anyone have a minute to take a
look please? Thanks...


-Lewis


On Thu, Jan 11, 2024 at 7:34 AM Lewis Hyatt <lhyatt@gmail.com> wrote:
>
> Can I please ping this one? Thanks...
> https://gcc.gnu.org/pipermail/gcc-patches/2023-December/641247.html
>
> -Lewis
>
> On Thu, Dec 21, 2023 at 7:37 AM Lewis Hyatt <lhyatt@gmail.com> wrote:
> >
> > Hello-
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80755
> >
> > Here is a short fix for the ICE in libcpp noted in the PR. Bootstrap +
> > regtest all languages on x86-64 Linux. Is it OK please? Thanks!
> >
> > -Lewis
> >
> > -- >8 --
> >
> > In libcpp/files.cc, the function _cpp_has_header(), which implements
> > __has_include and __has_include_next, does not check for a NULL return value
> > from search_path_head(), leading to an ICE tripping an assert when
> > _cpp_find_file() tries to use it. Fix it by checking for that case and
> > silently returning false instead.
> >
> > As suggested by the PR author, it is easiest to make a testcase by using
> > the -idirafter option. To enable that, also modify the dg-additional-options
> > testsuite procedure to make the global $srcdir available, since -idirafter
> > requires the full path.
> >
> > libcpp/ChangeLog:
> >
> >         PR preprocessor/80755
> >         * files.cc (search_path_head): Add SUPPRESS_DIAGNOSTIC argument
> >         defaulting to false.
> >         (_cpp_has_header): Silently return false if the search path has been
> >         exhausted, rather than issuing a diagnostic and then hitting an
> >         assert.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * lib/gcc-defs.exp (dg-additional-options): Make $srcdir usable in a
> >         dg-additional-options directive.
> >         * c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h: New test.
> >         * c-c++-common/cpp/has-include-next-2.c: New test.
> > ---
> >  libcpp/files.cc                                      | 12 ++++++++----
> >  .../cpp/has-include-next-2-dir/has-include-next-2.h  |  3 +++
> >  gcc/testsuite/c-c++-common/cpp/has-include-next-2.c  |  4 ++++
> >  gcc/testsuite/lib/gcc-defs.exp                       |  1 +
> >  4 files changed, 16 insertions(+), 4 deletions(-)
> >  create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
> >  create mode 100644 gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
> >
> > diff --git a/libcpp/files.cc b/libcpp/files.cc
> > index 27301d79fa4..aaab4b13a6a 100644
> > --- a/libcpp/files.cc
> > +++ b/libcpp/files.cc
> > @@ -181,7 +181,8 @@ static bool read_file_guts (cpp_reader *pfile, _cpp_file *file,
> >  static bool read_file (cpp_reader *pfile, _cpp_file *file,
> >                        location_t loc);
> >  static struct cpp_dir *search_path_head (cpp_reader *, const char *fname,
> > -                                int angle_brackets, enum include_type);
> > +                                        int angle_brackets, enum include_type,
> > +                                        bool suppress_diagnostic = false);
> >  static const char *dir_name_of_file (_cpp_file *file);
> >  static void open_file_failed (cpp_reader *pfile, _cpp_file *file, int,
> >                               location_t);
> > @@ -1041,7 +1042,7 @@ _cpp_mark_file_once_only (cpp_reader *pfile, _cpp_file *file)
> >     nothing left in the path, returns NULL.  */
> >  static struct cpp_dir *
> >  search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
> > -                 enum include_type type)
> > +                 enum include_type type, bool suppress_diagnostic)
> >  {
> >    cpp_dir *dir;
> >    _cpp_file *file;
> > @@ -1070,7 +1071,7 @@ search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
> >      return make_cpp_dir (pfile, dir_name_of_file (file),
> >                          pfile->buffer ? pfile->buffer->sysp : 0);
> >
> > -  if (dir == NULL)
> > +  if (dir == NULL && !suppress_diagnostic)
> >      cpp_error (pfile, CPP_DL_ERROR,
> >                "no include path in which to search for %s", fname);
> >
> > @@ -2164,7 +2165,10 @@ bool
> >  _cpp_has_header (cpp_reader *pfile, const char *fname, int angle_brackets,
> >                  enum include_type type)
> >  {
> > -  cpp_dir *start_dir = search_path_head (pfile, fname, angle_brackets, type);
> > +  cpp_dir *start_dir = search_path_head (pfile, fname, angle_brackets, type,
> > +                                        /* suppress_diagnostic = */ true);
> > +  if (!start_dir)
> > +    return false;
> >    _cpp_file *file = _cpp_find_file (pfile, fname, start_dir, angle_brackets,
> >                                     _cpp_FFK_HAS_INCLUDE, 0);
> >    return file->err_no != ENOENT;
> > diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h b/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
> > new file mode 100644
> > index 00000000000..1e4be6ce7a3
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
> > @@ -0,0 +1,3 @@
> > +#if __has_include_next(<whatever>)
> > +/* This formerly led to an ICE when the current directory was the last one in the path.  */
> > +#endif
> > diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c b/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
> > new file mode 100644
> > index 00000000000..4928d3e992c
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
> > @@ -0,0 +1,4 @@
> > +/* PR preprocessor/80755 */
> > +/* { dg-do preprocess } */
> > +/* { dg-additional-options "-idirafter $srcdir/c-c++-common/cpp/has-include-next-2-dir" } */
> > +#include <has-include-next-2.h>
> > diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp
> > index fc569c18ad5..091520ff69e 100644
> > --- a/gcc/testsuite/lib/gcc-defs.exp
> > +++ b/gcc/testsuite/lib/gcc-defs.exp
> > @@ -280,6 +280,7 @@ if { [info exists env(GCC_RUNTEST_PARALLELIZE_DIR)] \
> >
> >  proc dg-additional-options { args } {
> >      upvar dg-extra-tool-flags extra-tool-flags
> > +    global srcdir
> >
> >      if { [llength $args] > 3 } {
> >         error "[lindex $args 0]: too many arguments"
  

Patch

diff --git a/libcpp/files.cc b/libcpp/files.cc
index 27301d79fa4..aaab4b13a6a 100644
--- a/libcpp/files.cc
+++ b/libcpp/files.cc
@@ -181,7 +181,8 @@  static bool read_file_guts (cpp_reader *pfile, _cpp_file *file,
 static bool read_file (cpp_reader *pfile, _cpp_file *file,
 		       location_t loc);
 static struct cpp_dir *search_path_head (cpp_reader *, const char *fname,
-				 int angle_brackets, enum include_type);
+					 int angle_brackets, enum include_type,
+					 bool suppress_diagnostic = false);
 static const char *dir_name_of_file (_cpp_file *file);
 static void open_file_failed (cpp_reader *pfile, _cpp_file *file, int,
 			      location_t);
@@ -1041,7 +1042,7 @@  _cpp_mark_file_once_only (cpp_reader *pfile, _cpp_file *file)
    nothing left in the path, returns NULL.  */
 static struct cpp_dir *
 search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
-		  enum include_type type)
+		  enum include_type type, bool suppress_diagnostic)
 {
   cpp_dir *dir;
   _cpp_file *file;
@@ -1070,7 +1071,7 @@  search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
     return make_cpp_dir (pfile, dir_name_of_file (file),
 			 pfile->buffer ? pfile->buffer->sysp : 0);
 
-  if (dir == NULL)
+  if (dir == NULL && !suppress_diagnostic)
     cpp_error (pfile, CPP_DL_ERROR,
 	       "no include path in which to search for %s", fname);
 
@@ -2164,7 +2165,10 @@  bool
 _cpp_has_header (cpp_reader *pfile, const char *fname, int angle_brackets,
 		 enum include_type type)
 {
-  cpp_dir *start_dir = search_path_head (pfile, fname, angle_brackets, type);
+  cpp_dir *start_dir = search_path_head (pfile, fname, angle_brackets, type,
+					 /* suppress_diagnostic = */ true);
+  if (!start_dir)
+    return false;
   _cpp_file *file = _cpp_find_file (pfile, fname, start_dir, angle_brackets,
 				    _cpp_FFK_HAS_INCLUDE, 0);
   return file->err_no != ENOENT;
diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h b/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
new file mode 100644
index 00000000000..1e4be6ce7a3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/has-include-next-2-dir/has-include-next-2.h
@@ -0,0 +1,3 @@ 
+#if __has_include_next(<whatever>)
+/* This formerly led to an ICE when the current directory was the last one in the path.  */
+#endif
diff --git a/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c b/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
new file mode 100644
index 00000000000..4928d3e992c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/has-include-next-2.c
@@ -0,0 +1,4 @@ 
+/* PR preprocessor/80755 */
+/* { dg-do preprocess } */
+/* { dg-additional-options "-idirafter $srcdir/c-c++-common/cpp/has-include-next-2-dir" } */
+#include <has-include-next-2.h>
diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp
index fc569c18ad5..091520ff69e 100644
--- a/gcc/testsuite/lib/gcc-defs.exp
+++ b/gcc/testsuite/lib/gcc-defs.exp
@@ -280,6 +280,7 @@  if { [info exists env(GCC_RUNTEST_PARALLELIZE_DIR)] \
 
 proc dg-additional-options { args } {
     upvar dg-extra-tool-flags extra-tool-flags
+    global srcdir
 
     if { [llength $args] > 3 } {
 	error "[lindex $args 0]: too many arguments"