| Message ID | 20231221123750.2303405-1-lhyatt@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | libcpp: Fix __has_include_next ICE in the last directory of the path [PR80755] | expand |
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"
On Thu, 21 Dec 2023, Lewis Hyatt wrote: > 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. OK.
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"