Page MenuHomePhabricator

[libunwind] adds a way to synthesise libgcc
AbandonedPublic

Authored by cjdb on Jul 23 2021, 1:03 PM.

Details

Reviewers
manojgupta
kristof.beyls
phosek
MaskRay
echristo
compnerd
Group Reviewers
Restricted Project
Summary

Note: the term "libgcc" refers to the all of libgcc.a, libgcc_eh.a,
and libgcc_s.so.

Enabling libunwind as a replacement for libgcc on Linux has proven to be challenging since libgcc_s.so is a required dependency in the Linux standard base. Some software is transitively dependent on libgcc because glibc makes hardcoded calls to functions in libgcc_s. For example, the function __GI___backtrace eventually makes its way to a hardcoded dlopen to libgcc_s' _Unwind_Backtrace. Since libgcc and libunwind have the same ABI, but different implementations, the two libraries end up cross-talking, which ultimately results in a segfault.

To solve this problem, libunwind needs to build a “libgcc” that is, link the necessary functions from compiler-rt and libunwind into an archive and shared object that advertise themselves as libgcc.a, libgcc_eh.a, and libgcc_s.so, so that glibc’s baked calls are diverted to the correct objects in memory. Fortunately for us, compiler-rt and libunwind use the same ABI as the libgcc family, so the problem is solvable at the llvm-project configuration level: no program source needs to be edited. Thus, the end result is for a user to configure their LLVM build with a flag that indicates they want to archive compiler-rt/unwind as libgcc. We achieve this by compiling libunwind with all the symbols necessary for compiler-rt to emulate the libgcc family, and then generate symlinks named for our "libgcc" that point to their corresponding libunwind counterparts.

We alternatively considered patching glibc so that the source doesn't directly refer to libgcc, but rather _defaults_ to libgcc, so that a system preferring compiler-rt/libunwind can point to these libraries at the config stage instead. Even if we modified the Linux standard base, this alternative won't work because binaries that are built using libgcc will still end up having crosstalk between the differing implementations.

This problem has been solved in this manner for FreeBSD, and this CL has been tested against Chrome OS.

This has a dependency on a compiler-rt-aware compiler.

Diff Detail

Event Timeline

cjdb created this revision.Jul 23 2021, 1:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 1:03 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
cjdb requested review of this revision.Jul 23 2021, 1:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 1:03 PM
cjdb edited the summary of this revision. (Show Details)Jul 23 2021, 1:07 PM
cjdb updated this revision to Diff 361316.Jul 23 2021, 1:10 PM

rebases to reactivate CI

If I'm understanding correctly, there are two components to this: shared libraries, and static libraries.

If you have both libgcc_s.so and libunwind.so in the same process, exposing APIs with the same names, the result is confusing at best, sure. Either we need APIs with different names, or libunwind.so needs to replace libgcc_s.so. Ideally, we'd do both, to give users flexibility. But if we're not going to rename the libunwind.so APIs, we shouldn't install libunwind.so at all, since it isn't usable.

For the static libraries, I'm not sure what problem you're trying to solve. If someone tries to link against both sets of static libraries, that's pretty clearly user error. And the symbols have hidden visibility, so they shouldn't interfere with other shared libraries.

cjdb added a comment.Jul 23 2021, 1:56 PM

If I'm understanding correctly, there are two components to this: shared libraries, and static libraries.

If you have both libgcc_s.so and libunwind.so in the same process, exposing APIs with the same names, the result is confusing at best, sure. Either we need APIs with different names, or libunwind.so needs to replace libgcc_s.so. Ideally, we'd do both, to give users flexibility. But if we're not going to rename the libunwind.so APIs, we shouldn't install libunwind.so at all, since it isn't usable.

The problem here isn't that the process wants to use both libgcc_s.so and libunwind.so, but rather that it sometimes "needs" to. glibc has a hardcoded dlopens to libgcc_s.so, so anyone using glibc will find libgcc_s.so is inescapable even when using libunwind.so, which rules out both an API rename and replacing libgcc_s.so.

