libunwind fix and cleanup

[12101111]

Fix:

1. "system-llvm-libunwind" now only skip build-script for linux target
2. workaround from Fix libunwind build: Define __LITTLE_ENDIAN__ for LE targets #65972 is not needed, upstream fix it in llvm/llvm-project@68c5070 ( LLVM 11 )
3. remove code for MSCV and Apple in compile(), as they are not used
4. fix Building rustc in Clang 9.0 error: invalid argument '-std=c++11' not allowed with 'C' #69222 , compile c files and cpp files in different config
5. fix conditional compilation for musl target.
6. fix that x86_64-fortanix-unknown-sgx don't link libunwind built in build-script into rlib

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

"system-llvm-libunwind" now only skip build-script for linux target

Can you say more about why? I would expect us to be able to link to a system-installed (rather than locally compiled libunwind) on any platform, is this not true today? Is there some reason we shouldn't try to support this?

workaround from #65972 is not needed, upstream fix it in llvm/llvm-project@68c5070 ( LLVM 11 )

I think we'll need to revert this for now at least, as we currently support a minimum system LLVM version of 10. Good to know there's an upstream fix though!

Otherwise this seems pretty OK to me.

[12101111]

Can you say more about why? I would expect us to be able to link to a system-installed (rather than locally compiled libunwind) on any platform, is this not true today? Is there some reason we shouldn't try to support this?

Currently linux can choose whether to use libunwind or libgcc_s, other platform only support one implement, so enable or disable feature "system-llvm-libunwind" should have no effect.

Linux is special because it's just a kernel, and the userspace can randomly have libgcc_s or libunwind (or even don't either). Other os like BSD, solaris fuchsia, and mingw have a stable userspace.

I think we'll need to revert this for now at least, as we currently support a minimum system LLVM version of 10. Good to know there's an upstream fix though!

The build script will build against the source code from rust fork of llvm, and the version has been updated to 12.0.

rust/library/unwind/build.rs

Line 140 in dc8cb63

let root = Path::new("../../src/llvm-project/libunwind");

The version of llvm codegen backend is irrelevant with libunwind

[bstrie]

@12101111 , are there still changes that you intend to make to this PR?

[12101111]

@12101111 , are there still changes that you intend to make to this PR?

No.

[Mark-Simulacrum]

