[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #5 from Eric Gallager <egallager at gcc dot gnu.org> ---
(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...

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |iains at gcc dot gnu.org

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---

> 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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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?

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #8 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: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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> ---
Can we enable PIE on gcc now?

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For start we could revert the patch that prevents it, I think that was
r6-4396-g5148d2e38fa5ff6 + perhaps some follow-ups.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #11 from Iain Sandoe <iains at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-12-03

--- Comment #12 from H.J. Lu <hjl.tools at gmail dot com> ---
(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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #13 from Iain Sandoe <iains at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #15 from Iain Sandoe <iains at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51926
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51926&action=edit
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).

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #18 from Iain Sandoe <iains at gcc dot gnu.org> ---
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

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #19 from Iain Sandoe <iains at gcc dot gnu.org> ---
(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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #21 from Iain Sandoe <iains at gcc dot gnu.org> ---
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?

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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).

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51926|0                           |1
        is obsolete|                            |

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51929
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51929&action=edit
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).

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

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 #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51931
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51931&action=edit
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51929|0                           |1
        is obsolete|                            |
  Attachment #51931|0                           |1
        is obsolete|                            |

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51932
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51932&action=edit
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #27 from Iain Sandoe <iains at gcc dot gnu.org> ---
(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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #28 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: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 *.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #29 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: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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #30 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: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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #31 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #32 from Iain Sandoe <iains at gcc dot gnu.org> ---
I guess the first part (fixing with PIE) fixes a regression - but the remainder
is an enhancement?

[Bug pch/71934] pch cannot be disabled so gcc cannot be position independent

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934

--- Comment #33 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: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.