For the static libraries, I'm not sure what problem you're trying to solve. If someone tries to link against both sets of static libraries, that's pretty clearly user error. And the symbols have hidden visibility, so they shouldn't interfere with other shared libraries.

This solves a different part of the crosstalk problem. While trying to migrate Chrome OS from libgcc to compiler-rt and libunwind, I observed that there was crosstalk when statically linking objects too. In this case, I believe the crosstalk is between true libgcc.a and libunwind.so.

dxf added a subscriber: dxf.Jul 23 2021, 2:14 PM

If I'm understanding correctly, there are two components to this: shared libraries, and static libraries.

If you have both libgcc_s.so and libunwind.so in the same process, exposing APIs with the same names, the result is confusing at best, sure. Either we need APIs with different names, or libunwind.so needs to replace libgcc_s.so. Ideally, we'd do both, to give users flexibility. But if we're not going to rename the libunwind.so APIs, we shouldn't install libunwind.so at all, since it isn't usable.

The problem here isn't that the process wants to use both libgcc_s.so and libunwind.so, but rather that it sometimes "needs" to. glibc has a hardcoded dlopens to libgcc_s.so, so anyone using glibc will find libgcc_s.so is inescapable even when using libunwind.so, which rules out both an API rename and replacing libgcc_s.so.

I meant that there can only be one "true" unwinder, that exposes the libgcc_s.so symbols, and it has to be named libgcc_s.so. Having a libunwind.so installed on the same system exposing the same symbols is just confusing at best, even if it sort of works if they're the same version.

So one of the following should be true on a system with a library named libgcc_s.so:

  1. libunwind.so isn't be installed on the system.
  2. libunwind.so entry points forward to libgcc_s.so
  3. libunwind.so exposes APIs with different names, if someone wanted to explicitly use it for some reason. (Not sure if this is a likely use-case.)

For the static libraries, I'm not sure what problem you're trying to solve. If someone tries to link against both sets of static libraries, that's pretty clearly user error. And the symbols have hidden visibility, so they shouldn't interfere with other shared libraries.

This solves a different part of the crosstalk problem. While trying to migrate Chrome OS from libgcc to compiler-rt and libunwind, I observed that there was crosstalk when statically linking objects too. In this case, I believe the crosstalk is between true libgcc.a and libunwind.so.

If there's crosstalk between libgcc.a and libunwind.so, binaries built on systems with true libgcc.a will break on systems with libunwind.so installed as libgcc_s.so. Messing with the names is just covering up the issue.

cjdb added a comment.Jul 23 2021, 3:07 PM

If I'm understanding correctly, there are two components to this: shared libraries, and static libraries.

If you have both libgcc_s.so and libunwind.so in the same process, exposing APIs with the same names, the result is confusing at best, sure. Either we need APIs with different names, or libunwind.so needs to replace libgcc_s.so. Ideally, we'd do both, to give users flexibility. But if we're not going to rename the libunwind.so APIs, we shouldn't install libunwind.so at all, since it isn't usable.

The problem here isn't that the process wants to use both libgcc_s.so and libunwind.so, but rather that it sometimes "needs" to. glibc has a hardcoded dlopens to libgcc_s.so, so anyone using glibc will find libgcc_s.so is inescapable even when using libunwind.so, which rules out both an API rename and replacing libgcc_s.so.

I meant that there can only be one "true" unwinder, that exposes the libgcc_s.so symbols, and it has to be named libgcc_s.so. Having a libunwind.so installed on the same system exposing the same symbols is just confusing at best, even if it sort of works if they're the same version.

So one of the following should be true on a system with a library named libgcc_s.so:

  1. libunwind.so isn't be installed on the system.
  2. libunwind.so entry points forward to libgcc_s.so
  3. libunwind.so exposes APIs with different names, if someone wanted to explicitly use it for some reason. (Not sure if this is a likely use-case.)

(1) isn't interesting, and I think (3) is a non-starter in this case. (2) sounds to me like you're suggesting libunwind.so be a shell that points to true libgcc_s.so, which is the opposite of this patch. Could you please clarify what you mean here?
If a vendor builds libunwind using -DLIBUNWIND_SYNTH_LIBGCC=On, then that means they want libunwind to be the unwinder and there is some unfortunate circumstance that is requiring libunwind.so to masquerade as libgcc_s.so.

