Bug 71934 - pch cannot be disabled so gcc cannot be position independent
Summary: pch cannot be disabled so gcc cannot be position independent
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: pch (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Jakub Jelinek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-19 17:30 UTC by nsz
Modified: 2024-03-18 16:26 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-12-03 00:00:00


Attachments
gcc12-pr71934-reloc-wip.patch (6.67 KB, patch)
2021-12-04 15:53 UTC, Jakub Jelinek
Details | Diff
gcc12-pr71934-reloc-wip2.patch (7.42 KB, patch)
2021-12-05 20:40 UTC, Jakub Jelinek
Details | Diff
gcc12-pr71934.patch (8.68 KB, patch)
2021-12-06 08:43 UTC, Jakub Jelinek
Details | Diff
gcc12-pr71934.patch (8.69 KB, patch)
2021-12-06 09:23 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description nsz 2016-07-19 17:30:30 UTC
gcc should be possible to build as PIE by disabling PCH.
(e.g. for running gcc natively on an fdpic target)

some users might not care about PCH, but want PIE gcc,
making PCH position independent can make it slower,
but optionally disabling it should be non-controversial.
Comment 1 Andrew Pinski 2016-07-19 17:47:42 UTC
Actually you could exactly what is done for darwin targets to get it working on PIE.

Also what do you mean by disabled?  Do you mean not doing a PCH for libstdc++ (there is already an option for that; can't remember offhand) or mean execute a sorry for when someone tries it.  As I said there is a way to do PCH even with PIE using mmap early on.
Comment 2 Andrew Pinski 2016-07-19 17:49:20 UTC
Now having PCH around might not be useful anyways.  Someone would have to check to see if anyone uses PCH still.
Comment 3 Martin Liška 2016-07-19 20:08:44 UTC
(In reply to Andrew Pinski from comment #2)
> Now having PCH around might not be useful anyways.  Someone would have to
> check to see if anyone uses PCH still.

I suggested that 2 years ago:
https://gcc.gnu.org/ml/gcc/2015-05/msg00259.html

Looks that we've been waiting for C++ module support to eventually remove PCH support.
Comment 4 Rich Felker 2016-07-20 15:16:32 UTC
Note that some hosts are PIC/PIE only, or require relocating the program text at runtime (textrels). This includes anything without an MMU. GCC 5.x and presumably 6.x works fine on the NOMMU target I've tested with ELF/FDPIC ABI, but of course would blow up if somebody tried using a precompiled header file.

There should really be a build-time option to make gcc completely ignore the existence of .gch files and fallback to the normal headers as if they didn't exist. Even better would be just removing PCH support completely.
Comment 5 Eric Gallager 2021-08-29 04:00:42 UTC
(In reply to Martin Liška from comment #3)
> (In reply to Andrew Pinski from comment #2)
> > Now having PCH around might not be useful anyways.  Someone would have to
> > check to see if anyone uses PCH still.
> 
> I suggested that 2 years ago:
> https://gcc.gnu.org/ml/gcc/2015-05/msg00259.html
> 
> Looks that we've been waiting for C++ module support to eventually remove
> PCH support.

so, GCC has module support now...
Comment 6 Iain Sandoe 2021-09-02 13:39:51 UTC
> Actually you could exactly what is done for darwin targets to get it working on PIE.

Darwin does the same as Linux - disables PIE for the $host build (although PIE is the default for Darwin for a long time).

 - Making PCH position independent is probably a larger job than extending the modules tree streamer to replace it (neither is a trivial activity).

> Now having PCH around might not be useful anyways.  Someone would have to check to see if anyone uses PCH still.

- a) I don't see that anything has changed to make the value go down - quite the opposite, headers have become larger and more complex and parsing has only increased in load.

- b) There are build recipes in the wild that use it, ripping it out would need to be done in a way that didn't break such recipes.

> There should really be a build-time option to make gcc completely ignore the existence of .gch files and fallback to the normal headers as if they didn't exist.

This would be useful; I have a Darwin platform version that is forbidden to be no-PIE so that PCH is disabled by local patches, I might look into a configure option***, but see (b) above about simple removal - we'd have to figure out what to do with command lines that depend on it.

> so, GCC has module support now...

GCC has experimental modules support with some bugs.
modules suppport is not a direct replacement for PCH (e.g. it only works with >= C++20 and not with the other c-family FEs at all).

As noted above, though, it would seem likely that the modules tree streamer could be generalised to support some kind of FE hook that allowed each of the FEs to stream their own specific trees ...  but none of this is a trivial undertaking.

*** I'll put this on my TODO for aarch64-darwin.
Comment 7 Jakub Jelinek 2021-09-02 17:39:05 UTC
So, what is exactly the reason why we disallow PCH for PIEs?
Is it that we support in GC managed memory pointers into .rodata/.data/.text sections of the compiler (function pointers, data pointers, vtable pointers)?
If yes, are those at least marked as pointers in the GTY stuff?
Perhaps the compiler when writing PCH could just record the extents of those sections (or say PT_LOAD segments, on Linux e.g. using dl_iterate_phdr) if it detects the binary is position independent, and when loading the PCH also detect those extents and for GC pointers that fall into those extents adjust them for the new locations?
Comment 8 GCC Commits 2021-12-03 10:06:34 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:fe7c3ecff1f9c0520090a77fa824d8c5d9dbec12

commit r12-5768-gfe7c3ecff1f9c0520090a77fa824d8c5d9dbec12
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Dec 3 11:03:30 2021 +0100

    pch: Add support for PCH for relocatable executables [PR71934]
    
    So, if we want to make PCH work for PIEs, I'd say we can:
    1) add a new GTY option, say callback, which would act like
       skip for non-PCH and for PCH would make us skip it but
       remember for address bias translation
    2) drop the skip for tree_translation_unit_decl::language
    3) change get_unnamed_section to have const char * as
       last argument instead of const void *, change
       unnamed_section::data also to const char * and update
       everything related to that
    4) maybe add a host hook whether it is ok to support binaries
       changing addresses (the only thing I'm worried is if
       some host that uses function descriptors allocates them
       dynamically instead of having them somewhere in the
       executable)
    5) maybe add a gengtype warning if it sees in GTY tracked
       structure a function pointer without that new callback
       option
    
    Here is 1), 2), 3) implemented.
    
    Note, on stdc++.h.gch/O2g.gch there are just those 10 relocations without
    the second patch, with it a few more, but nothing huge.  And for non-PIEs
    there isn't really any extra work on the load side except freading two scalar
    values and fseek.
    
    2021-12-03  Jakub Jelinek  <jakub@redhat.com>
    
            PR pch/71934
    gcc/
            * ggc.h (gt_pch_note_callback): Declare.
            * gengtype.h (enum typekind): Add TYPE_CALLBACK.
            (callback_type): Declare.
            * gengtype.c (dbgprint_count_type_at): Handle TYPE_CALLBACK.
            (callback_type): New variable.
            (process_gc_options): Add CALLBACK argument, handle callback
            option.
            (set_gc_used_type): Adjust process_gc_options caller, if callback,
            set type to &callback_type.
            (output_mangled_typename): Handle TYPE_CALLBACK.
            (walk_type): Likewise.  Handle callback option.
            (write_types_process_field): Handle TYPE_CALLBACK.
            (write_types_local_user_process_field): Likewise.
            (write_types_local_process_field): Likewise.
            (write_root): Likewise.
            (dump_typekind): Likewise.
            (dump_type): Likewise.
            * gengtype-state.c (type_lineloc): Handle TYPE_CALLBACK.
            (state_writer::write_state_callback_type): New method.
            (state_writer::write_state_type): Handle TYPE_CALLBACK.
            (read_state_callback_type): New function.
            (read_state_type): Handle TYPE_CALLBACK.
            * ggc-common.c (callback_vec): New variable.
            (gt_pch_note_callback): New function.
            (gt_pch_save): Stream out gt_pch_save function address and relocation
            table.
            (gt_pch_restore): Stream in saved gt_pch_save function address and
            relocation table and apply relocations if needed.
            * doc/gty.texi (callback): Document new GTY option.
            * varasm.c (get_unnamed_section): Change callback argument's type and
            last argument's type from const void * to const char *.
            (output_section_asm_op): Change argument's type from const void *
            to const char *, remove unnecessary cast.
            * tree-core.h (struct tree_translation_unit_decl): Drop GTY((skip))
            from language member.
            * output.h (unnamed_section_callback): Change argument type from
            const void * to const char *.
            (struct unnamed_section): Use GTY((callback)) instead of GTY((skip))
            for callback member.  Change data member type from const void *
            to const char *.
            (struct noswitch_section): Use GTY((callback)) instead of GTY((skip))
            for callback member.
            (get_unnamed_section): Change callback argument's type and
            last argument's type from const void * to const char *.
            (output_section_asm_op): Change argument's type from const void *
            to const char *.
            * config/avr/avr.c (avr_output_progmem_section_asm_op): Likewise.
            Remove unneeded cast.
            * config/darwin.c (output_objc_section_asm_op): Change argument's type
            from const void * to const char *.
            * config/pa/pa.c (som_output_text_section_asm_op): Likewise.
            (som_output_comdat_data_section_asm_op): Likewise.
            * config/rs6000/rs6000.c (rs6000_elf_output_toc_section_asm_op):
            Likewise.
            (rs6000_xcoff_output_readonly_section_asm_op): Likewise.  Instead
            of dereferencing directive hardcode variable names and decide based on
            whether directive is NULL or not.
            (rs6000_xcoff_output_readwrite_section_asm_op): Change argument's type
            from const void * to const char *.
            (rs6000_xcoff_output_tls_section_asm_op): Likewise.  Instead
            of dereferencing directive hardcode variable names and decide based on
            whether directive is NULL or not.
            (rs6000_xcoff_output_toc_section_asm_op): Change argument's type
            from const void * to const char *.
            (rs6000_xcoff_asm_init_sections): Adjust get_unnamed_section callers.
    gcc/c-family/
            * c-pch.c (struct c_pch_validity): Remove pch_init member.
            (pch_init): Don't initialize v.pch_init.
            (c_common_valid_pch): Don't warn and punt if .text addresses change.
    libcpp/
            * include/line-map.h (class line_maps): Add GTY((callback)) to
            reallocator and round_alloc_size members.
