rpms / clang

Clone
Source Code
GIT

#95 set CLANG_DEFAULT_UNWIND_LIB instead of using custom patch
Merged 3 years ago by sergesanspaille. Opened 3 years ago by tbaeder.
Unknown source rawhide  into  rawhide

file removed
-51 
@@ -1,51 +0,0 @@
 

- From 27000b14439d22145487f55f8a85eb5f8cc98e30 Mon Sep 17 00:00:00 2001
 

- From: serge-sans-paille <sguelton@redhat.com>
 

- Date: Thu, 25 Feb 2021 14:08:28 +0100
 

- Subject: [PATCH 2/6] [PATCH][clang] ToolChain: Add -lgcc_s to the linker flags
 

-  when using libc++
 

- 

- The libc++ build for Fedora does not include an implementation of
 

- libunwind, so we need to explicitly link against something that
 

- provides this implementation.
 

- ---
 

-  clang/lib/Driver/ToolChain.cpp | 1 +
 

-  clang/test/Driver/netbsd.cpp   | 4 ++--
 

-  2 files changed, 3 insertions(+), 2 deletions(-)
 

- 

- diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
 

- index b2ddef1..c4c71e1 100644
 

- --- a/clang/lib/Driver/ToolChain.cpp
 

- +++ b/clang/lib/Driver/ToolChain.cpp
 