For the static libraries, I'm not sure what problem you're trying to solve. If someone tries to link against both sets of static libraries, that's pretty clearly user error. And the symbols have hidden visibility, so they shouldn't interfere with other shared libraries.

This solves a different part of the crosstalk problem. While trying to migrate Chrome OS from libgcc to compiler-rt and libunwind, I observed that there was crosstalk when statically linking objects too. In this case, I believe the crosstalk is between true libgcc.a and libunwind.so.

If there's crosstalk between libgcc.a and libunwind.so, binaries built on systems with true libgcc.a will break on systems with libunwind.so installed as libgcc_s.so. Messing with the names is just covering up the issue.

Some systems (e.g. Chrome OS) don't have this problem because true libgcc.a isn't available.

If there's crosstalk between libgcc.a and libunwind.so, binaries built on systems with true libgcc.a will break on systems with libunwind.so installed as libgcc_s.so. Messing with the names is just covering up the issue.

After this patch we'll no longer install true libgcc* libraries on system anymore to avoid any crosstalk issue. llvm's libunwind will be the only one present with anything libgcc* symlinked to libunwind/compiler-rt etc. This way, we will not have any real libgcc dependencies in Chrome OS or crosstalk.

So one of the following should be true on a system with a library named libgcc_s.so:

  1. libunwind.so isn't be installed on the system.
  2. libunwind.so entry points forward to libgcc_s.so
  3. libunwind.so exposes APIs with different names, if someone wanted to explicitly use it for some reason. (Not sure if this is a likely use-case.)

(1) isn't interesting, and I think (3) is a non-starter in this case. (2) sounds to me like you're suggesting libunwind.so be a shell that points to true libgcc_s.so, which is the opposite of this patch. Could you please clarify what you mean here?
If a vendor builds libunwind using -DLIBUNWIND_SYNTH_LIBGCC=On, then that means they want libunwind to be the unwinder and there is some unfortunate circumstance that is requiring libunwind.so to masquerade as libgcc_s.so.

Well, I suppose there is option (4), which is "libgcc_s.so entry points forward to libunwind.so".

What this patch implements, as far as I can tell, is "There are two copies of libunwind in the process, one named libunwind.so, and one named libgcc_s.so. If we're lucky, everything works." Unless I'm missing something?

If there's crosstalk between libgcc.a and libunwind.so, binaries built on systems with true libgcc.a will break on systems with libunwind.so installed as libgcc_s.so. Messing with the names is just covering up the issue.

After this patch we'll no longer install true libgcc* libraries on system anymore to avoid any crosstalk issue. llvm's libunwind will be the only one present with anything libgcc* symlinked to libunwind/compiler-rt etc. This way, we will not have any real libgcc dependencies in Chrome OS or crosstalk.

I'd rather not be in a situation where a program built with a normal production toolchain appears to work, but then randomly crashes because the unwinder clashes with libgcc. Unless Chrome OS actually refuses to load binaries built with a normal Linux toolchain, we should ensure those binaries work.

cjdb added a comment.Jul 23 2021, 5:02 PM

So one of the following should be true on a system with a library named libgcc_s.so:

  1. libunwind.so isn't be installed on the system.
  2. libunwind.so entry points forward to libgcc_s.so
  3. libunwind.so exposes APIs with different names, if someone wanted to explicitly use it for some reason. (Not sure if this is a likely use-case.)

(1) isn't interesting, and I think (3) is a non-starter in this case. (2) sounds to me like you're suggesting libunwind.so be a shell that points to true libgcc_s.so, which is the opposite of this patch. Could you please clarify what you mean here?
If a vendor builds libunwind using -DLIBUNWIND_SYNTH_LIBGCC=On, then that means they want libunwind to be the unwinder and there is some unfortunate circumstance that is requiring libunwind.so to masquerade as libgcc_s.so.

Well, I suppose there is option (4), which is "libgcc_s.so entry points forward to libunwind.so".

