On Thu, Feb 1, 2024 at 7:24 AM Rainer Orth <r...@cebitec.uni-bielefeld.de> wrote: > > Hi Lewis, > > > 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! > > FWIW, I've tested this (the initial) version of this patch on > sparc-sun-solaris2.11 (PASSes as before) and i386-pc-solaris2.11 (PASSes > now unlike before). > > Thanks. > Rainer
Thanks a lot! And sorry for that issue. I will push the updated version of that patch shortly.
