libcpp: Fix __has_include_next ICE in the last directory of the path [PR80755]
Checks
Commit Message
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
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"
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"
@@ -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;
new file mode 100644
@@ -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
new file mode 100644
@@ -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>
@@ -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"