What this patch implements, as far as I can tell, is "There are two copies of libunwind in the process, one named libunwind.so, and one named libgcc_s.so. If we're lucky, everything works." Unless I'm missing something?

I don't see why there would ever be two copies of libunwind. libgcc_s.so is a symlink to libunwind.so when installed with LIBUNWIND_SYNTH_LIBGCC=On, so an attempt to load libgcc_s.so would mean attempting to reload the original shared object. Having said that, processes needn't explicitly link with libunwind when their system vends using this setup because libgcc_s.so is libunwind.so, and so even if my understanding about how shared objects are loaded is wrong, the problem should be avoided nonetheless.

So one of the following should be true on a system with a library named libgcc_s.so:

  1. libunwind.so isn't be installed on the system.
  2. libunwind.so entry points forward to libgcc_s.so
  3. libunwind.so exposes APIs with different names, if someone wanted to explicitly use it for some reason. (Not sure if this is a likely use-case.)

(1) isn't interesting, and I think (3) is a non-starter in this case. (2) sounds to me like you're suggesting libunwind.so be a shell that points to true libgcc_s.so, which is the opposite of this patch. Could you please clarify what you mean here?
If a vendor builds libunwind using -DLIBUNWIND_SYNTH_LIBGCC=On, then that means they want libunwind to be the unwinder and there is some unfortunate circumstance that is requiring libunwind.so to masquerade as libgcc_s.so.

Well, I suppose there is option (4), which is "libgcc_s.so entry points forward to libunwind.so".

What this patch implements, as far as I can tell, is "There are two copies of libunwind in the process, one named libunwind.so, and one named libgcc_s.so. If we're lucky, everything works." Unless I'm missing something?

Oh, wait, I am missing something: you're installing a symlink, and dlopen determines whether a shared library is the same based on inode, not the path. So I guess this works out in its current form.

Oh, wait, I am missing something: you're installing a symlink, and dlopen determines whether a shared library is the same based on inode, not the path. So I guess this works out in its current form.

Err, no, that's not right; the CMake steps in the patch create a symlink in the build directory, then install() copies the whole library to the install directory, so it isn't symlinked anymore.

Having said that, processes needn't explicitly link with libunwind when their system vends using this setup because libgcc_s.so is libunwind.so, and so even if my understanding about how shared objects are loaded is wrong, the problem should be avoided nonetheless.

If nothing actually links to the library named libunwind.so, why install a library with that name?

I'd rather not be in a situation where a program built with a normal production toolchain appears to work, but then randomly crashes because the unwinder clashes with libgcc. Unless Chrome OS actually refuses to load binaries built with a normal Linux toolchain, we should ensure those binaries work.

Yes, Chrome OS actually requires compiling everything using the Chrome OS toolchain/SDK. Using binaries built using any other toolchain or Linux distros is not allowed so we are mostly fine in this regard.

I'd rather not be in a situation where a program built with a normal production toolchain appears to work, but then randomly crashes because the unwinder clashes with libgcc. Unless Chrome OS actually refuses to load binaries built with a normal Linux toolchain, we should ensure those binaries work.

Yes, Chrome OS actually requires compiling everything using the Chrome OS toolchain/SDK. Using binaries built using any other toolchain or Linux distros is not allowed so we are mostly fine in this regard.

Okay.

In that case, please make sure the CMake option explicitly notes this compatibility constraint.

cjdb added a comment.Jul 23 2021, 7:34 PM

Oh, wait, I am missing something: you're installing a symlink, and dlopen determines whether a shared library is the same based on inode, not the path. So I guess this works out in its current form.

Err, no, that's not right; the CMake steps in the patch create a symlink in the build directory, then install() copies the whole library to the install directory, so it isn't symlinked anymore.

My Chrome OS build shows this.

$ ls -l /usr/lib64/libgcc_s*
lrwxrwxrwx. 1 root root 12 Jul 21 23:31 /usr/lib64/libgcc_s.so -> libunwind.so
lrwxrwxrwx. 1 root root 14 Jul 21 23:31 /usr/lib64/libgcc_s.so.1 -> libunwind.so.1

Those look like symlinks to me?