r? @petrochenkov on the linking concern (I don't have an opinion either way)

Otherwise this seems OK.

[petrochenkov]

I'm ok with addressing #84124 (comment) later if it does get addressed.

[petrochenkov]

I don't understand what happens here with regards to C and C++, in particular what cpp_cfg.cpp(false) means.

• Are we building C++ code with C compiler (gcc, clang, etc), pass C++ flags to it (like -fno-rtti) and hope that it will build it as C++ based on the file extension?
• Why can't we always use a C++ compiler and build everything as C++? Are some targets missing a C++ compiler? If a C++ compiler is missing how can we build C++ code with a C compiler? Are the *.cpp files in libunwind even contain C++ code, or it's just some C++-compatible C code that can be built both as C and as C++?
• Why can't we always use a C compiler if the *.cpp files don't actually contain anything requiring C++?

[petrochenkov]

Could you also add comments explaining what features llvm-libunwind and system-llvm-libunwind mean exactly?

Especially with system-llvm-libunwind it's not clear how something that's ultimately a runtime choice (similar -C link-self-contained) is addressed by a build-time option.

[12101111]

Are we building C++ code with C compiler (gcc, clang, etc), pass C++ flags to it (like -fno-rtti) and hope that it will build it as C++ based on the file extension?

Clang don't allow this. It will trigger an error : #69222

Why can't we always use a C++ compiler and build everything as C++?

GCC don't allow this.

Are some targets missing a C++ compiler?

In our builder, GCC for musl is not built. Instead, we just use the wrapper script musl-gcc provided by musl : https://github.com/rust-lang/rust/blob/master/src/ci/docker/scripts/musl-toolchain.sh#L57. So there is no musl-g++ available in build time.

If a C++ compiler is missing how can we build C++ code with a C compiler? Are the *.cpp files in libunwind even contain C++ code, or it's just some C++-compatible C code that can be built both as C and as C++?

libunwind don't require libc++, just like LLVM libc which is a libc writing in c++, or rust no_std code

Why can't we always use a C compiler if the *.cpp files don't actually contain anything requiring C++?

But it need a real c++ compiler to compile it, and g++ just don't work if libstdc++ is missing.

[petrochenkov]

I've read some docs on the difference between gcc and g++ drivers and things are slightly more clear now.

Clang don't allow this. It will trigger an error : #69222
GCC don't allow this.
we just use the wrapper script musl-gcc provided by musl ... there is no musl-g++ available in build time.

Both clang, gcc and musl-gcc should be able to build everything as C++ (especially in "no-std" mode) if we are passing the language explicitly with -x c++.
Why can't this approach be used? It should be simpler.

But it need a real c++ compiler to compile it

Ok, this part is clear.

libunwind don't require libc++, just like LLVM libc which is a libc writing in c++, or rust no_std code

Ok, I now understand that gcc can still build C++ as language, it just has some different defaults and linked libraries.
Before that it wasn't clear how the C++ standard library question is relevant at all and why did you mention it.

[12101111]

As to features llvm-libunwind and system-llvm-libunwind, It's complicated.

#40113 add crt-static support of musl. This PR break the static build of musl on gnu system, and fixed by #44070. #47663 fix the mips support of libunwind. From that time to now, the linking of musl in crate libunwind is always like this:

#[cfg(target_env = "musl")]
#[link(name = "unwind", kind = "static", cfg(target_feature = "crt-static"))]
#[link(name = "gcc_s", cfg(not(target_feature = "crt-static")))]
extern {}

Note, in that time, libunwind.a is built by a script along with musl:

rust/src/ci/docker/scripts/musl-toolchain.sh

Lines 60 to 69 in f717b58

	# fixme(mati865): Replace it with https://github.com/rust-lang/rust/pull/59089
	mkdir libunwind-build
	cd libunwind-build
	cmake ../libunwind-release_$LLVM \
	-DLLVM_PATH=/build/llvm-release_$LLVM \
	-DLIBUNWIND_ENABLE_SHARED=0 \
	-DCMAKE_C_COMPILER=$CC \
	-DCMAKE_CXX_COMPILER=$CXX \
	-DCMAKE_C_FLAGS="$CFLAGS" \
	-DCMAKE_CXX_FLAGS="$CXXFLAGS"

Later, the llvm-libunwind feature is introduced in #59089, mostly used in fuchsia target, but this feature is also available for linux target. #63173 remove the need of shell script, build libunwind.a for musl in build.rs. Note that musl-g++ is missing in some builders: #63173 (comment). The idea of llvm-libunwind feature is link a static libunwind.a in any time.

Later, PR #59752 break llvm-libunwind feature: #62088, and fixed by #62287. That is why there are many link attributes in library/unwind/src/libunwind.rs

And another PR #72746 break the bootstrap of rust on glibc when llvm-libunwind feature is enabled, See #76020 (comment) and #76020 (comment). To fix this, the system-llvm-libunwind feature is introduced. In this PR, system-llvm-libunwind on glibc target will link to libunwind.so and don't touch the musl part. This PR fix the musl part.

[petrochenkov]

Yeah, the history is complicated.
I'm more interested in having the current state documented, e.g.

• feature = "llvm-libunwind" - use libunwind instead of libgcc if there a choice, do nothing if there's no choice.
• feature = "system-llvm-libunwind" - if libunwind is used and there's a choice, then use libunwind found elsewhere on the system instead of libunwind bundled with Rust distribution, do nothing if libunwind is not used or there's no choice

(I'm not sure that this ^^^ describes the rules correctly, just something that I would expect intuitively.)

[petrochenkov]

@12101111
Could you mark the PR with S-waiting-on-review (https://rustc-dev-guide.rust-lang.org/rustbot.html?highlight=label#issue-relabeling) once you think it's ready?

[12101111]

@rustbot label -S-waiting-on-author +S-waiting-on-review

[12101111]

This is ok:

"sccache" "clang++-11" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=x86_64-fortanix-unknown-sgx" "-static" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=x86_64-fortanix-unknown-sgx" "-D__ELF__" "-isystem/usr/include/x86_64-linux-gnu" "-mlvi-hardening" "-mllvm" "-x86-experimental-lvi-inline-asm-hardening" "-I" "../../src/llvm-project/libunwind/include" "-nostdinc++" "-fno-exceptions" "-fno-rtti" "-fstrict-aliasing" "-funwind-tables" "-fvisibility=hidden" "-fno-stack-protector" "-ffreestanding" "-fexceptions" "-U_FORTIFY_SOURCE" "-D_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" "-D_FORTIFY_SOURCE=0" "-DRUST_SGX=1" "-D__NO_STRING_INLINES" "-D__NO_MATH_INLINES" "-D_LIBUNWIND_IS_BAREMETAL" "-D__LIBUNWIND_IS_NATIVE_ONLY" "-DNDEBUG" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-std/x86_64-fortanix-unknown-sgx/release/build/unwind-755fc94387c8ab25/out/libunwind.o" "-c" "/checkout/src/llvm-project/libunwind/src/libunwind.cpp"

This don't compile and cause error: use of undeclared identifier 'alloca';

"sccache" "clang++-11" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "--target=x86_64-fortanix-unknown-sgx" "-static" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=x86_64-fortanix-unknown-sgx" "-D__ELF__" "-isystem/usr/include/x86_64-linux-gnu" "-mlvi-hardening" "-mllvm" "-x86-experimental-lvi-inline-asm-hardening" "-I" "../../src/llvm-project/libunwind/include" "-nostdinc++" "-fno-exceptions" "-fno-rtti" "-std=c++11" "-fstrict-aliasing" "-funwind-tables" "-fvisibility=hidden" "-fno-stack-protector" "-ffreestanding" "-fexceptions" "-U_FORTIFY_SOURCE" "-D_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" "-D_FORTIFY_SOURCE=0" "-DRUST_SGX=1" "-D__NO_STRING_INLINES" "-D__NO_MATH_INLINES" "-D_LIBUNWIND_IS_BAREMETAL" "-D__LIBUNWIND_IS_NATIVE_ONLY" "-DNDEBUG" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-std/x86_64-fortanix-unknown-sgx/release/build/unwind-58c16432097bb360/out/libunwind.o" "-c" "/checkout/src/llvm-project/libunwind/src/libunwind.cpp"

The only different is -std=c++11. So just don't add this flag to clang.

[petrochenkov]

@bors r+

[bors]

📌 Commit 52a3365 has been approved by petrochenkov

[petrochenkov]

How this worked previously though, when -std=c++11 was passed unconditionally?

[12101111]

In original code, the x86_64-fortanix-unknown-sgx branch don't add -std=c99 and -std=c++11 flags

[bors]

⌛ Testing commit 52a3365 with merge d316c43c5b2160ab17c20acdb44a0008bc2068d9...

[12101111]

broken_heart Test failed - checks-actions

[ALL  ]    --2021-05-26 12:00:31--  (try: 3)  http://isl.gforge.inria.fr/isl-0.20.tar.gz
[ALL  ]    Connecting to isl.gforge.inria.fr (isl.gforge.inria.fr)|128.93.193.15|:80... failed: Connection timed out.
[ALL  ]    Giving up.

Network error.

[petrochenkov]

Looks like CI is generally broken right now, will re-r+ once it's working again.

[petrochenkov]

@bors r+

[bors]

💡 This pull request was already approved, no need to approve it again.

• This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
• There's another pull request that is currently being tested, blocking this pull request: Update RLS #85721

[bors]

📌 Commit 52a3365 has been approved by petrochenkov

[bors]

⌛ Testing commit 52a3365 with merge 9814e83...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 9814e83 to master...