tests: Port `extern-fn-reachable` to rmake.rs #135458

jieyouxu · 2025-01-13T21:49:10Z

Summary

This PR ports tests/run-make/extern-fn-reachable to use rmake.rs. Notable changes:

We now use the object crate and look at the exported symbols specifically.
This test's coverage regressed against windows-msvc back in replace dynamic library module with libloading #90716, but since we use object now, we're able to claw the test coverage back.
The checks are now stricter:
1. It no longer looks for substring symbol matches in nm textual outputs, it inspects the symbol names precisely.
2. We now also explicitly check for the presence of leading underscore in exported symbol names on apple vs non-apple targets.
Added another case of #[no_mangle] fn fun6() {} (note the lack of pub) to check that Rust nameres visibility is orthogonal to symbol visibility in dylib.

History

Test was initially introduced as a run-pass¹ test as part of Don't mark reachable extern fns as internal #10539.
Test re-introduced as a run-make test in test: Enable extern-fn-reachable test #13741.
Later, the test coverage regressed in replace dynamic library module with libloading #90716.

Supersedes #128314.
Co-authored with @lolbinarycat.

try-job: x86_64-msvc
try-job: i686-msvc
try-job: i686-mingw
try-job: x86_64-mingw-1
try-job: x86_64-apple-1
try-job: aarch64-apple
try-job: test-various

no longer a thing nowadays ↩

…, r=<try> tests: Port `extern-fn-reachable` to rmake.rs Part of rust-lang#121876. ## Summary This PR ports `tests/run-make/extern-fn-reachable` to use `rmake.rs`. Notable changes: - We now use the `object` crate and look at the exported symbols specifically. - This test's coverage regressed against windows-msvc back in [replace dynamic library module with libloading rust-lang#90716](rust-lang#90716), but since we use `object` now, we're able to claw the test coverage back. - The checks are now stricter: 1. It no longer looks for substring symbol matches in `nm` textual outputs, it inspects the symbol names precisely. 2. We now also explicitly check for the presence of leading underscore in exported symbol names on apple vs non-apple targets. ## History - Test was initially introduced as a run-pass[^run-pass] test as part of [Don't mark reachable extern fns as internal rust-lang#10539](rust-lang#10539). - Test re-introduced as a run-make test in rust-lang#13741. - Later, the test coverage regressed in rust-lang#90716. [^run-pass]: no longer a thing nowadays Supersedes rust-lang#128314. Co-authored with `@lolbinarycat.` r? `@ghost` try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

jieyouxu · 2025-01-14T00:43:08Z

@bors try

bors · 2025-01-14T00:45:08Z

⌛ Trying commit 1332c85 with merge c0a7d85...

…, r=<try> tests: Port `extern-fn-reachable` to rmake.rs Part of rust-lang#121876. ## Summary This PR ports `tests/run-make/extern-fn-reachable` to use `rmake.rs`. Notable changes: - We now use the `object` crate and look at the exported symbols specifically. - This test's coverage regressed against windows-msvc back in [replace dynamic library module with libloading rust-lang#90716](rust-lang#90716), but since we use `object` now, we're able to claw the test coverage back. - The checks are now stricter: 1. It no longer looks for substring symbol matches in `nm` textual outputs, it inspects the symbol names precisely. 2. We now also explicitly check for the presence of leading underscore in exported symbol names on apple vs non-apple targets. - Added another case of `#[no_mangle] fn fun6() {}` (note the lack of `pub`) to check that Rust nameres visibility is orthogonal to symbol visiblity in dylib. ## History - Test was initially introduced as a run-pass[^run-pass] test as part of [Don't mark reachable extern fns as internal rust-lang#10539](rust-lang#10539). - Test re-introduced as a run-make test in rust-lang#13741. - Later, the test coverage regressed in rust-lang#90716. [^run-pass]: no longer a thing nowadays Supersedes rust-lang#128314. Co-authored with `@lolbinarycat.` r? `@ghost` try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

bors · 2025-01-14T03:16:17Z

☀️ Try build successful - checks-actions
Build commit: c0a7d85 (c0a7d85e66e544330818ab687d830297fb9d5883)

jieyouxu · 2025-01-14T03:16:37Z

r? compiler
@rustbot ready

tests/run-make/extern-fn-reachable/rmake.rs

lqd · 2025-01-14T14:25:04Z

I've already started looking at this. Might as well do it all, r? lqd

jieyouxu · 2025-01-14T15:18:20Z

Changes since last review:

Removed the leftover dylib path gymnastics.
Changed symbol checks to be for existence and not for exact count.

tests/run-make/extern-fn-reachable/rmake.rs

Co-authored-by: binarycat <binarycat@envs.net>

lqd · 2025-01-14T17:10:06Z

r=me once you're happy with CI / try builds

jieyouxu · 2025-01-14T17:47:43Z