Having said that, processes needn't explicitly link with libunwind when their system vends using this setup because libgcc_s.so is libunwind.so, and so even if my understanding about how shared objects are loaded is wrong, the problem should be avoided nonetheless.

If nothing actually links to the library named libunwind.so, why install a library with that name?

Two reasons:

  1. It could possibly break projects that do opt into using libunwind and the library isn't present.
  2. It'd be weird for libunwind to install libgcc* on its lonesome.
MaskRay added a subscriber: MaskRay.EditedJul 23 2021, 8:33 PM

I try to play with this patch and run into problems.
I have a -DLLVM_TARGETS_TO_BUILD=host -DLIBUNWIND_USE_COMPILER_RT=on -DLIBUNWIND_SYNTH_LIBGCC=x86_64 build at /tmp/out/custom1.
synth_libgcc fails:

% ninja -C /tmp/out/custom1 synth_libgcc
ninja: Entering directory `/tmp/out/custom1'
[1/1] cd /tmp/out/custom1/projects/libunwind/src && /usr/bin/cmake -E create_symlink  /tmp/out/custom1/./lib/libgcc.a
FAILED: projects/libunwind/src/CMakeFiles/synth_libgcc
cd /tmp/out/custom1/projects/libunwind/src && /usr/bin/cmake -E create_symlink  /tmp/out/custom1/./lib/libgcc.a
...

LIBUNWIND_BUILTINS_LIBRARY is empty in my build, so my libunwind.so.1.0 doesn't have builtins symbols.
if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG OR LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG) evaluates to true.


The intention of LIBUNWIND_BUILTINS_LIBRARY appears to merge libclang_rt.builtins-* and libunwind into libunwind.so.1.0.
Then a symlink libgcc_s.so.1 is provided.

But why is --version-script with some GCC_* version nodes used?
libclang_rt.builtins-* and libunwind lack quite a few symbols from a real libgcc_s.so.1.
Many prebuilt applications with a libgcc_s.so.1 DT_NEEDED entry can probably run with the fake libgcc_s.so.1, but many won't.

The reliable way is to rebuild these programs, instead of assuming libunwind+libclang_rt.builtins can provide the compatibility.

If such prebuilt programs are relatively few, just drop their symbol versioning (.gnu.version* sections and DT_GNU_VER* dynamic tags)

cp in.so out.so
r2 -wqc '/x feffff6f @ section..dynamic; w0 8 @ hit0_0' out.so
llvm-objcopy -R .gnu.version out.so

libunwind/gcc_s-i386.ver
__dlopen

A version script only affects defined symbols. Patterns like __dlopen are not defined and should be removed.

} Unwind_Only;

The syntax is completed ignored in GNU ld and ld.lld. There is no dependency. Just use };

You can remove global:. It is ignored.


Enabling libunwind as a replacement for libgcc on Linux has proven to be challenging since libgcc_s.so is a required dependency in the Linux standard base.
https://refspecs.linuxbase.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/libgcc-s.html

The latest version is 5 which is available on https://refspecs.linuxfoundation.org/lsb.shtml


For example, the function GI_backtrace eventually makes its way to a hardcoded dlopen to libgcc_s' _Unwind_Backtrace. Since libgcc and libunwind have the same ABI, but different implementations, the two libraries end up cross-talking, which ultimately results in a segfault.

POSIX does not require that pthread_cancel unwinds the stack and executes landing pads (usually local variable destructors from -fexceptions C++ code).
glibc and FreeBSD implementations provide the feature. musl doesn't.

My opinion is that this is an unneeded feature. The landing pad execution doesn't help a lot of -fno-exceptions code anyway.

If you can patch glibc, you may just drop the __libc_dlopen (LIBGCC_S_SO); and dlsym _Unwind_* code.
Then your glibc will not dlopen libgcc.so.1, and the problem will be solved.

cjdb updated this revision to Diff 361785.Jul 26 2021, 1:50 PM

updates CMake option description and adds documentation for how to use this feature

cjdb added a subscriber: echristo.Jul 26 2021, 1:51 PM

I try to play with this patch and run into problems.
I have a -DLLVM_TARGETS_TO_BUILD=host -DLIBUNWIND_USE_COMPILER_RT=on -DLIBUNWIND_SYNTH_LIBGCC=x86_64 build at /tmp/out/custom1.
synth_libgcc fails:

% ninja -C /tmp/out/custom1 synth_libgcc
ninja: Entering directory `/tmp/out/custom1'
[1/1] cd /tmp/out/custom1/projects/libunwind/src && /usr/bin/cmake -E create_symlink  /tmp/out/custom1/./lib/libgcc.a
FAILED: projects/libunwind/src/CMakeFiles/synth_libgcc
cd /tmp/out/custom1/projects/libunwind/src && /usr/bin/cmake -E create_symlink  /tmp/out/custom1/./lib/libgcc.a
...

