Mark .rmeta files as /SAFESEH on x86 Windows. #117115

zetafunction · 2023-10-24T05:08:44Z

Chrome links .rlibs with /WHOLEARCHIVE or -Wl,--whole-archive to prevent the linker from discarding static initializers. This works well, except on Windows x86, where lld complains:

error: /safeseh: lib.rmeta is not compatible with SEH

The fix is simply to mark the .rmeta as SAFESEH aware. This is trivially true, since the metadata file does not contain any executable code.

rustbot · 2023-10-24T05:08:51Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @b-naber (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

zetafunction · 2023-10-24T05:17:33Z

Is there a good place to put a helper function for managing the @feat.00 symbol? Right now, this is copied from

rust/compiler/rustc_codegen_ssa/src/back/link.rs

Line 1901 in b2d2184

 // Add feature flags to the object file. On MSVC this is optional but LLD will complain if 

which I discovered in the course of fixing this bug. @nbdd0121 @petrochenkov

bjorn3 · 2023-10-24T08:29:00Z

You can probably put it in create_object_file and then remove this code from link.rs.

zetafunction · 2023-10-24T20:27:43Z

You can probably put it in create_object_file and then remove this code from link.rs.

Thanks, that's a good suggestion. I moved it to create_object_file(); however, I made it so create_object_file() always returns an object with mangling enabled.

link.rs separately (re-)disables mangling.

I think having create_object_file() silently return an Object with mangling disabled only for COFF objects is too action-at-a-distance.

tests/run-make/windows-safeseh/Makefile

bjorn3 · 2023-10-24T20:40:15Z

Thanks, that's a good suggestion. I moved it to create_object_file(); however, I made it so create_object_file() always returns an object with mangling enabled.

link.rs separately (re-)disables mangling.

I think having create_object_file() silently return an Object with mangling disabled only for COFF objects is too action-at-a-distance.

Makes sense.

LGTM with the above nit fixed.

Chrome links .rlibs with /WHOLEARCHIVE or -Wl,--whole-archive to prevent the linker from discarding static initializers. This works well, except on Windows x86, where lld complains: error: /safeseh: lib.rmeta is not compatible with SEH The fix is simply to mark the .rmeta as SAFESEH aware. This is trivially true, since the metadata file does not contain any executable code.

bjorn3 · 2023-10-24T21:04:27Z

@bors r

bors · 2023-10-24T21:04:29Z

📌 Commit 8fa800d has been approved by bjorn3

It is now in the queue for this repository.

bors · 2023-10-26T04:04:54Z

⌛ Testing commit 8fa800d with merge ccb160d...

bors · 2023-10-26T05:59:41Z

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing ccb160d to master...

rust-timer · 2023-10-26T07:10:44Z

Finished benchmarking commit (ccb160d): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

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)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	1.3%	[1.3%, 1.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

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: missing data
Artifact size: 304.46 MiB -> 304.45 MiB (-0.00%)

rustbot assigned b-naber Oct 24, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 24, 2023

zetafunction marked this pull request as ready for review October 24, 2023 06:13

zetafunction force-pushed the linking branch from c30c74b to 5529fbc Compare October 24, 2023 20:25

bjorn3 reviewed Oct 24, 2023

View reviewed changes

tests/run-make/windows-safeseh/Makefile Outdated Show resolved Hide resolved

zetafunction force-pushed the linking branch from 5529fbc to 8fa800d Compare October 24, 2023 20:42

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 Oct 24, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 26, 2023

bors merged commit ccb160d into rust-lang:master Oct 26, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 26, 2023

bors mentioned this pull request Oct 26, 2023

Add arm64e-apple-ios & arm64e-apple-darwin targets #115526

Merged

zetafunction deleted the linking branch October 26, 2023 08:28

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mark .rmeta files as /SAFESEH on x86 Windows. #117115

Mark .rmeta files as /SAFESEH on x86 Windows. #117115

zetafunction commented Oct 24, 2023

rustbot commented Oct 24, 2023

zetafunction commented Oct 24, 2023

bjorn3 commented Oct 24, 2023

zetafunction commented Oct 24, 2023

bjorn3 commented Oct 24, 2023

bjorn3 commented Oct 24, 2023

bors commented Oct 24, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-timer commented Oct 26, 2023

Mark .rmeta files as /SAFESEH on x86 Windows. #117115

Mark .rmeta files as /SAFESEH on x86 Windows. #117115

Conversation

zetafunction commented Oct 24, 2023

rustbot commented Oct 24, 2023

zetafunction commented Oct 24, 2023

bjorn3 commented Oct 24, 2023

zetafunction commented Oct 24, 2023

bjorn3 commented Oct 24, 2023

bjorn3 commented Oct 24, 2023

bors commented Oct 24, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-timer commented Oct 26, 2023

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size