Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to nightly rustc to 2023-04-19 #31381

Merged
merged 12 commits into from
May 11, 2023
2 changes: 1 addition & 1 deletion ci/rust-version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fi
if [[ -n $RUST_NIGHTLY_VERSION ]]; then
nightly_version="$RUST_NIGHTLY_VERSION"
else
nightly_version=2023-03-04
nightly_version=2023-04-19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now's our chance!

Suggested change
nightly_version=2023-04-19
nightly_version=2023-04-20

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm? why bumping the nightly version is needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For maximum memes :)

(but no, not needed)

fi


Expand Down
2 changes: 1 addition & 1 deletion ci/test-checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ fi

_ ci/order-crates-for-publishing.py

nightly_clippy_allows=()
nightly_clippy_allows=(--allow=clippy::redundant_clone)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add this @ryoqun ?

Copy link
Member Author

@ryoqun ryoqun May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CriesofCarrots in short, I was exhausted for bumping nightly this time, which took months. (details: #31381 (comment))

just saw #31690. sorry for trouble because of inter-branch clippy inconsistency. to be honest, i didn't anticipated
this.

after initial triage from @brooksprumo, i sensed fixing this isn't straigtforward: #31381 (comment)

so, i opted to merge this pr in as-is. (this urgency is coming from the fact my upcoming pr will be blocked on rustc 1.70+)

seems @apfitzge is starting to tackle on this: #31685.

initially, i thought this exception isn't end of world thing, but seems to cause actual annoyance. I'll work on this.

Why did you add this @ryoqun ?

so, put differently, crude way to solicit collaboration. ;)


# run nightly clippy for `sdk/` as there's a moderate amount of nightly-only code there
_ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" clippy --workspace --all-targets --features dummy-for-ci-check -- \
Expand Down
82 changes: 59 additions & 23 deletions programs/bpf_loader/src/serialization.rs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lichtso could you review changes in this file? i think it's the only review-worth diff in this pr to merge. :)

Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ mod tests {
},
std::{
cell::RefCell,
mem::transmute,
rc::Rc,
slice::{self, from_raw_parts, from_raw_parts_mut},
},
Expand Down Expand Up @@ -994,58 +995,94 @@ mod tests {
}

// the old bpf_loader in-program deserializer bpf_loader::id()
#[allow(clippy::type_complexity)]
#[deny(unsafe_op_in_unsafe_fn)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally, i want this to be enabled at tree-wide. but this is different story.

pub unsafe fn deserialize_unaligned<'a>(
input: *mut u8,
) -> (&'a Pubkey, Vec<AccountInfo<'a>>, &'a [u8]) {
// this boring boilerplate struct is needed until inline const...
struct Ptr<T>(std::marker::PhantomData<T>);
impl<T> Ptr<T> {
const COULD_BE_UNALIGNED: bool = std::mem::align_of::<T>() > 1;

#[inline(always)]
fn read_possibly_unaligned(input: *mut u8, offset: usize) -> T {
unsafe {
let src = input.add(offset) as *const T;
if Self::COULD_BE_UNALIGNED {
src.read_unaligned()
} else {
src.read()
}
}
}

// rustc inserts debug_assert! for misaligned pointer dereferences when
// deserializing, starting from [1]. so, use std::mem::transmute as the last resort
// while preventing clippy from complaining to suggest not to use it.
// [1]: https://github.com/rust-lang/rust/commit/22a7a19f9333bc1fcba97ce444a3515cb5fb33e6
// as for the ub nature of the misaligned pointer dereference, this is
// acceptable in this code, given that this is cfg(test) and it's cared only with
// x86-64 and the target only incurs some performance penalty, not like segfaults
// in other targets.
Comment on lines +1019 to +1026
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmakarov this is fyi. I think you'll need to update the original code at sdk/program/entrypoint{,_deprecated}.rs when updating platform-tool to be based on rust toolchain 1.70+.

I already chatted with @Lichtso and @alessandrod. hope this comment does make sense for the uninformed. :)

#[inline(always)]
fn ref_possibly_unaligned<'a>(input: *mut u8, offset: usize) -> &'a T {
#[allow(clippy::transmute_ptr_to_ref)]
unsafe {
transmute(input.add(offset) as *const T)
}
}

// See ref_possibly_unaligned's comment
#[inline(always)]
fn mut_possibly_unaligned<'a>(input: *mut u8, offset: usize) -> &'a mut T {
#[allow(clippy::transmute_ptr_to_ref)]
unsafe {
transmute(input.add(offset) as *mut T)
}
}
}