LIBUNWIND_BUILTINS_LIBRARY is empty in my build, so my libunwind.so.1.0 doesn't have builtins symbols.
if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG OR LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG) evaluates to true.

LIBUNWIND_BUILTINS_LIBRARY may require you to do a two stage build but this is related to the pre-existing LIBUNWIND_USE_COMPILER_RT option, not because of any changes in this patch. Stage1: Build clang and compiler-rt. Stage 2: Use the freshly-built clang to compile libunwind.

I’ve added a manual that explains how to build using this feature in the latest revision. PTAL and try again.


The intention of LIBUNWIND_BUILTINS_LIBRARY appears to merge libclang_rt.builtins-* and libunwind into libunwind.so.1.0.
Then a symlink libgcc_s.so.1 is provided.

But why is --version-script with some GCC_* version nodes used?
libclang_rt.builtins-* and libunwind lack quite a few symbols from a real libgcc_s.so.1.

Many prebuilt applications with a libgcc_s.so.1 DT_NEEDED entry can probably run with the fake libgcc_s.so.1, but many won't.

Our experience with testing this on the whole of Chrome OS suggests that what’s available is sufficient for our use case.

The reliable way is to rebuild these programs, instead of assuming libunwind+libclang_rt.builtins can provide the compatibility.

We build (almost) everything in Chrome OS from source and have very few vendor binaries so compatibility is not a huge problem for us.

This is mostly a patch to assist distros that can manage their libraries (e.g. Chrome OS, FreeBSD, etc.); it’s not useful for average LLVM users to build for themselves. This feature is opt-in: anyone wanting to use this patch must’ve figured out their compatibility requirements beforehand.

If such prebuilt programs are relatively few, just drop their symbol versioning (.gnu.version* sections and DT_GNU_VER* dynamic tags)

cp in.so out.so
r2 -wqc '/x feffff6f @ section..dynamic; w0 8 @ hit0_0' out.so
llvm-objcopy -R .gnu.version out.so

libunwind/gcc_s-i386.ver
__dlopen

A version script only affects defined symbols. Patterns like __dlopen are not defined and should be removed.

Thanks for noticing, I’ll audit the version script again to remove symbols not provided by compiler-rt. I’ll need a day or so to look into this as I’ll be testing this on Chrome OS CQ before pushing here.

} Unwind_Only;

The syntax is completed ignored in GNU ld and ld.lld. There is no dependency. Just use };

You can remove global:. It is ignored.

Thanks for noting this. My understanding of the ld documentation suggests that we should beep both of these. I’ll play with stuff and report back, but this will require at least 48h.


Enabling libunwind as a replacement for libgcc on Linux has proven to be challenging since libgcc_s.so is a required dependency in the Linux standard base.
https://refspecs.linuxbase.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/libgcc-s.html

The latest version is 5 which is available on https://refspecs.linuxfoundation.org/lsb.shtml

Thanks, but LSB 5 still has the same requirements.


For example, the function GI_backtrace eventually makes its way to a hardcoded dlopen to libgcc_s' _Unwind_Backtrace. Since libgcc and libunwind have the same ABI, but different implementations, the two libraries end up cross-talking, which ultimately results in a segfault.

POSIX does not require that pthread_cancel unwinds the stack and executes landing pads (usually local variable destructors from -fexceptions C++ code).
glibc and FreeBSD implementations provide the feature. musl doesn't.

