Fix - Uses std::mem::transmute and std::ptr::write in unsafe code in append_vec.rs

[Lichtso]

Problem

In nightly rust we will encounter the following error messages:

error: casting &T to &mut T is undefined behavior, even if the reference is
unused, consider instead using an UnsafeCell
--> runtime/src/append_vec.rs:718:17
|
718 | *(&self.meta.data_len as *const u64 as *mut u64) =
new_data_len;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[deny(cast_ref_to_mut)] on by default
error: casting &T to &mut T is undefined behavior, even if the reference is
unused, consider instead using an UnsafeCell
--> runtime/src/append_vec.rs:733:17
|
733 | *(&self.account_meta.executable as *const bool as *mut u8) =
new_executable_byte;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Summary of Changes

Uses explicit std::mem::transmute and std::ptr::write in unsafe code instead.

[codecov]

Codecov Report

Merging #32711 (837bc1b) into master (20fc3a5) will increase coverage by 0.0%.
Report is 3 commits behind head on master.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #32711   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         784      784           
  Lines      210979   210981    +2     
=======================================
+ Hits       173026   173046   +20     
+ Misses      37953    37935   -18     

[brooksprumo]

Both set_data_len_unsafe() and set_executable_as_byte() are annotated with #[allow(clippy::cast_ref_to_mut)]. I would not have expected a clippy error; do you know why this happened?

[Lichtso]

You are right, we can remove that lint now. It is deprecated see: rust-lang/rust#111567

[brooksprumo]

we can remove that lint now

Gotcha. Do you intend to do that within this PR, or a subsequent PR?

[brooksprumo]

Also, code changes look good to me. These functions are only used in tests, so it's easy to audit all their callers.

We may even want to inline the functions into their callers, since they are only called once (get_executable_byte() and set_executable_as_byte()) or twice (set_data_len_unsafe()). This would further restrict their usage and would also put the code directly within the context being called, making inspection easier. Not something that needs to happen in this PR though.

[jeffwashington]

lgtm

[Urgau]

This is still UB. Casting/transmuting a immutable reference to a mutable reference is always UB. The only sound way of mutating an immutable reference to a mutable reference is to use a Cell or UnsafeCell.

See this playground example for proof with Miri.

error: Undefined Behavior: attempting a write access using <1679> at alloc852[0x0], but that tag only grants SharedReadOnly permission for this location
  --> src/main.rs:7:9
   |
7  | /         std::ptr::write(
8  | |             std::mem::transmute::<*const u64, *mut u64>(&data_len),
9  | |             new_data_len,
10 | |         );
   | |         ^
   | |         |
   | |_________attempting a write access using <1679> at alloc852[0x0], but that tag only grants SharedReadOnly permission for this location
   |           this error occurs as part of an access at alloc852[0x0..0x8]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1679> was created by a SharedReadOnly retag at offsets [0x0..0x8]
  --> src/main.rs:8:57
   |
8  |             std::mem::transmute::<*const u64, *mut u64>(&data_len),
   |                                                         ^^^^^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `main` at src/main.rs:7:9: 10:10

NOTE: The current cast_ref_to_mut or the now uplifted invalid_reference_casting lint unfortunately do not (yet) lint against, but I'm planning on fixing it.