From e3cf7e53339be5b8061c21b1e05f229d46d72efc Mon Sep 17 00:00:00 2001 From: Oneirical Date: Thu, 18 Jul 2024 12:10:38 -0400 Subject: [PATCH 1/3] rewrite foreign-double-unwind to rmake --- .../src/external_deps/c_build.rs | 24 +++++++- .../run-make-support/src/external_deps/cc.rs | 59 +++++++++++++++++++ .../src/external_deps/rustc.rs | 43 ++++++++++++++ src/tools/run-make-support/src/lib.rs | 2 +- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/foreign-double-unwind/Makefile | 12 ---- tests/run-make/foreign-double-unwind/rmake.rs | 21 +++++++ 7 files changed, 147 insertions(+), 15 deletions(-) delete mode 100644 tests/run-make/foreign-double-unwind/Makefile create mode 100644 tests/run-make/foreign-double-unwind/rmake.rs diff --git a/src/tools/run-make-support/src/external_deps/c_build.rs b/src/tools/run-make-support/src/external_deps/c_build.rs index 35b2bf75c95f5..2c4fdcdb82879 100644 --- a/src/tools/run-make-support/src/external_deps/c_build.rs +++ b/src/tools/run-make-support/src/external_deps/c_build.rs @@ -1,12 +1,13 @@ use std::path::PathBuf; use crate::artifact_names::static_lib_name; -use crate::external_deps::cc::cc; +use crate::external_deps::cc::{cc, cxx}; use crate::external_deps::llvm::llvm_ar; use crate::path_helpers::path; use crate::targets::is_msvc; /// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. +/// Built from a C file. #[track_caller] pub fn build_native_static_lib(lib_name: &str) -> PathBuf { let obj_file = if is_msvc() { format!("{lib_name}") } else { format!("{lib_name}.o") }; @@ -25,3 +26,24 @@ pub fn build_native_static_lib(lib_name: &str) -> PathBuf { llvm_ar().obj_to_ar().output_input(&lib_path, &obj_file).run(); path(lib_path) } + +/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. +/// Built from a C++ file. +#[track_caller] +pub fn build_native_static_lib_cxx(lib_name: &str) -> PathBuf { + let obj_file = if is_msvc() { format!("{lib_name}") } else { format!("{lib_name}.o") }; + let src = format!("{lib_name}.cpp"); + let lib_path = static_lib_name(lib_name); + if is_msvc() { + cxx().arg("-EHs").arg("-c").out_exe(&obj_file).input(src).run(); + } else { + cxx().arg("-c").out_exe(&obj_file).input(src).run(); + }; + let obj_file = if is_msvc() { + PathBuf::from(format!("{lib_name}.obj")) + } else { + PathBuf::from(format!("{lib_name}.o")) + }; + llvm_ar().obj_to_ar().output_input(&lib_path, &obj_file).run(); + path(lib_path) +} diff --git a/src/tools/run-make-support/src/external_deps/cc.rs b/src/tools/run-make-support/src/external_deps/cc.rs index 840bfa0d2b46e..6d1f19d45756d 100644 --- a/src/tools/run-make-support/src/external_deps/cc.rs +++ b/src/tools/run-make-support/src/external_deps/cc.rs @@ -15,6 +15,12 @@ pub fn cc() -> Cc { Cc::new() } +/// Construct a new platform-specific CXX compiler invocation. +#[track_caller] +pub fn cxx() -> Cc { + Cc::cxx() +} + /// A platform-specific C compiler invocation builder. The specific C compiler used is /// passed down from compiletest. #[derive(Debug)] @@ -44,6 +50,21 @@ impl Cc { Self { cmd } } + /// Construct a new platform-specific C compiler invocation. + #[track_caller] + pub fn cxx() -> Self { + let compiler = env_var("CXX"); + + let mut cmd = Command::new(compiler); + + let default_cflags = env_var("CXX_DEFAULT_FLAGS"); + for flag in default_cflags.split(char::is_whitespace) { + cmd.arg(flag); + } + + Self { cmd } + } + /// Specify path of the input file. pub fn input>(&mut self, path: P) -> &mut Self { self.cmd.arg(path.as_ref()); @@ -192,3 +213,41 @@ pub fn extra_cxx_flags() -> Vec<&'static str> { } } } + +/// `EXTRARSCXXFLAGS` +pub fn extra_rs_cxx_flags() -> Vec<&'static str> { + // Adapted from tools.mk (trimmed): + // + // ```makefile + // ifdef IS_WINDOWS + // ifdef IS_MSVC + // else + // EXTRARSCXXFLAGS := -lstatic:-bundle=stdc++ + // endif + // else + // ifeq ($(UNAME),Darwin) + // EXTRARSCXXFLAGS := -lc++ + // else + // ifeq ($(UNAME),FreeBSD) + // else + // ifeq ($(UNAME),SunOS) + // else + // ifeq ($(UNAME),OpenBSD) + // else + // EXTRARSCXXFLAGS := -lstdc++ + // endif + // endif + // endif + // endif + // endif + // ``` + if is_windows() { + if is_msvc() { vec![] } else { vec!["-lstatic:-bundle=stdc++"] } + } else { + match &uname()[..] { + "Darwin" => vec!["-lc++"], + "FreeBSD" | "SunOS" | "OpenBSD" => vec![], + _ => vec!["-lstdc++"], + } + } +} diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index 71d28dd9675fb..cabfbd59c482e 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -5,6 +5,7 @@ use crate::command::Command; use crate::env::env_var; use crate::path_helpers::cwd; use crate::util::set_host_rpath; +use crate::{is_msvc, is_windows, uname}; /// Construct a new `rustc` invocation. This will automatically set the library /// search path as `-L cwd()`. Use [`bare_rustc`] to avoid this. @@ -314,4 +315,46 @@ impl Rustc { self.cmd.arg(format!("-Clinker-flavor={linker_flavor}")); self } + + /// `EXTRARSCXXFLAGS` + pub fn extra_rs_cxx_flags(&mut self) -> &mut Self { + // Adapted from tools.mk (trimmed): + // + // ```makefile + // ifdef IS_WINDOWS + // ifdef IS_MSVC + // else + // EXTRARSCXXFLAGS := -lstatic:-bundle=stdc++ + // endif + // else + // ifeq ($(UNAME),Darwin) + // EXTRARSCXXFLAGS := -lc++ + // else + // ifeq ($(UNAME),FreeBSD) + // else + // ifeq ($(UNAME),SunOS) + // else + // ifeq ($(UNAME),OpenBSD) + // else + // EXTRARSCXXFLAGS := -lstdc++ + // endif + // endif + // endif + // endif + // endif + // ``` + let flag = if is_windows() { + if is_msvc() { None } else { Some("-lstatic:-bundle=stdc++") } + } else { + match &uname()[..] { + "Darwin" => Some("-lc++"), + "FreeBSD" | "SunOS" | "OpenBSD" => None, + _ => Some("-lstdc++"), + } + }; + if let Some(flag) = flag { + self.cmd.arg(flag); + } + self + } } diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index e6a45f57de608..b1208aa720aa8 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -37,7 +37,7 @@ pub use wasmparser; pub use external_deps::{c_build, cc, clang, htmldocck, llvm, python, rustc, rustdoc}; // These rely on external dependencies. -pub use c_build::build_native_static_lib; +pub use c_build::{build_native_static_lib, build_native_static_lib_cxx}; pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc}; pub use clang::{clang, Clang}; pub use htmldocck::htmldocck; diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 745f00c4f5276..52bc1d7824835 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -29,7 +29,6 @@ run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile run-make/extern-multiple-copies2/Makefile run-make/fmt-write-bloat/Makefile -run-make/foreign-double-unwind/Makefile run-make/foreign-exceptions/Makefile run-make/foreign-rust-exceptions/Makefile run-make/incr-add-rust-src-component/Makefile diff --git a/tests/run-make/foreign-double-unwind/Makefile b/tests/run-make/foreign-double-unwind/Makefile deleted file mode 100644 index b5e52808d2fc5..0000000000000 --- a/tests/run-make/foreign-double-unwind/Makefile +++ /dev/null @@ -1,12 +0,0 @@ -# ignore-cross-compile -# needs-unwind -include ../tools.mk - -all: foo - $(call RUN,foo) | $(CGREP) -v unreachable - -foo: foo.rs $(call NATIVE_STATICLIB,foo) - $(RUSTC) $< -lfoo $(EXTRARSCXXFLAGS) - -$(TMPDIR)/libfoo.o: foo.cpp - $(call COMPILE_OBJ_CXX,$@,$<) diff --git a/tests/run-make/foreign-double-unwind/rmake.rs b/tests/run-make/foreign-double-unwind/rmake.rs new file mode 100644 index 0000000000000..b2ac8bfbeadf1 --- /dev/null +++ b/tests/run-make/foreign-double-unwind/rmake.rs @@ -0,0 +1,21 @@ +// When using foreign function interface (FFI) with C++, it is possible +// to run into a "double unwind" if either both Rust and C++ run into a panic +// and exception at the same time, or C++ encounters two exceptions. In this case, +// one of the panic unwinds would be leaked and the other would be kept, leading +// to undefined behaviour. After this was fixed in #92911, this test checks that +// the keyword "unreachable" indicative of this bug triggering in this specific context +// does not appear after successfully compiling and executing the program. +// See https://github.com/rust-lang/rust/pull/92911 + +//@ needs-unwind +// Reason: this test exercises panic unwinding +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib_cxx, run, rustc}; + +fn main() { + build_native_static_lib_cxx("foo"); + rustc().input("foo.rs").arg("-lfoo").extra_rs_cxx_flags().run(); + run("foo").assert_stdout_not_contains("unreachable"); +} From 796d8e9d664a39a02b1db998ebddd84418a06aae Mon Sep 17 00:00:00 2001 From: Oneirical Date: Thu, 18 Jul 2024 13:04:45 -0400 Subject: [PATCH 2/3] rewrite and rename issue-36710 to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../foo.cpp | 0 .../foo.rs | 0 .../run-make/cpp-global-destructors/rmake.rs | 27 +++++++++++++++++++ tests/run-make/issue-36710/Makefile | 19 ------------- 5 files changed, 27 insertions(+), 20 deletions(-) rename tests/run-make/{issue-36710 => cpp-global-destructors}/foo.cpp (100%) rename tests/run-make/{issue-36710 => cpp-global-destructors}/foo.rs (100%) create mode 100644 tests/run-make/cpp-global-destructors/rmake.rs delete mode 100644 tests/run-make/issue-36710/Makefile diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 52bc1d7824835..45abfb7312628 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -42,7 +42,6 @@ run-make/issue-26006/Makefile run-make/issue-28595/Makefile run-make/issue-33329/Makefile run-make/issue-35164/Makefile -run-make/issue-36710/Makefile run-make/issue-47551/Makefile run-make/issue-69368/Makefile run-make/issue-84395-lto-embed-bitcode/Makefile diff --git a/tests/run-make/issue-36710/foo.cpp b/tests/run-make/cpp-global-destructors/foo.cpp similarity index 100% rename from tests/run-make/issue-36710/foo.cpp rename to tests/run-make/cpp-global-destructors/foo.cpp diff --git a/tests/run-make/issue-36710/foo.rs b/tests/run-make/cpp-global-destructors/foo.rs similarity index 100% rename from tests/run-make/issue-36710/foo.rs rename to tests/run-make/cpp-global-destructors/foo.rs diff --git a/tests/run-make/cpp-global-destructors/rmake.rs b/tests/run-make/cpp-global-destructors/rmake.rs new file mode 100644 index 0000000000000..02928b9de9df3 --- /dev/null +++ b/tests/run-make/cpp-global-destructors/rmake.rs @@ -0,0 +1,27 @@ +// Some start files were missed when originally writing the logic to swap in musl start files. +// This caused #36710. After the fix in #50105, this test checks that linking to C++ code +// with global destructors works. +// See https://github.com/rust-lang/rust/pull/50105 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +// FIXME(Oneirical): are these really necessary? This test is supposed to test a musl +// bug... and it ignores musl? This wasn't part of the original test at its creation, which +// had no ignores. + +//# ignore-none no-std is not supported +//# ignore-wasm32 FIXME: don't attempt to compile C++ to WASM +//# ignore-wasm64 FIXME: don't attempt to compile C++ to WASM +//# ignore-nvptx64-nvidia-cuda FIXME: can't find crate for `std` +//# ignore-musl FIXME: this makefile needs teaching how to use a musl toolchain +//# (see dist-i586-gnu-i586-i686-musl Dockerfile) +//# ignore-sgx + +use run_make_support::{build_native_static_lib_cxx, run, rustc}; + +fn main() { + build_native_static_lib_cxx("foo"); + rustc().input("foo.rs").arg("-lfoo").extra_rs_cxx_flags().run(); + run("foo"); +} diff --git a/tests/run-make/issue-36710/Makefile b/tests/run-make/issue-36710/Makefile deleted file mode 100644 index 7b91107a23495..0000000000000 --- a/tests/run-make/issue-36710/Makefile +++ /dev/null @@ -1,19 +0,0 @@ -# ignore-cross-compile -# ignore-none no-std is not supported -# ignore-wasm32 FIXME: don't attempt to compile C++ to WASM -# ignore-wasm64 FIXME: don't attempt to compile C++ to WASM -# ignore-nvptx64-nvidia-cuda FIXME: can't find crate for `std` -# ignore-musl FIXME: this makefile needs teaching how to use a musl toolchain -# (see dist-i586-gnu-i586-i686-musl Dockerfile) -# ignore-sgx - -include ../tools.mk - -all: foo - $(call RUN,foo) - -foo: foo.rs $(call NATIVE_STATICLIB,foo) - $(RUSTC) $< -lfoo $(EXTRARSCXXFLAGS) --target $(TARGET) - -$(TMPDIR)/libfoo.o: foo.cpp - $(call COMPILE_OBJ_CXX,$@,$<) From 84b648c0d12d9805b06bfdd34e8c52944c423a1d Mon Sep 17 00:00:00 2001 From: Oneirical Date: Thu, 18 Jul 2024 13:12:38 -0400 Subject: [PATCH 3/3] rewrite foreign-exceptions to rmake --- .../src/external_deps/rustc.rs | 15 +++++++++++++++ .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/foreign-double-unwind/rmake.rs | 4 ++-- tests/run-make/foreign-exceptions/Makefile | 12 ------------ tests/run-make/foreign-exceptions/rmake.rs | 18 ++++++++++++++++++ 5 files changed, 35 insertions(+), 15 deletions(-) delete mode 100644 tests/run-make/foreign-exceptions/Makefile create mode 100644 tests/run-make/foreign-exceptions/rmake.rs diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index cabfbd59c482e..07cfd8435abd5 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -344,6 +344,21 @@ impl Rustc { // endif // ``` let flag = if is_windows() { + // So this is a bit hacky: we can't use the DLL version of libstdc++ because + // it pulls in the DLL version of libgcc, which means that we end up with 2 + // instances of the DW2 unwinding implementation. This is a problem on + // i686-pc-windows-gnu because each module (DLL/EXE) needs to register its + // unwind information with the unwinding implementation, and libstdc++'s + // __cxa_throw won't see the unwinding info we registered with our statically + // linked libgcc. + // + // Now, simply statically linking libstdc++ would fix this problem, except + // that it is compiled with the expectation that pthreads is dynamically + // linked as a DLL and will fail to link with a statically linked libpthread. + // + // So we end up with the following hack: we link use static:-bundle to only + // link the parts of libstdc++ that we actually use, which doesn't include + // the dependency on the pthreads DLL. if is_msvc() { None } else { Some("-lstatic:-bundle=stdc++") } } else { match &uname()[..] { diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 45abfb7312628..64d4106f8ef6a 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -29,7 +29,6 @@ run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile run-make/extern-multiple-copies2/Makefile run-make/fmt-write-bloat/Makefile -run-make/foreign-exceptions/Makefile run-make/foreign-rust-exceptions/Makefile run-make/incr-add-rust-src-component/Makefile run-make/incr-foreign-head-span/Makefile diff --git a/tests/run-make/foreign-double-unwind/rmake.rs b/tests/run-make/foreign-double-unwind/rmake.rs index b2ac8bfbeadf1..9bd3b4c0fea5b 100644 --- a/tests/run-make/foreign-double-unwind/rmake.rs +++ b/tests/run-make/foreign-double-unwind/rmake.rs @@ -12,10 +12,10 @@ //@ ignore-cross-compile // Reason: the compiled binary is executed -use run_make_support::{build_native_static_lib_cxx, run, rustc}; +use run_make_support::{build_native_static_lib_cxx, run_fail, rustc}; fn main() { build_native_static_lib_cxx("foo"); rustc().input("foo.rs").arg("-lfoo").extra_rs_cxx_flags().run(); - run("foo").assert_stdout_not_contains("unreachable"); + run_fail("foo").assert_stdout_not_contains("unreachable"); } diff --git a/tests/run-make/foreign-exceptions/Makefile b/tests/run-make/foreign-exceptions/Makefile deleted file mode 100644 index 56c41b274fb2e..0000000000000 --- a/tests/run-make/foreign-exceptions/Makefile +++ /dev/null @@ -1,12 +0,0 @@ -# ignore-cross-compile -# needs-unwind -include ../tools.mk - -all: foo - $(call RUN,foo) - -foo: foo.rs $(call NATIVE_STATICLIB,foo) - $(RUSTC) $< -lfoo $(EXTRARSCXXFLAGS) - -$(TMPDIR)/libfoo.o: foo.cpp - $(call COMPILE_OBJ_CXX,$@,$<) diff --git a/tests/run-make/foreign-exceptions/rmake.rs b/tests/run-make/foreign-exceptions/rmake.rs new file mode 100644 index 0000000000000..dd24b41ca1598 --- /dev/null +++ b/tests/run-make/foreign-exceptions/rmake.rs @@ -0,0 +1,18 @@ +// This test was created to check that compilation and execution still works +// after the addition of a new feature, in #65646: the ability to unwind panics +// and exceptions back and forth between Rust and C++. Should this feature be broken, +// the test should fail. +// See https://github.com/rust-lang/rust/pull/65646 + +//@ needs-unwind +// Reason: this test exercises panic unwinding +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib_cxx, run, rustc}; + +fn main() { + build_native_static_lib_cxx("foo"); + rustc().input("foo.rs").arg("-lfoo").extra_rs_cxx_flags().run(); + run("foo"); +}