Comment 9 H.J. Lu 2021-12-03 13:28:12 UTC
Can we enable PIE on gcc now?
Comment 10 Jakub Jelinek 2021-12-03 13:30:54 UTC
For start we could revert the patch that prevents it, I think that was
r6-4396-g5148d2e38fa5ff6 + perhaps some follow-ups.
Comment 11 Iain Sandoe 2021-12-03 13:42:55 UTC
I think this needs to be done selectively (I posted some patches which probably need some polish).

We have to remember that (much thought I really appreciates Jakub's work on this) this only solves part of the problem - PCH is still not relocatable thus:

* Not all targets will be able to do PIE (32b hosts in particular might have trouble to find a free space in the VMA for the PCH)  - Certainly, I have not much oconfidence that we can accommodate non-PIE without relocatable PCH on m32 Darwin.

* Also it should be possible to have a PIE toolset that generates non-PIE by default - where at the moment you would only get a PIE toolset iff you --enable-default-pie.

* so I think that there should be an option to allow PIE tools rather than relying on --enable-default-PIE..

So my guess is that the full answer is:

Yes, for hosts that can find a 512Mb or sol space in the VMA that is confidently available - and no otherwise.
Comment 12 H.J. Lu 2021-12-03 13:46:41 UTC
(In reply to Iain Sandoe from comment #11)
> So my guess is that the full answer is:
> 
> Yes, for hosts that can find a 512Mb or sol space in the VMA that is
> confidently available - and no otherwise.

We can add a host option to enable PIE.
Comment 13 Iain Sandoe 2021-12-03 14:02:11 UTC
Have two patches that implement "--enable-pie-tools" to do this

as noted they need some polish and I suspect that we need a "PIEflag.m4" modelled in the same way as PICflag.m4 (which covers both enable and disable) instead of hard-wiring -fno-PIE etc. into the main configure.

I will try to update the patches over the next few days and re-post them.
Comment 14 Jakub Jelinek 2021-12-03 14:03:38 UTC
Making PCH relocatable (as last resort) is doable too, after all, we already relocate it once during PCH storing, all the info is there.
We even don't need something like the saving_htab for it, all we need it the OS is unable to give us the expected memory mapping map it elsewhere, compute the bias (difference from where it is actually mapped and where it was expected to be mapped) and add that bias to all pointers in the mapped blob and to all the ggc roots's pointers that point into the original expected memory mapping range as well.
If there is no overlap between the expected and actual mapping, making sure the bias isn't added twice is even easier (just always compare if it is in the old range and only then add bias), but I bet that can't be ensured, so we need to use something to track whether we've done that already (e.g. what we use for GC marking normally?).

I think on Linux this shouldn't trigger too often even on 32-bit architectures, because ASLR isn't all over the address space, just +- a couple of megabytes at most.
Comment 15 Iain Sandoe 2021-12-03 14:10:23 UTC
if it were only the main exe, I think we'd be OK on m32 Darwin too - but after 10.7  everything gets ASLRd (kernel, DSOs, dynamic linker and exe) so even though each one is in a smallish range, the combined effect is to leave no really obvious window.

On m64 - we have plenty of VMA to play with - and that looks OK.

I suppose that if there were a graceful way to recover on m32 (so that it was just a performance hit not a compile fail) then that would be good enough.
Comment 16 Jakub Jelinek 2021-12-03 14:14:44 UTC
And if doing it at restore time would be too hard, there is always an option to precompute a "relocation" table during PCH saving and store it at the very end of the PCH file, so that normally when successfully loading PCH at the expected address we don't need to touch that area, but if we map it elsewhere, we can read it and relocate.  Such relocation table could either contain all the addresses in the global GTY roots that need += bias (relative to say gt_ggc_rtab or something similar) and all the addresses in the mapped blob that need += bias (relative to the start of the mapped blob), ideally sorted during PCH saving by increasing address.  If that would be too large, another option would be try to make it more compact, e.g. after that sorting of addresses represent it as unsigned short delta from the last address, unless that ushort is 0, which then would be followed by full sizeof(void*) address relative to gt_ggc_rtab or the map base;
perhaps even ushort 0 followed by address relative to gt_ggc_rtab and ushort 1 followed by address relative to map base.
Comment 17 Jakub Jelinek 2021-12-04 15:53:04 UTC
Created attachment 51926 [details]
gcc12-pr71934-reloc-wip.patch

Completely untested patch to perform PCH relocation.
Due to nested_ptr unfortunately the relocate_ptrs hook isn't always called with the address of the pointer in the object so that we can remember at PCH save time where the pointer is, so I had to add another argument to the hook.  In the common case the new middle argument is NULL, which stands for the first argument
is pointer into the object and that address should be remembered for possible later relocation.  For the nested_ptr case the first argument is address of some temporary pointer and the new middle argument is the pointer to the pointer.
And the last case is for some weirdo case in tree-cfg where we call it even on an unrelated address of LOCATION_BLOCK, it passes the same pointer to first and second argument and the hook treats it as adjust, but don't actually remember the address.
The relocations are then stored with what I've talked about, ushort diff from last address if it fits, or 0 followed by size_t difference from the base.  In a small PCH I've created for stdio.h there were ~ 56000 pointers that need relocations in the image and I needed a single 0 entry at the start with the size_t and all others fit into ushort.

The host pch_use_address hooks need to be adjusted so that they do support mapping at a different address, only the Linux hook has been adjusted (and for testing I've actually hardcoded there that it forces the relocation always).
Comment 18 Iain Sandoe 2021-12-05 12:51:36 UTC
I needed two small additions to build (c-family+Ada+fortran+lto)
** Go and D not tested.

diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index 93b6eb5bb45..af6b6bd14d4 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -171,7 +171,7 @@ gt_pch_nx (Entity_Id &)
 void
 gt_pch_nx (Entity_Id *x, gt_pointer_operator op, void *cookie)
 {
-  op (x, cookie);
+  op (x, NULL, cookie);
 }
 
 struct dummy_type_hasher : ggc_cache_ptr_hash<tree_entity_vec_map>
diff --git a/gcc/config/host-darwin.h b/gcc/config/host-darwin.h
index 4acae9cf341..f3a477e8837 100644
--- a/gcc/config/host-darwin.h
+++ b/gcc/config/host-darwin.h
@@ -18,7 +18,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 extern void * darwin_gt_pch_get_address (size_t sz, int fd);
-extern int darwin_gt_pch_use_address (void *addr, size_t sz, int fd, 
+extern int darwin_gt_pch_use_address (void *&addr, size_t sz, int fd, 
                                      size_t off);
 
 #undef HOST_HOOKS_GT_PCH_GET_ADDRESS

======

I fixed up host-darwin.c in the same way as you did for host-linux (and disabled some of the extra checks I've got in place at the moment).

======

It looks like it adds around 10M to the largest libstdc++ case for x86_64 (151=>161Mb) so not too bad (6.6%).

1. C and C++ pch.exp seem OK

2. libstdc++ seems OK (some of the tests will use the PCH).

3. I have one failing ObjC test...

(possibly a missing or incorrectly specified GTY(()) - will investigate when I have a chance).

FAIL: objc.dg/pch/interface-1.m -fnext-runtime -Wno-objc-root-class -O0 -g -I. -Dwith_PCH (internal compiler error)
FAIL: objc.dg/pch/interface-1.m -fnext-runtime -Wno-objc-root-class -O0 -g -I. -Dwith_PCH (test for excess errors)
Excess errors:
<built-in>: internal compiler error: tree check: expected identifier_node, have error_mark in private_lookup_attribute, at attribs.c:1971
Comment 19 Iain Sandoe 2021-12-05 14:44:10 UTC
(In reply to Iain Sandoe from comment #18)
> I needed two small additions to build (c-family+Ada+fortran+lto)

> 3. I have one failing ObjC test...
> 
> (possibly a missing or incorrectly specified GTY(()) - will investigate when
> I have a chance).

Seems to be a problem with the specification of a meta-data array
(fix under test), not sure why the problem does not show until we try relocating.

With the fix all the PCH and libstdc++ cases behave 'as normal'.

===== but...

It seems that this change breaks the PIE fix, although it is not obvious (to me) why it would - I'd expect change in position of the exe to be independent of change of position of data pointers .. but I do not know this code that well.
Comment 20 Jakub Jelinek 2021-12-05 14:52:09 UTC
Thanks, you've got far further than I ended up with.
I'll incorporate your changes tomorrow.  And indeed, the PIE relocation code needs to take the bias into account and I even noted I need to do that, but just forgot to do it in the patch in the end.  Will tweak that tomorrow as well.
As for the file size, perhaps a way to shrink it slightly more would be to use uleb128 differences instead of unsigned short.
Comment 21 Iain Sandoe 2021-12-05 15:22:12 UTC
I'm not sure that the file size is that critical.

IIUC, we never even read the relocation table unless the target returns a different mapped address from the one in the file, and we hope to choose VMA positions that are likely to succeed most of the time so that this is an infrequent fall-back?
Comment 22 Jakub Jelinek 2021-12-05 15:30:47 UTC
That is true, but for the common case the relocation table sits in between two parts that need to be used, so in the common case it fseeks over it.
I was hoping the relocation table could be last, but that seems very hard because  part of the PCH file are stored and read in other functions (in c-family).
Comment 23 Jakub Jelinek 2021-12-05 20:40:59 UTC
Created attachment 51929 [details]
gcc12-pr71934-reloc-wip2.patch

Updated untested patch that incorporates your 2 fixes and uses uleb128 to store the relocation data, which shrinks it to roughly 50% of the previous size (1.01 bytes for one pointer on average).
Comment 24 Jakub Jelinek 2021-12-06 08:43:54 UTC
Created attachment 51931 [details]
gcc12-pr71934.patch

Bootstrap/regtest on x86_64-linux passed, but on i686-linux I saw
+FAIL: 17_intro/headers/c++2011/stdc++.cc (test for excess errors)
+FAIL: 17_intro/headers/c++2011/stdc++_multiple_inclusion.cc (test for excess errors)
regressions.  I've tracked that to not remembering in the relocation table
if a pointer that needs relocation is immediately at mmi.preferred_base.

This updated patch fixes that, fixes also a bug in host-linux.c found by Iain, updates gty.texi and adds ChangeLog.
Comment 25 Jakub Jelinek 2021-12-06 09:23:28 UTC
Created attachment 51932 [details]
gcc12-pr71934.patch

Fix a small thinko in host-linux.c, for addr == (void *) MAP_FAILED we need to fail, not blindly try to write at it.
Comment 26 Jakub Jelinek 2021-12-06 09:35:37 UTC
Note, the code to emit that relocation table isn't for free, besides that ~3% growth of the *.gch files it also slows down the PCH generation, e.g. for stdc++.h at -O2 -g on my box from ~3.45s to ~4.13s, so some 20%, I bet mainly to qsort the array of ~2.5 million pointers.
But upon PCH restore it shouldn't be noticeable, it is one size_t fread and one fseek when no relocation is needed.
Comment 27 Iain Sandoe 2021-12-06 09:43:10 UTC
(In reply to Jakub Jelinek from comment #26)
> Note, the code to emit that relocation table isn't for free, besides that
> ~3% growth of the *.gch files it also slows down the PCH generation, e.g.
> for stdc++.h at -O2 -g on my box from ~3.45s to ~4.13s, so some 20%, I bet
> mainly to qsort the array of ~2.5 million pointers.

the one CS problem indirection cannot solve is having too many pointers ;).

Do you have a feel for what the growth in time is for the whole parse+output?

(we assume that PCH remains useful so long as the read-in is quicker than that, right?)

> But upon PCH restore it shouldn't be noticeable, it is one size_t fread and
> one fseek when no relocation is needed.
Comment 28 GCC Commits 2021-12-06 10:21:06 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:4dc6d19222581c77a174d44d97507d234fb7e39b

commit r12-5802-g4dc6d19222581c77a174d44d97507d234fb7e39b
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 6 11:18:58 2021 +0100

    avr: Fix AVR build [PR71934]
    
    On Mon, Dec 06, 2021 at 11:00:30AM +0100, Martin Liška wrote:
    > Jakub, I think the patch broke avr-linux target:
    >
    > g++  -fno-PIE -c   -g   -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-erro
    > /home/marxin/Programming/gcc/gcc/config/avr/avr.c: In function âvoid avr_output_data_section_asm_op(const void*)â:
    > /home/marxin/Programming/gcc/gcc/config/avr/avr.c:10097:26: error: invalid conversion from âconst void*â to âconst char*â [-fpermissive]
    
    This patch fixes that.
    
    2021-12-06  Jakub Jelinek  <jakub@redhat.com>
    
            PR pch/71934
            * config/avr/avr.c (avr_output_data_section_asm_op,
            avr_output_bss_section_asm_op): Change argument type from const void *
            to const char *.
Comment 29 GCC Commits 2021-12-09 14:44:01 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:747380f47da0da6c11fd5262ac428bb53433ea19

commit r12-5855-g747380f47da0da6c11fd5262ac428bb53433ea19
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Dec 9 15:40:15 2021 +0100

    pch: Add support for relocation of the PCH data [PR71934]
    
    The following patch adds support for relocation of the PCH blob on PCH
    restore if we don't manage to get the preferred map slot for it.
    The GTY stuff knows where all the pointers are, after all it relocates
    it once during PCH save from the addresses where it was initially allocated
    to addresses in the preferred map slot.
    But, if we were to do it solely using GTY info upon PCH restore, we'd need
    another set of GTY functions, which I think would make it less maintainable
    and I think it would also be more costly at PCH restore time.  Those
    functions would need to call something to add bias to pointers that haven't
    been marked yet and make sure not to add bias to any pointer twice.
    
    So, this patch instead builds a relocation table (sorted list of addresses
    in the blob which needs relocation) at PCH save time, stores it in a very
    compact form into the gch file and upon restore, adjusts pointers in GTY
    roots (that is right away in the root structures) and the addresses in the
    relocation table.
    The cost on stdc++.gch/O2g.gch (previously 85MB large) is about 3% file size
    growth, there are 2.5 million pointers that need relocation in the gch blob
    and the relocation table uses uleb128 for address deltas and needs ~1.01 bytes
    for one address that needs relocation, and about 20% compile time during
    PCH save (I think it is mainly because of the need to qsort those 2.5
    million pointers).  On PCH restore, if it doesn't need relocation (the usual
    case), it is just an extra fread of sizeof (size_t) data and fseek
    (in my tests real time on vanilla tree for #include <bits/stdc++.h> CU
    was ~0.175s and with the patch but no relocation ~0.173s), while if it needs
    relocation it took ~0.193s, i.e. 11.5% slower.
    Without PCH that
     #include <bits/stdc++.h>
     int i;
    testcase compiles with -O2 -g in ~1.199s, i.e. 6.2 times slower than PCH with
    relocation and 6.9 times than PCH without relocation.
    
    The discovery of the pointers in the blob that need relocation is done
    in the relocate_ptrs hook which does the pointer relocation during PCH save.
    Unfortunately, I had to make one change to the gengtype stuff due to the
    nested_ptr feature of GTY, which some libcpp headers and stringpool.c use.
    The relocate_ptrs hook had 2 arguments, pointer to the pointer and a cookie.
    When relocate_ptrs is done, in most cases it is called solely on the
    subfields of the current object, so e.g.
              if ((void *)(x) == this_obj)
                op (&((*x).u.fld[0].rt_rtx), cookie);
    so relocate_ptrs can assert that ptr_p is within the
    state->ptrs[state->ptrs_i]->obj ..
    state->ptrs[state->ptrs_i]->obj+state->ptrs[state->ptrs_i]->size-sizeof(void*)
    range and compute from that the address in the blob which will need
    relocation (state->ptrs[state->ptrs_i]->new_addr is the new address
    given to it and ptr_p-state->ptrs[state->ptrs_i]->obj is the relative
    offset.  Unfortunately, for nested_ptr gengtype emits something like:
          {
            union tree_node * x0 =
              ((*x).val.node.node) ? HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).val.node.node))) : NULL;
            if ((void *)(x) == this_obj)
              op (&(x0), cookie);
            (*x).val.node.node = (x0) ? CPP_HASHNODE (GCC_IDENT_TO_HT_IDENT ((x0))) : NULL;
          }
    so relocate_ptrs is called with an address of some temporary variable and
    so doesn't know where the pointer will finally be.
    So, I've added another argument to relocate_ptrs (and to
    gt_pointer_operator).  For the most common case I pass NULL as the new middle
    argument to that function, first one remains pointer to the pointer that
    needs adjustment and last the cookie.  The NULL seems to be cheap to compute
    and short in the gt*.[ch] files and stands for ptr_p is an address within
    the this_obj's range, remember its address.  For the nested_ptr case, the
    new middle argument contains actual address of the pointer that might need
    to be relocated, so instead of the above
              op (&(x0), &((*x).val.node.node), cookie);
    in there.  And finally, e.g. for the reorder case I need a way to tell
    restore_ptrs to ignore a particular address for the relocation purposes
    and only treat it the old way.  I've used for that the case when
    the first and second arguments are equal.
    
    In order to enable support for mapping PCH as fallback at different
    addresses than the preferred ones, a small change is needed to the
    host pch_use_address hooks.  One change I've done to all of them is
    the change of the type of the first argument from void * to void *&,
    such that the actual address can be told to the callers (or shall I
    instead use void **?), but another change that still needs to be done
    in them if they want the relocation is actually not fail if they couldn't
    get a preferred address, but instead modify what the first argument
    refers to.  I've done that only for host-linux.c and Iain is testing
    similar change for host-darwin.c.  Didn't change hpux, netbsd, openbsd,
    solaris, mingw32 or the fallbacks because I can't test those.
    
    Tested also with the:
    --- gcc/config/host-linux.c.jj  2021-12-06 22:22:42.007777367 +0100
    +++ gcc/config/host-linux.c     2021-12-07 00:21:53.052674040 +0100
    @@ -191,6 +191,8 @@ linux_gt_pch_use_address (void *&base, s
       if (size == 0)
         return -1;
    
    +base = (char *) base + ((size + 8191) & (size_t) -4096);
    +
       /* Try to map the file with MAP_PRIVATE.  */
       addr = mmap (base, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, offset);
    
    hack which forces all PCH restores to be relocated.  An earlier version of the
    patch has been also regrest with base = (char *) base + 16384; in that spot,
    so both relocation to a non-overlapping spot and to an overlapping spot have
    been tested.
    
    2021-12-09  Jakub Jelinek  <jakub@redhat.com>
    
            PR pch/71934
            * coretypes.h (gt_pointer_operator): Use 3 pointer arguments instead
            of two.
            * gengtype.c (struct walk_type_data): Add in_nested_ptr argument.
            (walk_type): Temporarily set d->in_nested_ptr around nested_ptr
            handling.
            (write_types_local_user_process_field): Pass a new middle pointer
            to gt_pointer_operator op calls, if d->in_nested_ptr pass there
            address of d->prev_val[2], otherwise NULL.
            (write_types_local_process_field): Likewise.
            * ggc-common.c (relocate_ptrs): Add real_ptr_p argument.  If equal
            to ptr_p, do nothing, otherwise if NULL remember ptr_p's
            or if non-NULL real_ptr_p's corresponding new address in
            reloc_addrs_vec.
            (reloc_addrs_vec): New variable.
            (compare_ptr, read_uleb128, write_uleb128): New functions.
            (gt_pch_save): When iterating over objects through relocate_ptrs,
            save current i into state.ptrs_i.  Sort reloc_addrs_vec and emit
            it as uleb128 of differences between pointer addresses into the
            PCH file.
            (gt_pch_restore): Allow restoring of PCH to a different address
            than the preferred one, in that case adjust global pointers by bias
            and also adjust by bias addresses read from the relocation table
            as uleb128 differences.  Otherwise fseek over it.  Perform
            gt_pch_restore_stringpool only after adjusting callbacks and for
            callback adjustments also take into account the bias.
            (default_gt_pch_use_address): Change type of first argument from
            void * to void *&.
            (mmap_gt_pch_use_address): Likewise.
            * ggc-tests.c (gt_pch_nx): Pass NULL as new middle argument to op.
            * hash-map.h (hash_map::pch_nx_helper): Likewise.
            (gt_pch_nx): Likewise.
            * hash-set.h (gt_pch_nx): Likewise.
            * hash-table.h (gt_pch_nx): Likewise.
            * hash-traits.h (ggc_remove::pch_nx): Likewise.
            * hosthooks-def.h (default_gt_pch_use_address): Change type of first
            argument from void * to void *&.
            (mmap_gt_pch_use_address): Likewise.
            * hosthooks.h (struct host_hooks): Change type of first argument of
            gt_pch_use_address hook from void * to void *&.
            * machmode.h (gt_pch_nx): Expect a callback with 3 pointers instead of
            two in the middle argument.
            * poly-int.h (gt_pch_nx): Likewise.
            * stringpool.c (gt_pch_nx): Pass NULL as new middle argument to op.
            * tree-cfg.c (gt_pch_nx): Likewise, except for LOCATION_BLOCK pass
            the same &(block) twice.
            * value-range.h (gt_pch_nx): Pass NULL as new middle argument to op.
            * vec.h (gt_pch_nx): Likewise.
            * wide-int.h (gt_pch_nx): Likewise.
            * config/host-darwin.c (darwin_gt_pch_use_address): Change type of
            first argument from void * to void *&.
            * config/host-darwin.h (darwin_gt_pch_use_address): Likewise.
            * config/host-hpux.c (hpux_gt_pch_use_address): Likewise.
            * config/host-linux.c (linux_gt_pch_use_address): Likewise.  If
            it couldn't succeed to mmap at the preferred location, set base
            to the actual one.  Update addr in the manual reading loop instead of
            base.
            * config/host-netbsd.c (netbsd_gt_pch_use_address): Change type of
            first argument from void * to void *&.
            * config/host-openbsd.c (openbsd_gt_pch_use_address): Likewise.
            * config/host-solaris.c (sol_gt_pch_use_address): Likewise.
            * config/i386/host-mingw32.c (mingw32_gt_pch_use_address): Likewise.
            * config/rs6000/rs6000-gen-builtins.c (write_init_file): Pass NULL
            as new middle argument to op in the generated code.
            * doc/gty.texi: Adjust samples for the addition of middle pointer
            to gt_pointer_operator callback.
    gcc/ada/
            * gcc-interface/decl.c (gt_pch_nx): Pass NULL as new middle argument
            to op.
    gcc/c-family/
            * c-pch.c (c_common_no_more_pch): Pass a temporary void * var
            with NULL value instead of NULL to host_hooks.gt_pch_use_address.
    gcc/c/
            * c-decl.c (resort_field_decl_cmp): Pass the same pointer twice
            to resort_data.new_value.
    gcc/cp/
            * module.cc (nop): Add another void * argument.
            * name-lookup.c (resort_member_name_cmp): Pass the same pointer twice
            to resort_data.new_value.
Comment 30 GCC Commits 2021-12-09 14:56:00 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:bf15cd665e74791aae87e7e151a0cf0c4cb54684

commit r12-5856-gbf15cd665e74791aae87e7e151a0cf0c4cb54684
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Dec 9 15:54:33 2021 +0100

    pch: Fix up Darwin and HPUX pch_use_address hooks [PR71934]
    
    In the last change, I've changed the arguments from void * to void *&,
    but missed the fact that these hooks will in that case update the value
    the caller will see in an undesirable way.
    
    2021-12-09  Jakub Jelinek  <jakub@redhat.com>
    
            PR pch/71934
            * config/host-darwin.c (darwin_gt_pch_use_address): When reading
            manually the file into mapped area, update mapped_addr as
            an automatic variable rather than addr which is a reference parameter.
            * config/host-hpux.c (hpux_gt_pch_use_address): When reading
            manually the file into mapped area, update addr as
            an automatic variable rather than base which is a reference parameter.
Comment 31 GCC Commits 2021-12-09 15:25:32 UTC
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:0d5db1dd65b45286082f82f600ca0a3e6e43e06e

commit r12-5857-g0d5db1dd65b45286082f82f600ca0a3e6e43e06e
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Mon Dec 6 07:50:08 2021 +0000

    Darwin, PCH: Rework hooks for relocatable implementation [PR71934].
    
    Now we have a relocatable PCH implementation we can revise the
    hooks that find and use the mmapped memory.  Specifically, this
    removes the extra checking and diagnostic output for cases that
    were likely to fail in a non-relocatable scenario.
    
    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
    
    gcc/ChangeLog:
    
            PR pch/71934
            * config/host-darwin.c (SAFE_ALLOC_SIZE): Remove.
            (darwin_gt_pch_get_address): Rework for relocatable PCH.
            (darwin_gt_pch_use_address): Likewise.
Comment 32 Iain Sandoe 2021-12-09 15:43:35 UTC
I guess the first part (fixing with PIE) fixes a regression - but the remainder is an enhancement?
Comment 33 GCC Commits 2021-12-09 16:57:03 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:fa9f40bacbd187996e03f93086fae1ab9052f51b

commit r12-5868-gfa9f40bacbd187996e03f93086fae1ab9052f51b
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Dec 9 17:55:28 2021 +0100

    pch: Fix aarch64 build [PR71934]
    
    On Thu, Dec 09, 2021 at 05:42:10PM +0100, Christophe Lyon wrote:
    > This also broke aarch64 I think:
    > In file included from
    > /tmp/6140018_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/aarch64/aarch64-sve-builtins.cc:3920:0:
    > ./gt-aarch64-sve-builtins.h: In function 'void
    > gt_pch_p_19registered_function(void*, void*, gt_pointer_operator, void*)':
    > ./gt-aarch64-sve-builtins.h:86:44: error: no matching function for call to
    > 'gt_pch_nx(aarch64_sve::function_instance*, void (*&)(void*, void*, void*),
    > void*&)'
    >      gt_pch_nx (&((*x).instance), op, cookie);
    
    Fixed thusly.
    
    2021-12-09  Jakub Jelinek  <jakub@redhat.com>
    
            PR pch/71934
            * config/aarch64/aarch64-sve-builtins.cc (gt_pch_nx): Change type of
            second argument from function with 2 pointer arguments to function
            with 3 pointer arguments.