On 1/30/24 21:49, Lewis Hyatt wrote:
On Fri, Jan 26, 2024 at 04:16:54PM -0500, Jason Merrill wrote:
On 12/5/23 20:52, Lewis Hyatt wrote:
Hello-
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608
There are two related issues here really, a regression since GCC 11 where we
can ICE after restoring a PCH, and a deeper issue with bogus locations
assigned to macros that were defined prior to restoring a PCH. This patch
fixes the ICE regression with a simple change, and I think it's appropriate
for GCC 14 as well as backport to 11, 12, 13. The bad locations (wrong, but
not generally causing an ICE, and mostly affecting only the output of
-Wunused-macros) are not as problematic, and will be harder to fix. I could
take a stab at that for GCC 15. In the meantime the patch adds XFAILed
tests for the wrong locations (as well as passing tests for the regression
fix). Does it look OK please? Bootstrap + regtest all languages on x86-64
Linux. Thanks!
OK for trunk and branches, thanks!
Thanks for the review! That is all taken care of. I have one more request if
you don't mind please... There have been some further comments on the PR
indicating that the new xfailed testcase I added is failing in an unexpected
way on at least one architecture. To recap, the idea here was that
1) libcpp needs new logic to be able to output correct locations for this
case. That will be some new code that is suitable for stage 1, not now.
2) In the meantime, we fixed things up enough to avoid an ICE that showed up
in GCC 11, and added an xfailed testcase to remind about #1.
The problem is that, the reason that libcpp outputs the wrong locations, is
that it has always used a location from the old line_map instance to index
into the new line_map instance, and so the exact details of the wrong
locations it outputs depend on the state of those two line maps, which may
differ depending on system includes and things like that. So I was hoping to
make one further one-line change to libcpp, not yet to output correct
locations, but at least to output one which is the same always and doesn't
depend on random things. This would assign all restored macros to a
consistent location, one line following the #include that triggered the PCH
process. I think this probably shouldn't be backported but it would be nice
to get into GCC 14, while nothing critical, at least it would avoid the new
test failure that's being reported. But more generally, I think using a
location from a totally different line map is dangerous and could have worse
consequences that haven't been seen yet. Does it look OK please? Thanks!
Can we use the line of the #include, as the test expects, rather than
the following line?
Jason
