]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
Avoid adjusting an eh_frame symbol into a section symbol plus offset
authorMatthew Malcomson <matthew.malcomson@arm.com>
Fri, 4 Nov 2022 10:01:50 +0000 (10:01 +0000)
committerMatthew Malcomson <matthew.malcomson@arm.com>
Fri, 4 Nov 2022 10:03:01 +0000 (10:03 +0000)
commitea22e23a0c9fbc97f7b501a9e3670c8ca764306b
tree2979be36ffa29655eca440fa8774c195ef098264
parentfb1ff58603dd0ebc184b0e07c0628c6eb746c491
Avoid adjusting an eh_frame symbol into a section symbol plus offset

GNU bfd linker removes duplicate CIE and FDE entries in the exception
handling information.  When it does this entries in the .eh_frame
section end up in different positions to where they were originally.

In order to account for that, when the linker removes an FDE/CIE entry
from one object's .eh_frame section in order to prefer an equivalent
entry in another object's .eh_frame section, the linker adjusts symbols
which were pointing to the first entry to point to the second.

If the assembler has changed symbols pointing into the .eh_frame section
such that they are now described by a section symbol plus offset, the
linker can not perform this transformation.  This means that symbols can
end up pointing at different information than they originally pointed
at.

NOTE1: This changes the behaviour of this on *all* targets.  As it
stands that seems like the correct approach since the linker behaviour
that we are accounting for is a general behaviour.  On top of that, this
translation should not make a change in functionality if the linker
behaviour were not enabled for some target (since without the linker
behaviour this transformation should not affect anything -- which is why
it's believed to be safe in the first place).  However it is still
important to note that we have not actually tested these changes on
other architectures.

NOTE2: Since the GNU linker makes its decision to look for items to
merge or not based on the *output* section name, there is a mapping
between output sections and input sections that can be modified by the
user, and we may not even be using the GNU linker in the first place,
our patch can not be 100% accurate and robust when choosing which
sections to avoid this adjustment.

It is still desirable to avoid problematic adjustment in the common case
of using the GNU linker with the default mapping between input sections
and output sections.  Though it may not be desirable to hard-code a
feature of the default linker script at the time of writing this patch
into GAS.
Here we use the same check that the assembler uses in gas/dw2gencfi.c to
identify a .eh_frame section.  This has the benefits of being a check
the assembler is using already (so the assembler is internally
consistent) and matching the split that the default bfd linker scripts
make between all input sections that said scripts name.

E.g. the default linker script for aarch64-none-elf matches the
following patterns for an output section named .eh_frame_hdr
{ *(.eh_frame_hdr) *(.eh_frame_entry .eh_frame_entry.*) }
and matches the below for an output section named .eh_frame
{ KEEP (*(.eh_frame)) *(.eh_frame.*) }.
The linker then applies the problematic transformation to the .eh_frame
output section and not to the .eh_frame_hdr section.
The check we use here makes a corresponding decision to all sections
which would be caught by the above patterns.  I.e. it avoids adjusting
symbols in sections which would end up in the .eh_frame output section
and does not avoid adjusting symbols in sections which would end up in
the .eh_frame_hdr output section.

NOTE3: This behaviour by itself is not causing any problems for us.  The
trigger for making this change (especially in morello binutils) is that
when crtbeginT.o "registers" an object's exception handling information
with a static glibc, it uses `adrp` and `add` to access the
__EH_FRAME_BEGIN__ symbol.  Currently the relocation on `adrp` is
adjusted into a "section symbol plus offset" transformation (which ends
up as exactly the start of the section symbol in the crtbeginT.o
object), but the `add` instruction is not adjusted in this way.
It is this *difference* that is problematic.

It means that we can end up with a broken pointer using the
.eh_frame page and the __EH_FRAME_BEGIN__ offset into a page.

With base AArch64, both these instructions would be adjusted to point to
the .eh_frame section of crtbeginT.o.  This is still a buggy behaviour
in the assembler due to the reasons given above, but it at least meant
that the static glibc got a sensible pointer (though one starting after
any exception frame information on the crti.o and crt1.o object files).

With this change, both instructions stay pointing at __EH_FRAME_BEGIN__
in the object file.  That means that the linker will leave both
instructions pointing at the same place after de-duplication of
exception information.  That place is not guaranteed to be the start of
the total exception frame information, but in practice it is always
closer to the start of the debug frame than without having made this
patch.

We could have stopped static glibc from crashing by making sure that we
accessed the .eh_frame section symbol for both instructions rather than
using __EH_FRAME_BEGIN__ for both.  This would behave in the same way as
stock AArch64.
This would mean that static glibc would not be affected by the
particulars of how the GNU bfd linker merges CIE and FDE entries
together.  On the other hand it would mean that static glibc would never
have the ability to unwind through start code in crt1.o and crti.o.

I don't have a particularly strong opinion on which of these is the best
approach, I chose this one since it gives the static glibc access to the
full debug information for the moment.
gas/testsuite/gas/aarch64/eh-frame-symbols.d [new file with mode: 0644]
gas/testsuite/gas/aarch64/eh-frame-symbols.s [new file with mode: 0644]
gas/write.c