* [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors
[not found] <bug-80753-4@http.gcc.gnu.org/bugzilla/>
@ 2020-09-23 14:24 ` matthijs at stdin dot nl
2020-09-23 16:00 ` matthijs at stdin dot nl
` (4 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: matthijs at stdin dot nl @ 2020-09-23 14:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80753
Matthijs Kooijman <matthijs at stdin dot nl> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |matthijs at stdin dot nl
--- Comment #3 from Matthijs Kooijman <matthijs at stdin dot nl> ---
I also ran into this, this is still a problem in gcc 10.0.1.
This is a particular problem in the Arduino environment, which relies on
preprocessor error messages to automatically pick libraries to include. This
bug prevents it from detecting that a particular library is needed and from
adding it to the include path, breaking the build (that could have worked
without this bug).
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors
[not found] <bug-80753-4@http.gcc.gnu.org/bugzilla/>
2020-09-23 14:24 ` [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors matthijs at stdin dot nl
@ 2020-09-23 16:00 ` matthijs at stdin dot nl
2023-06-12 17:15 ` pinskia at gcc dot gnu.org
` (3 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: matthijs at stdin dot nl @ 2020-09-23 16:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80753
--- Comment #4 from Matthijs Kooijman <matthijs at stdin dot nl> ---
I looked a bit at the source, and it seems the the problem is (not
surprisingly) that `__has_include` causes the header filename to be put into
the cache, and an error message is only generated when the entire include path
has been processed without resolving the entry from the cache (essentially this
means that an error is only triggered when the entry is put into the cache,
*except* when it happens as part of `__has_include`).
Relevant function is _cpp_find_file:
https://github.com/gcc-mirror/gcc/blob/da13b7737662da11f8fefb28eaf4ed7c50c51767/libcpp/files.c#L506
This is called with kind = _cpp_FFK_HAS_INCLUDE for `__has_include` which
prevents an error here:
https://github.com/gcc-mirror/gcc/blob/da13b7737662da11f8fefb28eaf4ed7c50c51767/libcpp/files.c#L591-L592
And the cache entry is immediately returned on subsequent calls here:
https://github.com/gcc-mirror/gcc/blob/da13b7737662da11f8fefb28eaf4ed7c50c51767/libcpp/files.c#L523-L526
It seems there are continuous lookups in the cache and it took me a while to
realize how the cache actually works (I initially thought that maybe files that
were *not* found) were not actually put in the cache, but AFAIU the
`pfile->file_hash` cache works like this:
- Hash slots are indexed by filename
- Each slot contains a linked list of entries.
- Each entry contains the directory lookups start from. This can be the
start_dir, but also the first quote_include or bracket_include dir. Iow, the
cache entry is only valid for a lookup that starts at that directory, or has
progressed to that directory, to allow a "" include to prime the cache for a <>
include as well.
- Once a file is found, or the search path is exhausted, the result is stored
in the cache for start_dir and for "" and <> includes if the start dir for
those has been passed.
Anyway, this means that a failed lookup based from __has_include() will cause
the failed result to be put into the cache for one or more directories without
emitting an error and always be returned for subsequent includes without
emitting an error.
An obvious fix would be to simply not put the result in the cache for
_cpp_FFK_HAS_INCLUDE, but that would be a missed cache opportunity.
An alternative would be to add a boolean "error_emitted" to each _cpp_file* in
the cache (cannot add it to the cache entry itself, since the same _cpp_file*
might end up in different cache entries), that defaults to false and is set to
true by open_file_failed. When kind is not _cpp_FFK_HAS_INCLUDE, and returning
a cache entry that has "error_emitted" set to false and has an errno call
open_file_failed to emit an error. This would require that open_file_failed to
emit the right output in this case (i.e. the cached _cpp_file* should not rely
on the context in which it was generated), but AFAICS this would be the case.
I'm not quite familiar with building and patching gcc (and also really need to
get this yak hair back to my actual work), so I probably won't be providing a
patch here. Maybe my above analysis enables someone else to do so? :-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors
[not found] <bug-80753-4@http.gcc.gnu.org/bugzilla/>
2020-09-23 14:24 ` [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors matthijs at stdin dot nl
2020-09-23 16:00 ` matthijs at stdin dot nl
@ 2023-06-12 17:15 ` pinskia at gcc dot gnu.org
2023-06-13 9:35 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-12 17:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80753
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jpegqs at gmail dot com
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110226 has been marked as a duplicate of this bug. ***
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors
[not found] <bug-80753-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2023-06-12 17:15 ` pinskia at gcc dot gnu.org
@ 2023-06-13 9:35 ` jakub at gcc dot gnu.org
2023-06-15 12:17 ` cvs-commit at gcc dot gnu.org
2023-08-01 10:43 ` jakub at gcc dot gnu.org
5 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-06-13 9:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80753
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 55316
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55316&action=edit
gcc14-pr80753.patch
Untested fix.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors
[not found] <bug-80753-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2023-06-13 9:35 ` jakub at gcc dot gnu.org
@ 2023-06-15 12:17 ` cvs-commit at gcc dot gnu.org
2023-08-01 10:43 ` jakub at gcc dot gnu.org
5 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-15 12:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80753
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:37f373e974ac0a272b0e2b24e27c08833ad43470
commit r14-1870-g37f373e974ac0a272b0e2b24e27c08833ad43470
Author: Jakub Jelinek <jakub@redhat.com>
Date: Thu Jun 15 14:16:17 2023 +0200
libcpp: Diagnose #include after failed __has_include [PR80753]
As can be seen in the testcase, we don't diagnose #include/#include_next
of a non-existent header if __has_include/__has_include_next is done for
that header first.
The problem is that we normally error the first time some header is not
found, but in the _cpp_FFK_HAS_INCLUDE case obviously don't want to
diagnose
it, just expand it to 0. And libcpp caches both successful includes and
unsuccessful ones.
The following patch fixes that by remembering that we haven't diagnosed
error when using __has_include* on it, and diagnosing it when using the
cache entry in normal mode the first time.
I think _cpp_FFK_NORMAL is the only mode in which we normally diagnose
errors, for _cpp_FFK_PRE_INCLUDE that open_file_failed isn't reached
and for _cpp_FFK_FAKE neither.
2023-06-15 Jakub Jelinek <jakub@redhat.com>
PR preprocessor/80753
libcpp/
* files.cc (struct _cpp_file): Add deferred_error bitfield.
(_cpp_find_file): When finding a file in cache with deferred_error
set in _cpp_FFK_NORMAL mode, call open_file_failed and clear the
flag.
Set deferred_error in _cpp_FFK_HAS_INCLUDE mode if open_file_failed
hasn't been called.
gcc/testsuite/
* c-c++-common/missing-header-5.c: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug preprocessor/80753] __has_include and __has_include_next taints subsequent I/O errors
[not found] <bug-80753-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2023-06-15 12:17 ` cvs-commit at gcc dot gnu.org
@ 2023-08-01 10:43 ` jakub at gcc dot gnu.org
5 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-08-01 10:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80753
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
Target Milestone|--- |14.0
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for GCC 14+. No plans to backport, this used to be an accepts-invalid
bug and the general policy is not to backport bugfixes for accepts-invalid
bugs, as we would start rejecting something that wasn't rejected before.
^ permalink raw reply [flat|nested] 6+ messages in thread