Hi Folks,
> On 10 Nov 2021, at 08:14, Iain Sandoe <i...@sandoe.co.uk> wrote:
>> On 9 Nov 2021, at 12:18, Jakub Jelinek via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>
>> On Tue, Nov 09, 2021 at 11:40:08AM +0000, Iain Sandoe wrote:
>>> There were two issues, of which one remains and probably affects all
>>> targets.
>>> 2. This problem remains.
This problem is also present on master without making any changes to the PCH
implementation - if one fixes up the read-in to simulate a corrupted file, cc1
hangs
(which means it’s no barrier to the revised PCH implementation)
>>> - if we try to emit a diagnostic when the PCH read-in has failed, it seems
>>> that
>>> cc1 hangs somewhere in trying to lookup line table info.
>>>
>>> - this was happening with the Darwin fixed PCH memory address because it
>>> was trying to report a fatal error in being unable to read the file (or
>>> trying to
>>> execute fancy_abort, in response to a segv).
>>
>> I guess once we:
>> /* Read in all the scalar variables. */
>> for (rt = gt_pch_scalar_rtab; *rt; rt++)
>> for (rti = *rt; rti->base != NULL; rti++)
>> if (fread (rti->base, rti->stride, 1, f) != 1)
>> fatal_error (input_location, "cannot read PCH file: %m");
>>
>> /* Read in all the global pointers, in 6 easy loops. */
>> for (rt = gt_ggc_rtab; *rt; rt++)
>> for (rti = *rt; rti->base != NULL; rti++)
>> for (i = 0; i < rti->nelt; i++)
>> if (fread ((char *)rti->base + rti->stride * i,
>> sizeof (void *), 1, f) != 1)
>> fatal_error (input_location, "cannot read PCH file: %m");
>> we overwrite the GTY(()) marked global vars including
>> extern GTY(()) class line_maps *line_table;
>> with pointers into the area we haven't mapped yet (or if the error happens
>> after that mmap but before everything is fixed up (e.g. the new relocation
>> processing), it is no wonder it doesn't work well.
indeed.
>>
>> Could we save line_table (and perhaps a few other vars) into non-GTY! copies
>> of them in ggc-common.c and instead of those fatal_error (input_location,
>> ...)
>> calls in gt_pch_restore and ggc_pch_read call fatal_pch_error (...) where
>> void
>> fatal_pch_error (const char *gmsg)
>> {
>> line_table = saved_line_table;
>> // Restore anything else that is needed for fatal_error
>> fatal_error (input_location, gmsg);
>> }
>
> That seems reasonable for the case that we call fatal_error from ggc-common,
> but
> I don’t think it will work if fancy_abort is called (for e.g. a segv) - we
> might need to
> make a local fancy_abort() as well for that specific file, perhaps.
>
> Or in some way defer overwriting the data until we’ve succeeded in
> reading/relocating
> the whole file (not sure what the largest PCH is we might encounter).
(answering my own question) around 150Mb for largest libstdc++ and similar for
an
Objective-C include of Foundation + AppKit etc.
The underlying reason here is that diagnostics have become much more
sophisticated,
and they do all sorts of context checking and include the libcpp stuff directly
which is a lot
of GTY(()) stuff.
I cannot immediately see any small set of state that we can save / restore
around the
PCH read in,
Perhaps what would be more realistic would be a call into the diagnostics stuff
to say
“disable fancy stuff and just report the minimum”,
as noted, I don’t think this (second) issue is actually a barrier to making the
PCH change
since it’s preexisting - I just ran into it while debugging suitable VM
addresses to use with
ASLR on.
thoughts?
Iain