- @@ -1014,6 +1014,7 @@ void ToolChain::AddCXXStdlibLibArgs(const ArgList &Args,
 

-    switch (Type) {
 

-    case ToolChain::CST_Libcxx:
 

-      CmdArgs.push_back("-lc++");
 

- +    CmdArgs.push_back("-lgcc_s");
 

-      break;
 

-  

-    case ToolChain::CST_Libstdcxx:
 

- diff --git a/clang/test/Driver/netbsd.cpp b/clang/test/Driver/netbsd.cpp
 

- index 4af7d83..ff18c62 100644
 

- --- a/clang/test/Driver/netbsd.cpp
 

- +++ b/clang/test/Driver/netbsd.cpp
 

- @@ -131,7 +131,7 @@
 

-  // ARM-7: clang{{.*}}" "-cc1" "-triple" "armv5e-unknown-netbsd7.0.0-eabi"
 

-  // ARM-7: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/libexec/ld.elf_so"
 

-  // ARM-7: "-o" "a.out" "{{.*}}/usr/lib{{/|\\\\}}crt0.o" "{{.*}}/usr/lib{{/|\\\\}}eabi{{/|\\\\}}crti.o"
 

- -// ARM-7: "{{.*}}/usr/lib{{/|\\\\}}crtbegin.o" "{{.*}}.o" "-lc++" "-lm" "-lc"
 

- +// ARM-7: "{{.*}}/usr/lib{{/|\\\\}}crtbegin.o" "{{.*}}.o" "-lc++" "-lgcc_s" "-lm" "-lc"
 

-  // ARM-7: "{{.*}}/usr/lib{{/|\\\\}}crtend.o" "{{.*}}/usr/lib{{/|\\\\}}crtn.o"
 

-  

-  // AARCH64: clang{{.*}}" "-cc1" "-triple" "aarch64-unknown-netbsd"
 

- @@ -250,7 +250,7 @@
 

-  // S-ARM-7: clang{{.*}}" "-cc1" "-triple" "armv5e-unknown-netbsd7.0.0-eabi"
 

-  // S-ARM-7: ld{{.*}}" "--eh-frame-hdr" "-Bstatic"
 

-  // S-ARM-7: "-o" "a.out" "{{.*}}/usr/lib{{/|\\\\}}crt0.o" "{{.*}}/usr/lib{{/|\\\\}}eabi{{/|\\\\}}crti.o"
 

- -// S-ARM-7: "{{.*}}/usr/lib{{/|\\\\}}crtbegin.o" "{{.*}}.o" "-lc++" "-lm" "-lc"
 

- +// S-ARM-7: "{{.*}}/usr/lib{{/|\\\\}}crtbegin.o" "{{.*}}.o" "-lc++" "-lgcc_s" "-lm" "-lc"
 

-  // S-ARM-7: "{{.*}}/usr/lib{{/|\\\\}}crtend.o" "{{.*}}/usr/lib{{/|\\\\}}crtn.o"
 

-  

-  // S-AARCH64: clang{{.*}}" "-cc1" "-triple" "aarch64-unknown-netbsd"
 

- -- 

- 1.8.3.1
 

-

file modified
+7 -5 
@@ -4,7 +4,7 @@
 

  %global min_ver 0
 

  %global patch_ver 0
 

  %global rc_ver 3
 

- %global baserelease 5
 

+ %global baserelease 6
 

  

  %global clang_tools_binaries \
 

  	%{_bindir}/clang-apply-replacements \
 
@@ -85,13 +85,11 @@
 

  %endif
 

  Source4:	tstellar-gpg-key.asc
 

  

- 

  %if !0%{?compat_build}
 

  Patch21:	completion-model-cmake.patch
 

  %endif
 

  

  Patch4:		0001-PATCH-clang-Reorganize-gtest-integration.patch
 

- Patch11:	0002-PATCH-clang-ToolChain-Add-lgcc_s-to-the-linker-flags.patch
 

  Patch13:    0003-PATCH-clang-Make-funwind-tables-the-default-on-all-a.patch
 

  Patch15:    0004-PATCH-clang-Don-t-install-static-libraries.patch
 

  Patch17:    0005-PATCH-clang-Prefer-gcc-toolchains-with-libgcc_s.so-w.patch
 
@@ -277,7 +275,6 @@
 

  %setup -q -n %{clang_srcdir}
 

  

  %patch4  -p2 -b .gtest
 

- %patch11 -p2 -b .libcxx-fix
 

  %patch13 -p2 -b .unwind-default
 

  %patch15 -p2 -b .no-install-static
 

  %patch17 -p2 -b .check-gcc_s
 
@@ -369,7 +366,8 @@
 

  	\
 

  	-DCLANG_BUILD_EXAMPLES:BOOL=OFF \
 

  	-DBUILD_SHARED_LIBS=OFF \
 

- 	-DCLANG_REPOSITORY_STRING="%{?fedora:Fedora}%{?rhel:Red Hat} %{version}-%{release}"
 

+ 	-DCLANG_REPOSITORY_STRING="%{?fedora:Fedora}%{?rhel:Red Hat} %{version}-%{release}" \
 

+ 	-DCLANG_DEFAULT_UNWINDLIB=libgcc
 

  

  %cmake_build
 

  
@@ -539,6 +537,10 @@
 

  

  %endif
 

  %changelog
 

+ * Mon Mar 15 2021 Timm Bäder <tbaeder@redhat.com> 12.0.0-0.6.rc3
 

+ - Set CLANG_DEFAULT_UNWIND_LIB instead of using custom patch
 

+ - Add toolchains test to the tests.yml
 

+ 

  * Thu Mar 11 2021 sguelton@redhat.com - 12.0.0-0.5.rc3
 

  - LLVM 12.0.0 rc3

file modified
+2 
@@ -18,6 +18,7 @@
 

        - compiler-rt
 

        - libcxx-devel
 

        - glibc-devel
 

+       - glibc-static
 

        - gcc
 

        # Required for fedora-flags:
 

        - annobin
 
@@ -39,3 +40,4 @@
 

        - clang/rhbz_1647130
 

        - clang/llvm-toolchain
 

        - clang/fedora-flags
 

+       - clang/toolchains

I think this is the more correct way of doing this?
The runtest.sh that Tom sent me still works after this, but testing it on my local machine is a drag. I hope we run some testing on PRs in pagure? :)

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

We haven't added that toolchains test to clang yet, so maybe it would be a good idea to add it with this PR. You just need to add it to the tests.yml flle.

rebased onto d335464f7fe7bcfb0dd492de4be96c0d9742ba22
3 years ago

rebased onto 47b5634d0694a9e460bf814738b369b92e78c4c4
3 years ago

Build succeeded.

rebased onto 0fb931a638ab834708d1f91df8cd5c0bc07ad72f
3 years ago

Looks like the tests worked. Just rebased on the latest rawhide.

The original patch uses libgcc_s and your patch uses libgcc. Any reason why we shouldn't use the shared library?

No reason other than using a provided way of using the gcc unwind library instead of a downstream patch.

I do see -lgcc_s as well in the linker command line though:

build ❯ bin/clang++ ./hello.cpp -stdlib=libc++ -###
(lots omitted)
"-lc++" "-lm" "-lgcc_s" "-lgcc" "-lc" "-lgcc_s" "-lgcc"

Build succeeded.

I think it's better to use the shared library version instead of the static one (smaller binaries)

I get both -lgcc and -lgcc_s in the linker command line, regardless of stdlib:

$ clang++ -stdlib=libstdc++ ./hello.cpp -###
"-lstdc++" "-lm" "-lgcc_s" "-lgcc" "-lc" "-lgcc_s" "-lgcc"
$ clang++ -stdlib=libc++ ./hello.cpp -###
"-lc++" "-lgcc_s" "-lm" "-lgcc_s" "-lgcc" "-lc" "-lgcc_s" "-lgcc"

This is with system-installed clang 10.0.1 on F32.

The latter with an additional -lgcc_s directly after -lc++ since that's where the gcc_s patch adds it.
Edited 3 years ago by tbaeder

rebased onto 1536b825e1221cd40ac0bcbebca65af35cc43380
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

This will need a rebase, and I'd like to have 's/ligcc/libgcc_s/' if that works - it enforces the use of shared library which looks better.

libgcc_s is not a valid value for CLANG_DEFAULT_UNWIND_LIB, so that won't work. We can of course keep carrying the (now larger) downstream patch but only add libgcc_s if we're not doing a static build. I would like to avoid a downstream patch and I'm not sure if it makes any difference in practice.

rebased onto 6b7502fcf198e976d41425c016b53d8e5b13d5e1
3 years ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

To clarify further, passing "libgcc" as CLANG_DEFAULT_UNWIND_LIB does not automatically add libgcc instread of libgcc_s. It just means that ToolChain::getUnwindLibType() now returns UNW_LibGcc: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L944 instead of UNW_None previously.

In our case, that's important inside AddUnwindLibrary(), which adds either ilbgcc_s or libgcc_eh: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/CommonArgs.cpp#L1416-L1422

When I compile a clang locally with CLANG_DEFAULT_UNWIND_LIB set, I get an executable that links against libgcc_s, both when using libc++ and libc++:

build ❯ bin/clang++ ~/hello.cpp
build ❯ ldd ./a.out 
    linux-vdso.so.1 (0x00007ffe893f7000)
    libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f2327a39000)
    libm.so.6 => /lib64/libm.so.6 (0x00007f23278f4000)
    libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f23278d9000)
    libc.so.6 => /lib64/libc.so.6 (0x00007f232770f000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f2327c53000)