The test hasn't changed functional-wise since last try job run, and it passes locally so,
@bors r=@lqd rollup=iffy (inspects symbol visiblity that has platform-conditional logic)

bors · 2025-01-14T17:47:45Z

📌 Commit 98f673e has been approved by lqd

It is now in the queue for this repository.

Urgau · 2025-01-14T17:49:37Z

tests/run-make/extern-fn-reachable/rmake.rs

+    println!("expected_symbols = {:?}", expected_symbols);
+    println!("found_symbols = {:?}", found_symbols);
+    if !found_symbols.is_superset(&expected_symbols) {
+        for diff in expected_symbols.difference(&found_symbols) {
+            eprintln!("missing symbol: {}", diff);
+        }
+        panic!("missing expected symbols");
+    }


bors · 2025-01-16T02:31:25Z

⌛ Testing commit 98f673e with merge 5cd16b7...

bors · 2025-01-16T05:13:11Z

☀️ Test successful - checks-actions
Approved by: lqd
Pushing 5cd16b7 to master...

rust-timer · 2025-01-16T06:30:31Z

Finished benchmarking commit (5cd16b7): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary 1.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.6%	[1.0%, 2.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-0.8%, -0.8%]	1
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 761.656s -> 762.718s (0.14%)
Artifact size: 326.09 MiB -> 326.06 MiB (-0.01%)

This comment was marked as outdated.

Sign in to view

jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2025

This was referenced Jan 13, 2025

Tracking Issue for porting run-make tests to use Rust #121876

Closed

port tests/run-make/extern-fn-reachable to rmake #128314

Closed

This comment was marked as outdated.

Sign in to view

jieyouxu marked this pull request as ready for review January 14, 2025 03:16

rustbot assigned lcnr Jan 14, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 14, 2025

lqd reviewed Jan 14, 2025

View reviewed changes

tests/run-make/extern-fn-reachable/rmake.rs Outdated Show resolved Hide resolved

lqd reviewed Jan 14, 2025

View reviewed changes

tests/run-make/extern-fn-reachable/rmake.rs Outdated Show resolved Hide resolved

rustbot assigned lqd and unassigned lcnr Jan 14, 2025

jieyouxu force-pushed the migrate-extern-fn-reachable branch 2 times, most recently from 7887569 to 6d08192 Compare January 14, 2025 15:16

lqd reviewed Jan 14, 2025

View reviewed changes

tests/run-make/extern-fn-reachable/rmake.rs Outdated Show resolved Hide resolved

lqd reviewed Jan 14, 2025

View reviewed changes

tests/run-make/extern-fn-reachable/rmake.rs Show resolved Hide resolved

tests: port extern-fn-reachable to rmake.rs

98f673e

Co-authored-by: binarycat <binarycat@envs.net>

jieyouxu force-pushed the migrate-extern-fn-reachable branch from 6d08192 to 98f673e Compare January 14, 2025 17:05

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2025

Urgau reviewed Jan 14, 2025

View reviewed changes

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 16, 2025

bors merged commit 5cd16b7 into rust-lang:master Jan 16, 2025
7 checks passed

rustbot added this to the 1.86.0 milestone Jan 16, 2025

jieyouxu deleted the migrate-extern-fn-reachable branch January 16, 2025 05:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: Port `extern-fn-reachable` to rmake.rs #135458

tests: Port `extern-fn-reachable` to rmake.rs #135458

jieyouxu commented Jan 13, 2025 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

jieyouxu commented Jan 14, 2025

bors commented Jan 14, 2025

bors commented Jan 14, 2025

jieyouxu commented Jan 14, 2025

lqd commented Jan 14, 2025

jieyouxu commented Jan 14, 2025

lqd commented Jan 14, 2025

jieyouxu commented Jan 14, 2025

bors commented Jan 14, 2025

Urgau Jan 14, 2025

bors commented Jan 16, 2025

bors commented Jan 16, 2025

rust-timer commented Jan 16, 2025

tests: Port extern-fn-reachable to rmake.rs #135458

tests: Port extern-fn-reachable to rmake.rs #135458

Conversation

jieyouxu commented Jan 13, 2025 • edited Loading

Summary

History

Footnotes

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

jieyouxu commented Jan 14, 2025

bors commented Jan 14, 2025

bors commented Jan 14, 2025

jieyouxu commented Jan 14, 2025

lqd commented Jan 14, 2025

jieyouxu commented Jan 14, 2025

lqd commented Jan 14, 2025

jieyouxu commented Jan 14, 2025

bors commented Jan 14, 2025

Urgau Jan 14, 2025

Choose a reason for hiding this comment

bors commented Jan 16, 2025

bors commented Jan 16, 2025

rust-timer commented Jan 16, 2025

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

tests: Port `extern-fn-reachable` to rmake.rs #135458

tests: Port `extern-fn-reachable` to rmake.rs #135458

jieyouxu commented Jan 13, 2025 •

edited

Loading