Skip to content

Commit

Permalink
Auto merge of #126437 - Oneirical:test-we-forget, r=jieyouxu
Browse files Browse the repository at this point in the history
Migrate `issue-64153`, `invalid-staticlib` and `no-builtins-lto` `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).

try-job: x86_64-msvc
  • Loading branch information
bors committed Jun 18, 2024
2 parents c1f62a7 + df6d873 commit af3d100
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 44 deletions.
3 changes: 2 additions & 1 deletion src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc};
pub use clang::{clang, Clang};
pub use diff::{diff, Diff};
pub use llvm::{
llvm_filecheck, llvm_profdata, llvm_readobj, LlvmFilecheck, LlvmProfdata, LlvmReadobj,
llvm_filecheck, llvm_objdump, llvm_profdata, llvm_readobj, LlvmFilecheck, LlvmObjdump,
LlvmProfdata, LlvmReadobj,
};
pub use run::{cmd, run, run_fail, run_with_args};
pub use rustc::{aux_build, rustc, Rustc};
Expand Down
30 changes: 30 additions & 0 deletions src/tools/run-make-support/src/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ pub fn llvm_filecheck() -> LlvmFilecheck {
LlvmFilecheck::new()
}

/// Construct a new `llvm-objdump` invocation. This assumes that `llvm-objdump` is available
/// at `$LLVM_BIN_DIR/llvm-objdump`.
pub fn llvm_objdump() -> LlvmObjdump {
LlvmObjdump::new()
}

/// A `llvm-readobj` invocation builder.
#[derive(Debug)]
#[must_use]
Expand All @@ -44,9 +50,17 @@ pub struct LlvmFilecheck {
cmd: Command,
}

/// A `llvm-objdump` invocation builder.
#[derive(Debug)]
#[must_use]
pub struct LlvmObjdump {
cmd: Command,
}

crate::impl_common_helpers!(LlvmReadobj);
crate::impl_common_helpers!(LlvmProfdata);
crate::impl_common_helpers!(LlvmFilecheck);
crate::impl_common_helpers!(LlvmObjdump);

/// Generate the path to the bin directory of LLVM.
#[must_use]
Expand Down Expand Up @@ -131,3 +145,19 @@ impl LlvmFilecheck {
self
}
}

impl LlvmObjdump {
/// Construct a new `llvm-objdump` invocation. This assumes that `llvm-objdump` is available
/// at `$LLVM_BIN_DIR/llvm-objdump`.
pub fn new() -> Self {
let llvm_objdump = llvm_bin_dir().join("llvm-objdump");
let cmd = Command::new(llvm_objdump);
Self { cmd }
}

/// Provide an input file.
pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg(path.as_ref());
self
}
}
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 @@ -68,7 +68,6 @@ run-make/interdependent-c-libraries/Makefile
run-make/intrinsic-unreachable/Makefile
run-make/invalid-library/Makefile
run-make/invalid-so/Makefile
run-make/invalid-staticlib/Makefile
run-make/issue-107094/Makefile
run-make/issue-109934-lto-debuginfo/Makefile
run-make/issue-14698/Makefile
Expand All @@ -87,7 +86,6 @@ run-make/issue-40535/Makefile
run-make/issue-47384/Makefile
run-make/issue-47551/Makefile
run-make/issue-51671/Makefile
run-make/issue-64153/Makefile
run-make/issue-68794-textrel-on-minimal-lib/Makefile
run-make/issue-69368/Makefile
run-make/issue-83045/Makefile
Expand Down Expand Up @@ -137,7 +135,6 @@ run-make/native-link-modifier-verbatim-rustc/Makefile
run-make/native-link-modifier-whole-archive/Makefile
run-make/no-alloc-shim/Makefile
run-make/no-builtins-attribute/Makefile
run-make/no-builtins-lto/Makefile
run-make/no-duplicate-libs/Makefile
run-make/obey-crate-type-flag/Makefile
run-make/optimization-remarks-dir-pgo/Makefile
Expand Down
5 changes: 0 additions & 5 deletions tests/run-make/invalid-staticlib/Makefile

This file was deleted.

17 changes: 17 additions & 0 deletions tests/run-make/invalid-staticlib/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// If the static library provided is not valid (in this test,
// created as an empty file),
// rustc should print a normal error message and not throw
// an internal compiler error (ICE).
// See https://github.com/rust-lang/rust/pull/28673

use run_make_support::{fs_wrapper, rustc, static_lib_name};

fn main() {
fs_wrapper::create_file(static_lib_name("foo"));
rustc()
.arg("-")
.crate_type("rlib")
.arg("-lstatic=foo")
.run_fail()
.assert_stderr_contains("failed to add native library");
}
26 changes: 0 additions & 26 deletions tests/run-make/issue-64153/Makefile

This file was deleted.

File renamed without changes.
40 changes: 40 additions & 0 deletions tests/run-make/lto-avoid-object-duplication/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// ignore-tidy-tab
// Staticlibs don't include Rust object files from upstream crates if the same
// code was already pulled into the lib via LTO. However, the bug described in
// https://github.com/rust-lang/rust/issues/64153 lead to this exclusion not
// working properly if the upstream crate was compiled with an explicit filename
// (via `-o`).

// This test makes sure that functions defined in the upstream crates do not
// appear twice in the final staticlib when listing all the symbols from it.

//@ ignore-windows
// Reason: `llvm-objdump`'s output looks different on windows than on other platforms.
// Only checking on Unix platforms should suffice.
//FIXME(Oneirical): This could be adapted to work on Windows by checking how
// that output differs.

use run_make_support::{llvm_objdump, regex, rust_lib_name, rustc, static_lib_name};

fn main() {
rustc()
.crate_type("rlib")
.input("upstream.rs")
.output(rust_lib_name("upstream"))
.codegen_units(1)
.run();
rustc()
.crate_type("staticlib")
.input("downstream.rs")
.arg("-Clto")
.output(static_lib_name("downstream"))
.codegen_units(1)
.run();
let syms = llvm_objdump().arg("-t").input(static_lib_name("downstream")).run().stdout_utf8();
let re = regex::Regex::new(r#"\s*g\s*F\s.*issue64153_test_function"#).unwrap();
// Count the global instances of `issue64153_test_function`. There'll be 2
// if the `upstream` object file got erroneously included twice.
// The line we are testing for with the regex looks something like:
// 0000000000000000 g F .text.issue64153_test_function 00000023 issue64153_test_function
assert_eq!(re.find_iter(syms.as_str()).count(), 1);
}
File renamed without changes.
9 changes: 0 additions & 9 deletions tests/run-make/no-builtins-lto/Makefile

This file was deleted.

20 changes: 20 additions & 0 deletions tests/run-make/no-builtins-lto/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// The rlib produced by a no_builtins crate should be explicitely linked
// during compilation, and as a result be present in the linker arguments.
// See the comments inside this file for more details.
// See https://github.com/rust-lang/rust/pull/35637

use run_make_support::{rust_lib_name, rustc};

fn main() {
// Compile a `#![no_builtins]` rlib crate
rustc().input("no_builtins.rs").run();
// Build an executable that depends on that crate using LTO. The no_builtins crate doesn't
// participate in LTO, so its rlib must be explicitly
// linked into the final binary. Verify this by grepping the linker arguments.
rustc()
.input("main.rs")
.arg("-Clto")
.print("link-args")
.run()
.assert_stdout_contains(rust_lib_name("no_builtins"));
}

0 comments on commit af3d100

Please sign in to comment.