build ❯ bin/clang++ ~/hello.cpp -stdlib=libc++
build ❯ ldd ./a.out
    linux-vdso.so.1 (0x00007ffd9ebb9000)
    libc++.so.1 => /lib64/libc++.so.1 (0x00007fbf1194c000)
    libc++abi.so.1 => /lib64/libc++abi.so.1 (0x00007fbf11913000)
    libm.so.6 => /lib64/libm.so.6 (0x00007fbf117ce000)
    libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fbf117b3000)
    libc.so.6 => /lib64/libc.so.6 (0x00007fbf115e9000)
    libpthread.so.0 => /lib64/../lib64/libpthread.so.0 (0x00007fbf115c7000)
    /lib64/ld-linux-x86-64.so.2 (0x00007fbf11a46000)

The patch we remove here also breaks https://bugzilla.redhat.com/show_bug.cgi?id=1921951

rebased onto 1a476e520d1d7dee9cf49b557ff4eebd39805ba2
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto fb4ec027d8b1ff427f6486f910fbf99c64c2ecf8
3 years ago

The test failures even seem vaguely related (since they mention nounwind in the function attrs) but I cannot reproduce them locally.

rebased onto 283f230
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

That's a great simplification, thanks!

Pull-Request has been merged by sergesanspaille
3 years ago
Metadata
None
No Tags
Changes Summary 3
-51
file removed
0002-PATCH-clang-ToolChain-Add-lgcc_s-to-the-linker-flags.patch
+7 -5
file changed
clang.spec
+2 -0
file changed
tests/tests.yml
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.