cfi: do not transmute function pointers in formatting code

[Darksonn]

Follow-up to #115954.
Addresses #115199 point 2.
Related to #128728.
Discussion on the LKML.

cc @maurer @rcvalle @RalfJung

[rustbot]

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[RalfJung]

From a @rust-lang/opsem perspective (but speaking only for myself), I continue to have my gripes with CFI complaining about code that Rust considers to be entirely well-defined. I don't think we want to make a promise to follow some arbitrary rules that some third-party tool is enforcing. If @rust-lang/libs wants to carry this as a work-around until the situation is resolved, that's fine for me.

The proper fix is to figure out whether we can adjust CFI and the Rust spec to make "code rejected by CFI" a (strict) subset of "code that has UB or EB". But making all fn ptr transmute / type erasure schemes EB doesn't sound good, I assume there's people out there that rely on this working properly.

[rustbot]

Some changes occurred in coverage tests.

cc @Zalathar

[rcvalle]

@rustbot claim

[rcvalle]

@Darksonn since this issue happens with FineIBT only, would it be possible to add a regression test or test for FineIBT? (I don't know whether we can do that with our current test and CI infrastructure tbh.)

[maurer]

Re: FineIBT, that would be difficult to test because it would require us to pull objtool from the Linux kernel into the test environment - it's implemented by editing the compiled code after compilation.

The main way we could add a regression test would be to forbid #[no_sanitize(kcfi)] in core, as the problem with FineIBT is that it does not support not sanitizing a call-site, because the typecheck part of the sanitization is implemented at the call target (it relies on endbr to be able to assume the target will have sanitization).

[rcvalle]

Got it. Thank you, @maurer! SGTM. FYI, @1c3t3a and @jakos-sec are working on fixing all issues listed in #115199 and already fixed the weakly-linked functions issue in #138349 (which unblocked fixing some of the issues listed there), and will soon remove all no_sanitize in core and stdlib.

For this, I guess now for this it's whether the @rust-lang/libs is okay with the small refactoring as @RalfJung mentioned.

[RalfJung]

@bors r-
That gives us the chance to fix the comment. @Darksonn could you also squash all those comment commits into a single commit?

[Darksonn]

Squashed as per your request.

[Darksonn]

@rustbot ready

[m-ou-se]

@bors r+

[bors]

📌 Commit 6513df9 has been approved by m-ou-se

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6513df9 with merge 40dacd5...

[bors]

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 40dacd5 to master...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing f433fa4 (parent) -> 40dacd5 (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Job duration changes

1. dist-aarch64-linux: 8318.3s -> 5158.2s (-38.0%)
2. x86_64-apple-2: 5541.6s -> 6595.0s (19.0%)
3. dist-i686-msvc: 8273.7s -> 7045.0s (-14.9%)
4. dist-android: 2476.6s -> 2702.6s (9.1%)
5. dist-apple-various: 8429.3s -> 7712.8s (-8.5%)
6. dist-powerpc64le-linux: 9336.1s -> 10089.3s (8.1%)
7. x86_64-gnu-distcheck: 4846.0s -> 4460.8s (-8.0%)
8. dist-x86_64-apple: 9388.7s -> 8724.0s (-7.1%)
9. x86_64-apple-1: 8917.4s -> 8366.4s (-6.2%)
10. dist-powerpc64-linux: 5502.1s -> 5831.2s (6.0%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (40dacd5): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	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 (primary -0.6%, secondary -2.6%)

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.2%	[0.4%, 3.6%]	8
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.5%	[-11.9%, -0.7%]	3
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	-0.6%	[-11.9%, 3.6%]	11

Cycles

Results (primary 0.5%, secondary 9.6%)

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.5%	[0.5%, 0.5%]	3
Regressions ❌ (secondary)	9.6%	[9.6%, 9.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.5%	[0.5%, 0.5%]	3

Binary size

Results (primary -0.1%, secondary -0.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.1%	[0.0%, 0.3%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.3%, -0.0%]	46
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	4
All ❌✅ (primary)	-0.1%	[-0.3%, 0.3%]	50

Bootstrap: 781.719s -> 781.212s (-0.06%)
Artifact size: 365.16 MiB -> 365.11 MiB (-0.01%)