make Cell::swap panic if the Cells partially overlap

[RalfJung]

The following function ought to be sound:

fn as_cell_of_array<T, const N: usize>(c: &[Cell<T>; N]) -> &Cell<[T; N]> {
    unsafe { transmute(c) }
}

However, due to Cell::swap, it currently is not -- safe code can cause a use-after-free. This PR fixes that.

Fixes #80778

[rustbot]

r? @scottmcm

(rustbot has picked a reviewer for you, use r? to override)

[scottmcm]

Pondering: would it be reasonable for this to mem::swap(&mut*self.value.get(), &mut*other.value.get()) now that they're non-overlapping? Or is it important that it stays in pointers here?

r=me either way from a code perspective.

[RalfJung]

We could use mem::swap, yeah. What would be the advantage of that?

[scottmcm]

Probably doesn't make a substantial difference. I think of it as simpler since it doesn't need the loop (usually), but I suppose it'll still instantiate the ptr::swap_nonoverlapping code anyway.

[scottmcm]

@rust-lang/libs-api I was about to sign off on this, but started wondering if it needs an FCP first.

This is a soundness fix, but it also adds a new panic path, so I guess it probably should have an FCP?

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[dtolnay]

Thanks!

[dtolnay]

@bors r+

[bors]

📌 Commit e7a1e42 has been approved by dtolnay

It is now in the queue for this repository.

[bors]

⌛ Testing commit e7a1e42 with merge f6faef4...

[bors]

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing f6faef4 to master...

[rust-timer]

Finished benchmarking commit (f6faef4): 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

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)	2.0%	[0.7%, 3.2%]	6
Regressions ❌ (secondary)	2.0%	[0.8%, 2.9%]	6
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-1.4%	[-2.2%, -0.6%]	2
All ❌✅ (primary)	1.6%	[-0.5%, 3.2%]	7

Cycles

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.0%	[-5.0%, -0.8%]	10
Improvements ✅ (secondary)	-1.1%	[-1.1%, -1.1%]	1
All ❌✅ (primary)	-3.0%	[-5.0%, -0.8%]	10

Binary size

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

Bootstrap: 630.078s -> 631.441s (0.22%)
Artifact size: 316.22 MiB -> 316.24 MiB (0.01%)

[scottmcm]

Wow! Impressive cycle count perf improvements -- if they're real.