[llvm-libgcc] initial commit

[cjdb]

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][5]. 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][1]. Since libgcc_{eh.a,s.so} and
libunwind have the same ABI, but different implementations, the two
libraries end up [cross-talking, which ultimately results in a
segfault][2].

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][3], and this
CL has been tested against [Chrome OS][4].

[1]: https://github.com/bminor/glibc/blob/master/sysdeps/arm/backtrace.c#L68
[2]: https://bugs.chromium.org/p/chromium/issues/detail?id=1162190#c16
[3]: https://github.com/freebsd/freebsd-src/tree/main/lib/libgcc_s
[4]: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/2945947
[5]: https://refspecs.linuxbase.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/libgcc-s.html

[cjdb]

changes how the version script is laid out so that global symbols are all listed first, and exclusive symbols are listed afterward

[MaskRay]

llvm/tools/llvm-libgcc/lib/gcc_s.ver

16 ↗

(On Diff #404168)

You may use __LP64__

20 ↗

(On Diff #404168)

__LP64__

Unsure anyone will use this for ILP64. Sticking with LP64 and !__LP64__ should be fine.

[cjdb]

changes how GLOBAL_32BIT and GLOBAL_64BIT are enabled

llvm/tools/llvm-libgcc/lib/gcc_s.ver

20 ↗

(On Diff #404168)

Eh, it's not that much extra work to support it, especially if we use #else on the tail of 64-bit detection.

[arichardson]

llvm/tools/llvm-libgcc/lib/gcc_s.ver

20 ↗

(On Diff #404168)

If you use __SIZEOF_POINTER__ >= 8 it should also do the right thing when compiling for Arm Morello (which is L64P128)

[cjdb]

changes to SIZEOF_POINTER >= 8

[danielkiss]

llvm/tools/llvm-libgcc/lib/gcc_s.ver

4 ↗

(On Diff #403735)

__ARM_FP >= 0x04 is better because on some targets the 64bit operations are available.

[cjdb]

broadens armhf detection

[MaskRay]

Thanks for the update. From a high level,

• llvm-libgcc looks useful to me
• the refactored gcc_s.ver looks good to me.
• I'd wish that some CMake experts (e.g. @phosek @compnerd) can review the CMake logic, which is a significant portion of this patch. I am hesitant to LGTM this part.
• the motivation in the summary and the documentation focus on glibc loading libgcc_s.so.1, and people may wonder whether other systems (e.g. Linux musl) need this? If glibc is patched to not dlopen libgcc_so.so.1, is it still a problem? I think the symbol version will remain as a problem for GCC prebuilt objects. The documentation just doesn't make this as a clear motivation.
• apparently many folks are interested (therefore the comments). Ideally hope someone can stand up to test that the symbol versioning result is indeed closer to libgcc_s.so.1 ... (Sorry, it may not be me.)

[tschuett]

llvm/tools/llvm-libgcc/generate_version_script.py

1 ↗

(On Diff #405035)

#!/usr/bin/env python3 looks safer.

[compnerd]

llvm/tools/llvm-libgcc/CMakeLists.txt

7 ↗

(On Diff #405035)

Perhaps we can default the path since the layout is now fairly uniform due to the merged repository layout.

36 ↗

(On Diff #405035)

I think that bin rather than lib is appropriate. The CMAKE_RUNTIME_OUTPUT_DIRECTORY is used for the executables and dlls on Windows.

llvm/tools/llvm-libgcc/FindCompilerRT.cmake

1 ↗

(On Diff #405035)

I think that this is not really the way to do this. This should follow the standard CMake expectations around the behaviour of Find*.cmake. Alternatively, you could name this something else and completely elide the find_* usage.

91 ↗

(On Diff #405035)

This should be hoisted into the top level CMake. The FindCompilerRT.cmake should only search for the prebuilt version.

llvm/tools/llvm-libgcc/FindLLVMLibunwind.cmake

1 ↗

(On Diff #405035)

Similar

llvm/tools/llvm-libgcc/lib/CMakeLists.txt

17 ↗

(On Diff #405035)

add_custom_command by itself is difficult to use, please create a target to track the output so that dependencies can be wired properly.

30 ↗

(On Diff #405035)

Is there a reason to not use -nostdlib as a driver level option?

33 ↗

(On Diff #405035)

Is there a guarantee that we are not building these libraries? If we are building these libraries, I think that it would be better to use the target objects (i.e. $<TARGET_OBJECTS:unwind> rather than the --whole-archive, --no-whole-archive path.

34 ↗

(On Diff #405035)

I think that this should be outside of the -(, -).

38 ↗

(On Diff #405035)

This seems like an odd check. I think that we should drop it, libc is required and unless you are suggesting that it is possible to have a freestanding build of this library, we should assume that it is available. (I think that it is still problematic for non-Unix platforms due to spelling but we can deal with that if/when necessary as libgcc is not something that really applies there).

55 ↗

(On Diff #405035)

I would rather use ${CMAKE_SHARED_LIBRARY_PREFIX} and ${CMAKE_SHARED_LIBRARY_SUFFIX}. rather than lib and .so.

59 ↗

(On Diff #405035)

I think that the versioned symlinks should point to the same names not to the peers.

[cjdb]

gets CMake scripts into a decent state

[cjdb]

quick CMake fix

[compnerd]

llvm-libgcc/CMakeLists.txt

11

Any reason for insert rather that append?

37

I tend to prefer all caps for the On and Off, but its equivalent really.

llvm-libgcc/lib/CMakeLists.txt

4

newline before this line would be nice.

9

I tend to prefer the custom command before the custom target so you can see what the target is generating.

18

What differentiates this from the gcc_s_ver target? I think that you have two targets for the same output.

20

I think that breaking after the ( is a bit unnecessary:

set_target_properties(gcc_s_version_script PROPERTIES
  OUTPUT_PATH "${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver")

Similar throughout.

55

Dead code?

57

Why is this cached?

[compnerd]

I'd like to see at least https://reviews.llvm.org/D108416#inline-1146215, https://reviews.llvm.org/D108416#inline-1146214, and https://reviews.llvm.org/D108416#inline-1146209 addressed pre-commit. Would be nice to have the other ones resolved as well, but they are more stylistic.

[zero9178]

llvm-libgcc/docs/LLVMLibgcc.rst

97

neither compiler-rt, nor llvm-libgcc, are listed in LLVM_ENABLE_RUNTIMES

Is this a typo and is meant to be libunwind instead of llvm-libgcc?

[EricWF]

LGTM on behalf of libc++. But please address the comments left by the other reviewers.

[MaskRay]

llvm-libgcc/docs/LLVMLibgcc.rst

98

I think some folks prefer that compiler-rt is added to LLVM_ENABLE_RUNTIMES. How difficult is it to make this work?

Note that current plan is that libunwind in DLLVM_ENABLE_PROJECTS will be unsupported.

llvm-libgcc/generate_version_script.py

2

/usr/bin/env python3

Add a file-level comment what this script does.

[ldionne]

runtimes/CMakeLists.txt

181–200

I think this can be simplified like this:

if("llvm-libgcc" IN_LIST runtimes)
  if("compiler-rt" IN_LIST runtimes OR "compiler-rt" IN_LIST LLVM_ENABLE_PROJECTS)
    message(FATAL_ERROR
      "Attempting to build both compiler-rt and llvm-libgcc will cause irreconcilable "
      "target clashes. Please choose one or the other, but not both.")
  endif()

  if("libunwind" IN_LIST runtimes)
    message(
      FATAL_ERROR
      "Attempting to build both libunwind and llvm-libgcc will cause irreconcilable "
      "target clashes. Please choose one or the other, but not both.")
  endif()
endif()

[cjdb]

responds to feedback

llvm-libgcc/CMakeLists.txt

11

I copied this directly from either libunwind/CMakeLists.txt or runtimes/CMakeLists.txt. Looks as if it's being prepended though?

37

Acknowledged.

llvm-libgcc/docs/LLVMLibgcc.rst

97

Thanks!

98

compiler-rt must be listed in neither LLVM_ENABLE_RUNTIMES, nor LLVM_ENABLE_PROJECTS when llvm-libgcc is present. llvm-libgcc will take care of that for you. You can still have LLVM_ENABLE_RUNTIMES=compiler-rt, but not when llvm-libgcc is also in the list.

llvm-libgcc/generate_version_script.py

2

Done, but what's the benefit of this?

llvm-libgcc/lib/CMakeLists.txt

20

I find the current way to improve readability a lot.

57

I think this was user-settable at one point, but per yesterday, this entire variable is redundant.

runtimes/CMakeLists.txt

181–200

Thanks, this is way simpler!

[compnerd]

I believe that the break after ( in CMake is counter the style of the rest of the project. I'd recommend against it.

[cjdb]

cleans up a few things

[cjdb]

• makes it harder to accidentally build llvm-libgcc by removing it from LLVM_ENABLE_RUNTIMES=all
• installs shared objects properly

[cjdb]

rebases to activate CI

[phosek]

LGTM