Skip to content

Commit

Permalink
Auto merge of #127926 - Oneirical:classical-orctestra, r=<try>
Browse files Browse the repository at this point in the history
Migrate `foreign-double-unwind`, `issue-36710` and `foreign-exceptions` `run-make` tests to rmake

Part of #121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).

~~This is expected to fail CI, I am getting a weird `core dumped` error on `foreign-double-unwind` locally. Posting this to start a discussion.~~

EDIT: I got it, `foreign_double_unwind` is *supposed* to fail in the execution stage, but this wasn't obvious, because the original test was just piping the stdout to `CGREP` (which silently ignores errors).

try-job: aarch64-apple
try-job: armhf-gnu
try-job: test-various
try-job: x86_64-msvc
try-job: x86_64-gnu-llvm-18
try-job: i686-msvc
  • Loading branch information
bors committed Jul 25, 2024
2 parents e7d66ea + 84b648c commit 82ce034
Show file tree
Hide file tree
Showing 13 changed files with 207 additions and 48 deletions.
24 changes: 23 additions & 1 deletion src/tools/run-make-support/src/external_deps/c_build.rs
Original file line number Diff line number Diff line change
@@ -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") };
Expand All @@ -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)
}
59 changes: 59 additions & 0 deletions src/tools/run-make-support/src/external_deps/cc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg(path.as_ref());
Expand Down Expand Up @@ -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++"],
}
}
}
58 changes: 58 additions & 0 deletions src/tools/run-make-support/src/external_deps/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -314,4 +315,61 @@ 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() {
// 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()[..] {
"Darwin" => Some("-lc++"),
"FreeBSD" | "SunOS" | "OpenBSD" => None,
_ => Some("-lstdc++"),
}
};
if let Some(flag) = flag {
self.cmd.arg(flag);
}
self
}
}
2 changes: 1 addition & 1 deletion src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,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;
Expand Down
3 changes: 0 additions & 3 deletions src/tools/tidy/src/allowed_run_make_makefiles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,11 @@ run-make/extern-fn-reachable/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
run-make/incr-foreign-head-span/Makefile
run-make/interdependent-c-libraries/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
Expand Down
File renamed without changes.
File renamed without changes.
27 changes: 27 additions & 0 deletions tests/run-make/cpp-global-destructors/rmake.rs
Original file line number Diff line number Diff line change
@@ -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");
}
12 changes: 0 additions & 12 deletions tests/run-make/foreign-double-unwind/Makefile

This file was deleted.

21 changes: 21 additions & 0 deletions tests/run-make/foreign-double-unwind/rmake.rs
Original file line number Diff line number Diff line change
@@ -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_fail, rustc};

fn main() {
build_native_static_lib_cxx("foo");
rustc().input("foo.rs").arg("-lfoo").extra_rs_cxx_flags().run();
run_fail("foo").assert_stdout_not_contains("unreachable");
}
12 changes: 0 additions & 12 deletions tests/run-make/foreign-exceptions/Makefile

This file was deleted.

18 changes: 18 additions & 0 deletions tests/run-make/foreign-exceptions/rmake.rs
Original file line number Diff line number Diff line change
@@ -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");
}
19 changes: 0 additions & 19 deletions tests/run-make/issue-36710/Makefile

This file was deleted.

0 comments on commit 82ce034

Please sign in to comment.