My opinion is that this is an unneeded feature. The landing pad execution doesn't help a lot of -fno-exceptions code anyway.

If you can patch glibc, you may just drop the __libc_dlopen (LIBGCC_S_SO); and dlsym _Unwind_* code.
Then your glibc will not dlopen libgcc.so.1, and the problem will be solved.

The POSIX spec isn’t relevant for applications that are already dependent on glibc behaviour. The commit message shows an example calling the backtrace function, which isn’t pthread-related. LSB requirement also makes it a non-starter for a glibc patch, and use of -static-libgcc will still require a dlopen.

Please keep in mind that this patch isn’t for mainstream developers, but for distro managers who are looking to completely replace libgcc in their system.

cjdb removed subscribers: echristo, MaskRay.
cjdb updated this revision to Diff 365837.Aug 11 2021, 1:23 PM
  • rebases since it's been a while
  • revises version scripts so that they're exactly ((compiler-rt-symbols ∪ llvm-libunwind-symbols) ∩ libgcc_s-symbols)
  • removes global: and dependency labels

@MaskRay I think I've now addressed all your feedback.

compnerd requested changes to this revision.Aug 11 2021, 1:42 PM
compnerd added subscribers: beanz, compnerd.

I think that this should be moved into a subdirectory in llvm/runtimes that @beanz has setup already. It is pulling pieces from the compiler-rt builtins and libunwind and is not really isolated into one or the other. Additionally, this really does deserve a warning that this requires careful setup and can cause issues.

This revision now requires changes to proceed.Aug 11 2021, 1:42 PM
MaskRay added a comment.EditedAug 11 2021, 1:48 PM

(I have implemented some missing parts of ld.lld symbol versioning recently: D107235 D107234 D107535. This ensures if we ever have a non-default version symbol, the version script should be good.)

My understanding of this patch:
Synthesize a symbol versioned libgcc_s.so.1 from libclang_rt.builtins-*.a and libunwind.a.

Many applications built with GCC libgcc_s.so.1 can run with their libgcc_s.so.1 runtime replaced with the synthesized one.

I may conjecture people's concern with the patch:

  • maintenance burden. This is a new build way most users don't exercise (for now). Is unintentionally breaking ChromeOS builds ground for reverting their patches, assuming their patches are otherwise good.
  • does this script fit in libunwind/ or should it sit somewhere else?
  • do the set of symbols enable good code sharing? Many platforms have their customization. Personally I think the code sharing may be less than what people might feel about

I am chatting with a few others on thoughts:)


Having a version script for every architecture is difficult to maintain.
For a new symbol, a contributor needs to figure out every version map.

I think two better alternatives:

  • use C preprocessed version script file with conditional macros like #ifdef __x86_64__.
  • concatenate the base version script with an arch-specific version script.

Most symbols are shared anyway.

libunwind/gcc_s-aarch64.ver
147

This just makes the symbol emutls_init@@Unversioned. It doesn't mean unversioned.

You can just omit the symbols.

local: *; will localize these unversioned symbols, so local: *; cannot be used.

MaskRay added inline comments.Aug 11 2021, 1:50 PM
libunwind/gcc_s-armv7a.ver
105

You may place more symbols on one line if that can improve readability.

Perhaps some comments as well.

beanz added a comment.Aug 11 2021, 2:02 PM

I agree with @compnerd that this would make more sense as a feature of the runtimes build directory since it is bringing together pieces from multiple runtimes (libunwind & compiler-rt).

libunwind/CMakeLists.txt
82

I'm confused about this option. Is it a bool as documented, or a string as implemented? Please clarify.

MaskRay added a subscriber: dim.Aug 11 2021, 3:01 PM

Personally I think the code sharing this patch may enable is less than what people may feel about.
Different platforms may not use the same set of source files.
But I want authoritative opinions from FreeBSD (@dim):)

cjdb abandoned this revision.Aug 19 2021, 3:28 PM

We considered an approach to do this in libunwind but reviewers opinion was to make libgcc its own separate build. After some back and forth, we agreed that this would probably be the best path forward. This effort is now being tracked by D108416.