let mut offset: usize = 0;

// number of accounts present

#[allow(clippy::cast_ptr_alignment)]
let num_accounts = *(input.add(offset) as *const u64) as usize;
let num_accounts = Ptr::<u64>::read_possibly_unaligned(input, offset) as usize;
offset += size_of::<u64>();

// account Infos

let mut accounts = Vec::with_capacity(num_accounts);
for _ in 0..num_accounts {
let dup_info = *(input.add(offset) as *const u8);
let dup_info = Ptr::<u8>::read_possibly_unaligned(input, offset);
offset += size_of::<u8>();
if dup_info == NON_DUP_MARKER {
#[allow(clippy::cast_ptr_alignment)]
let is_signer = *(input.add(offset) as *const u8) != 0;
let is_signer = Ptr::<u8>::read_possibly_unaligned(input, offset) != 0;
offset += size_of::<u8>();

#[allow(clippy::cast_ptr_alignment)]
let is_writable = *(input.add(offset) as *const u8) != 0;
let is_writable = Ptr::<u8>::read_possibly_unaligned(input, offset) != 0;
offset += size_of::<u8>();

let key: &Pubkey = &*(input.add(offset) as *const Pubkey);
let key = Ptr::<Pubkey>::ref_possibly_unaligned(input, offset);
offset += size_of::<Pubkey>();

#[allow(clippy::cast_ptr_alignment)]
let lamports = Rc::new(RefCell::new(&mut *(input.add(offset) as *mut u64)));
let lamports = Rc::new(RefCell::new(Ptr::mut_possibly_unaligned(input, offset)));
offset += size_of::<u64>();

#[allow(clippy::cast_ptr_alignment)]
let data_len = *(input.add(offset) as *const u64) as usize;
let data_len = Ptr::<u64>::read_possibly_unaligned(input, offset) as usize;
offset += size_of::<u64>();

let data = Rc::new(RefCell::new({
let data = Rc::new(RefCell::new(unsafe {
from_raw_parts_mut(input.add(offset), data_len)
}));
offset += data_len;

let owner: &Pubkey = &*(input.add(offset) as *const Pubkey);
let owner: &Pubkey = Ptr::<Pubkey>::ref_possibly_unaligned(input, offset);
offset += size_of::<Pubkey>();

#[allow(clippy::cast_ptr_alignment)]
let executable = *(input.add(offset) as *const u8) != 0;
let executable = Ptr::<u8>::read_possibly_unaligned(input, offset) != 0;
offset += size_of::<u8>();

#[allow(clippy::cast_ptr_alignment)]
let rent_epoch = *(input.add(offset) as *const u64);
let rent_epoch = Ptr::<u64>::read_possibly_unaligned(input, offset);
offset += size_of::<u64>();

accounts.push(AccountInfo {
Expand All @@ -1066,16 +1103,15 @@ mod tests {

// instruction data

#[allow(clippy::cast_ptr_alignment)]
let instruction_data_len = *(input.add(offset) as *const u64) as usize;
let instruction_data_len = Ptr::<u64>::read_possibly_unaligned(input, offset) as usize;
offset += size_of::<u64>();

let instruction_data = { from_raw_parts(input.add(offset), instruction_data_len) };
let instruction_data = unsafe { from_raw_parts(input.add(offset), instruction_data_len) };
offset += instruction_data_len;

// program Id

let program_id: &Pubkey = &*(input.add(offset) as *const Pubkey);
let program_id = Ptr::<Pubkey>::ref_possibly_unaligned(input, offset);

(program_id, accounts, instruction_data)
}
Expand Down
2 changes: 1 addition & 1 deletion zk-token-sdk/src/zk_token_proof_instruction.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
///! Instructions provided by the ZkToken Proof program
//! Instructions provided by the ZkToken Proof program
pub use crate::instruction::*;
use {
bytemuck::bytes_of,
Expand Down