Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sd-bus: set retain attribute on BUS_ERROR_MAP_ELF_REGISTER #19144

Merged
merged 1 commit into from Mar 29, 2021

Conversation

MaskRay
Copy link
Contributor

@MaskRay MaskRay commented Mar 29, 2021

LLD 13 and GNU ld 2.37 support -z start-stop-gc which allows garbage
collection of C identifier name sections despite the __start_/__stop_
references. Simply set the retain attribute so that GCC 11 (if
configure-time binutils is 2.36 or newer)/Clang 13 will set the
SHF_GNU_RETAIN section attribute to prevent garbage collection.

Without the patch, there are linker errors like the following with -z
start-stop-gc.

ld: error: undefined symbol: __start_SYSTEMD_BUS_ERROR_MAP
>>> referenced by bus-error.c:93 (../src/libsystemd/sd-bus/bus-error.c:93)
>>>               sd-bus_bus-error.c.o:(bus_error_name_to_errno) in archive src/libsystemd/libsystemd_static.a

If you are not reluctant to read a (very long) writeup, https://maskray.me/blog/2021-01-31-metadata-sections-comdat-and-shf-link-order :)

LLD 13 and GNU ld 2.37 support -z start-stop-gc which allows garbage
collection of C identifier name sections despite the __start_/__stop_
references.  Simply set the retain attribute so that GCC 11 (if
configure-time binutils is 2.36 or newer)/Clang 13 will set the
SHF_GNU_RETAIN section attribute to prevent garbage collection.

Without the patch, there are linker errors like the following with -z
start-stop-gc.

```
ld: error: undefined symbol: __start_SYSTEMD_BUS_ERROR_MAP
>>> referenced by bus-error.c:93 (../src/libsystemd/sd-bus/bus-error.c:93)
>>>               sd-bus_bus-error.c.o:(bus_error_name_to_errno) in archive src/libsystemd/libsystemd_static.a
```
@keszybz
Copy link
Member

keszybz commented Mar 29, 2021

LGTM.

*/

#define BUS_ERROR_MAP_ELF_REGISTER \
_Pragma("GCC diagnostic ignored \"-Wattributes\"") \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pragma will take effect until the end of the file where BUS_ERROR_MAP_ELF_REGISTER is used. That seems OK.

@keszybz
Copy link
Member

keszybz commented Mar 29, 2021

bionic-i386: TEST-10-ISSUE-2467: FAIL ( 1 s)
bionic-390x: timeout in test-process-util.

@keszybz keszybz merged commit 945317a into systemd:main Mar 29, 2021
@MaskRay MaskRay deleted the bus-error branch March 29, 2021 16:55
MaskRay added a commit to llvm/llvm-project that referenced this pull request Apr 16, 2021
Change the default to facilitate GC for metadata section usage, so that they
don't need SHF_LINK_ORDER or SHF_GROUP just to drop the unhelpful rule (if they
want to be unconditionally retained, use SHF_GNU_RETAIN
(`__attribute__((retain))`) or linker script `KEEP`).

The dropped SHF_GROUP special case makes the behavior of -z start-stop-gc and -z
nostart-stop-gc closer to GNU ld>=2.37 (https://sourceware.org/PR27451).

However, we default to -z start-stop-gc (which actually matches more closely to
GNU ld before 2015-10 https://sourceware.org/PR19167), which is different from
modern GNU ld (which has the unhelpful rule to work around glibc). As a
compensation, we special case `__libc_` sections as a workaround for glibc<2.34
(https://sourceware.org/PR27492).

Since -z start-stop-gc as the default actually matches the traditional GNU ld
behavior, there isn't much to be aware of. There was a systemd usage which has
been fixed by systemd/systemd#19144
arichardson pushed a commit to arichardson/llvm-project that referenced this pull request Sep 10, 2021
Change the default to facilitate GC for metadata section usage, so that they
don't need SHF_LINK_ORDER or SHF_GROUP just to drop the unhelpful rule (if they
want to be unconditionally retained, use SHF_GNU_RETAIN
(`__attribute__((retain))`) or linker script `KEEP`).

The dropped SHF_GROUP special case makes the behavior of -z start-stop-gc and -z
nostart-stop-gc closer to GNU ld>=2.37 (https://sourceware.org/PR27451).

However, we default to -z start-stop-gc (which actually matches more closely to
GNU ld before 2015-10 https://sourceware.org/PR19167), which is different from
modern GNU ld (which has the unhelpful rule to work around glibc). As a
compensation, we special case `__libc_` sections as a workaround for glibc<2.34
(https://sourceware.org/PR27492).

Since -z start-stop-gc as the default actually matches the traditional GNU ld
behavior, there isn't much to be aware of. There was a systemd usage which has
been fixed by systemd/systemd#19144
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
Change the default to facilitate GC for metadata section usage, so that they
don't need SHF_LINK_ORDER or SHF_GROUP just to drop the unhelpful rule (if they
want to be unconditionally retained, use SHF_GNU_RETAIN
(`__attribute__((retain))`) or linker script `KEEP`).

The dropped SHF_GROUP special case makes the behavior of -z start-stop-gc and -z
nostart-stop-gc closer to GNU ld>=2.37 (https://sourceware.org/PR27451).

However, we default to -z start-stop-gc (which actually matches more closely to
GNU ld before 2015-10 https://sourceware.org/PR19167), which is different from
modern GNU ld (which has the unhelpful rule to work around glibc). As a
compensation, we special case `__libc_` sections as a workaround for glibc<2.34
(https://sourceware.org/PR27492).

Since -z start-stop-gc as the default actually matches the traditional GNU ld
behavior, there isn't much to be aware of. There was a systemd usage which has
been fixed by systemd/systemd#19144
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants