From 05c8f687d35baca3cb044def3aa3012877c29998 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 7 Oct 2023 13:15:38 +0900 Subject: [PATCH] Enable frozen_abi on banking trace file (#33501) * Enable frozen_abi on banking trace file * Fix ci with really correct bugfix... * Remove tracker_callers * Fix typo... * Fix AbiExample for Arc/Rc's Weaks * Added comment for AbiExample impl of SystemTime * Simplify and document EvenAsOpaque with new usage * Minor clean-ups * Simplify SystemTime::example() with UNIX_EPOCH... * Add comment for AbiExample subtleties (cherry picked from commit 95810d876a7cf8bdf9991ff5b887074c8d835de1) # Conflicts: # Cargo.lock --- Cargo.lock | 7 +++ core/src/banking_trace.rs | 7 ++- core/src/sigverify.rs | 2 +- frozen-abi/Cargo.toml | 1 + frozen-abi/src/abi_digester.rs | 54 +++++++++++++--- frozen-abi/src/abi_example.rs | 109 +++++++++++++++++++++++++-------- perf/Cargo.toml | 5 ++ perf/build.rs | 26 ++++++++ perf/src/cuda_runtime.rs | 2 +- perf/src/lib.rs | 4 ++ perf/src/packet.rs | 2 +- perf/src/recycler.rs | 9 +++ programs/sbf/Cargo.lock | 3 + sdk/src/packet.rs | 19 +++++- 14 files changed, 208 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f40166dbab93d..47e0f04157410b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5931,8 +5931,12 @@ dependencies = [ name = "solana-frozen-abi" version = "1.17.1" dependencies = [ +<<<<<<< HEAD "ahash 0.8.3", "blake3", +======= + "bitflags 2.3.3", +>>>>>>> 95810d876a (Enable frozen_abi on banking trace file (#33501)) "block-buffer 0.10.4", "bs58", "bv", @@ -6439,7 +6443,10 @@ dependencies = [ "rand 0.8.5", "rand_chacha 0.3.1", "rayon", + "rustc_version 0.4.0", "serde", + "solana-frozen-abi", + "solana-frozen-abi-macro", "solana-logger", "solana-metrics", "solana-rayon-threadlimit", diff --git a/core/src/banking_trace.rs b/core/src/banking_trace.rs index 760121dc7c557d..ba76b794ba2919 100644 --- a/core/src/banking_trace.rs +++ b/core/src/banking_trace.rs @@ -62,16 +62,17 @@ pub struct BankingTracer { active_tracer: Option, } -#[derive(Serialize, Deserialize, Debug)] +#[frozen_abi(digest = "Eq6YrAFtTbtPrCEvh6Et1mZZDCARUg1gcK2qiZdqyjUz")] +#[derive(Serialize, Deserialize, Debug, AbiExample)] pub struct TimedTracedEvent(pub std::time::SystemTime, pub TracedEvent); -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, AbiExample, AbiEnumVisitor)] pub enum TracedEvent { PacketBatch(ChannelLabel, BankingPacketBatch), BlockAndBankHash(Slot, Hash, Hash), } -#[derive(Serialize, Deserialize, Debug, Clone, Copy)] +#[derive(Serialize, Deserialize, Debug, Clone, Copy, AbiExample, AbiEnumVisitor)] pub enum ChannelLabel { NonVote, TpuVote, diff --git a/core/src/sigverify.rs b/core/src/sigverify.rs index 8140efac7ec2aa..b496452078d883 100644 --- a/core/src/sigverify.rs +++ b/core/src/sigverify.rs @@ -16,7 +16,7 @@ use { solana_sdk::{packet::Packet, saturating_add_assign}, }; -#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, AbiExample)] pub struct SigverifyTracerPacketStats { pub total_removed_before_sigverify_stage: usize, pub total_tracer_packets_received_in_sigverify_stage: usize, diff --git a/frozen-abi/Cargo.toml b/frozen-abi/Cargo.toml index 3121b6968ebdf5..898b6d9b205a10 100644 --- a/frozen-abi/Cargo.toml +++ b/frozen-abi/Cargo.toml @@ -38,6 +38,7 @@ cc = { workspace = true, features = ["jobserver", "parallel"] } [target.'cfg(not(target_os = "solana"))'.dev-dependencies] solana-logger = { workspace = true } +bitflags = { workspace = true } [build-dependencies] rustc_version = { workspace = true } diff --git a/frozen-abi/src/abi_digester.rs b/frozen-abi/src/abi_digester.rs index 0d0886daae7438..b014efd2ba1570 100644 --- a/frozen-abi/src/abi_digester.rs +++ b/frozen-abi/src/abi_digester.rs @@ -17,7 +17,7 @@ pub struct AbiDigester { data_types: std::rc::Rc>>, depth: usize, for_enum: bool, - opaque_scope: Option, + opaque_type_matcher: Option, } pub type DigestResult = Result; @@ -70,7 +70,7 @@ impl AbiDigester { data_types: std::rc::Rc::new(std::cell::RefCell::new(vec![])), for_enum: false, depth: 0, - opaque_scope: None, + opaque_type_matcher: None, } } @@ -81,16 +81,16 @@ impl AbiDigester { data_types: self.data_types.clone(), depth: self.depth, for_enum: false, - opaque_scope: self.opaque_scope.clone(), + opaque_type_matcher: self.opaque_type_matcher.clone(), } } - pub fn create_new_opaque(&self, top_scope: &str) -> Self { + pub fn create_new_opaque(&self, type_matcher: &str) -> Self { Self { data_types: self.data_types.clone(), depth: self.depth, for_enum: false, - opaque_scope: Some(top_scope.to_owned()), + opaque_type_matcher: Some(type_matcher.to_owned()), } } @@ -103,7 +103,7 @@ impl AbiDigester { data_types: self.data_types.clone(), depth, for_enum: false, - opaque_scope: self.opaque_scope.clone(), + opaque_type_matcher: self.opaque_type_matcher.clone(), }) } @@ -116,15 +116,15 @@ impl AbiDigester { data_types: self.data_types.clone(), depth, for_enum: true, - opaque_scope: self.opaque_scope.clone(), + opaque_type_matcher: self.opaque_type_matcher.clone(), }) } pub fn digest_data(&mut self, value: &T) -> DigestResult { let type_name = normalize_type_name(type_name::()); if type_name.ends_with("__SerializeWith") - || (self.opaque_scope.is_some() - && type_name.starts_with(self.opaque_scope.as_ref().unwrap())) + || (self.opaque_type_matcher.is_some() + && type_name.contains(self.opaque_type_matcher.as_ref().unwrap())) { // we can't use the AbiEnumVisitor trait for these cases. value.serialize(self.create_new()) @@ -661,6 +661,34 @@ mod tests { #[frozen_abi(digest = "9PMdHRb49BpkywrmPoJyZWMsEmf5E1xgmsFGkGmea5RW")] type TestBitVec = bv::BitVec; + mod bitflags_abi { + use crate::abi_example::{AbiExample, EvenAsOpaque, IgnoreAsHelper}; + + bitflags::bitflags! { + #[frozen_abi(digest = "HhKNkaeAd7AohTb8S8sPKjAWwzxWY2DPz5FvkWmx5bSH")] + #[derive(Serialize, Deserialize)] + struct TestFlags: u8 { + const TestBit = 0b0000_0001; + } + } + + impl AbiExample for TestFlags { + fn example() -> Self { + Self::empty() + } + } + + impl IgnoreAsHelper for TestFlags {} + // This (EvenAsOpaque) marker trait is needed for bitflags-generated types because we can't + // impl AbiExample for its private type: + // thread '...TestFlags_frozen_abi...' panicked at ...: + // derive or implement AbiExample/AbiEnumVisitor for + // solana_frozen_abi::abi_digester::tests::_::InternalBitFlags + impl EvenAsOpaque for TestFlags { + const TYPE_NAME_MATCHER: &'static str = "::_::InternalBitFlags"; + } + } + mod skip_should_be_same { #[frozen_abi(digest = "4LbuvQLX78XPbm4hqqZcHFHpseDJcw4qZL9EUZXSi2Ss")] #[derive(Serialize, AbiExample)] @@ -691,4 +719,12 @@ mod tests { Variant2(u8, u16, #[serde(skip)] u32), } } + + #[frozen_abi(digest = "B1PcwZdUfGnxaRid9e6ZwkST3NZ2KUEYobA1DkxWrYLP")] + #[derive(Serialize, AbiExample)] + struct TestArcWeak(std::sync::Weak); + + #[frozen_abi(digest = "4R8uCLR1BVU1aFgkSaNyKcFD1FeM6rGdsjbJBFpnqx4v")] + #[derive(Serialize, AbiExample)] + struct TestRcWeak(std::rc::Weak); } diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index c7765c4a573544..ab68c6ff25e32a 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -6,6 +6,24 @@ use { std::any::type_name, }; +// The most important trait for the abi digesting. This trait is used to create any complexities of +// object graph to generate the abi digest. The frozen abi test harness calls T::example() to +// instantiate the tested root type and traverses its fields recursively, abusing the +// serde::serialize(). +// +// This trait applicability is similar to the Default trait. That means all referenced types must +// implement this trait. AbiExample is implemented for almost all common types in this file. +// +// When implementing AbiExample manually, you need to return a _minimally-populated_ value +// from it to actually generate a meaningful digest. This impl semantics is unlike Default, which +// usually returns something empty. See actual impls for inspiration. +// +// The requirement of AbiExample impls even applies to those types of `#[serde(skip)]`-ed fields. +// That's because the abi digesting needs a properly initialized object to enter into the +// serde::serialize() to begin with, even knowning they aren't used for serialization and thus abi +// digest. Luckily, `#[serde(skip)]`-ed fields' AbiExample impls can just delegate to T::default(), +// exploiting the nature of this artificial impl requirement as an exception from the usual +// AbiExample semantics. pub trait AbiExample: Sized { fn example() -> Self; } @@ -137,25 +155,12 @@ tuple_example_impls! { } } -// Source: https://github.com/rust-lang/rust/blob/ba18875557aabffe386a2534a1aa6118efb6ab88/src/libcore/array/mod.rs#L417 -macro_rules! array_example_impls { - {$n:expr, $t:ident $($ts:ident)*} => { - impl AbiExample for [T; $n] where T: AbiExample { - fn example() -> Self { - [$t::example(), $($ts::example()),*] - } - } - array_example_impls!{($n - 1), $($ts)*} - }; - {$n:expr,} => { - impl AbiExample for [T; $n] { - fn example() -> Self { [] } - } - }; +impl AbiExample for [T; N] { + fn example() -> Self { + std::array::from_fn(|_| T::example()) + } } -array_example_impls! {32, T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T} - // Source: https://github.com/rust-lang/rust/blob/ba18875557aabffe386a2534a1aa6118efb6ab88/src/libcore/default.rs#L137 macro_rules! example_impls { ($t:ty, $v:expr) => { @@ -232,7 +237,14 @@ impl AbiExample for BitVec { } impl IgnoreAsHelper for BitVec {} -impl EvenAsOpaque for BitVec {} +// This (EvenAsOpaque) marker trait is needed for BitVec because we can't impl AbiExample for its +// private type: +// thread '...TestBitVec_frozen_abi...' panicked at ...: +// derive or implement AbiExample/AbiEnumVisitor for +// bv::bit_vec::inner::Inner +impl EvenAsOpaque for BitVec { + const TYPE_NAME_MATCHER: &'static str = "bv::bit_vec::inner::"; +} pub(crate) fn normalize_type_name(type_name: &str) -> String { type_name.chars().filter(|c| *c != '&').collect() @@ -329,6 +341,23 @@ impl AbiExample for std::sync::Arc { } } +// When T is weakly owned by the likes of `std::{sync, rc}::Weak`s, we need to uphold the ownership +// of T in some way at least during abi digesting... However, there's no easy way. Stashing them +// into static is confronted with Send/Sync issue. Stashing them into thread_local is confronted +// with not enough (T + 'static) lifetime bound.. So, just leak the examples. This should be +// tolerated, considering ::example() should ever be called inside tests, not in production code... +fn leak_and_inhibit_drop<'a, T>(t: T) -> &'a mut T { + Box::leak(Box::new(t)) +} + +impl AbiExample for std::sync::Weak { + fn example() -> Self { + info!("AbiExample for (Arc's Weak): {}", type_name::()); + // leaking is needed otherwise Arc::upgrade() will always return None... + std::sync::Arc::downgrade(leak_and_inhibit_drop(std::sync::Arc::new(T::example()))) + } +} + impl AbiExample for std::rc::Rc { fn example() -> Self { info!("AbiExample for (Rc): {}", type_name::()); @@ -336,6 +365,14 @@ impl AbiExample for std::rc::Rc { } } +impl AbiExample for std::rc::Weak { + fn example() -> Self { + info!("AbiExample for (Rc's Weak): {}", type_name::()); + // leaking is needed otherwise Rc::upgrade() will always return None... + std::rc::Rc::downgrade(leak_and_inhibit_drop(std::rc::Rc::new(T::example()))) + } +} + impl AbiExample for std::sync::Mutex { fn example() -> Self { info!("AbiExample for (Mutex): {}", type_name::()); @@ -457,6 +494,13 @@ impl AbiExample for std::path::PathBuf { } } +#[cfg(not(target_os = "solana"))] +impl AbiExample for std::time::SystemTime { + fn example() -> Self { + std::time::SystemTime::UNIX_EPOCH + } +} + use std::net::{IpAddr, Ipv4Addr, SocketAddr}; impl AbiExample for SocketAddr { fn example() -> Self { @@ -470,13 +514,22 @@ impl AbiExample for IpAddr { } } -// This is a control flow indirection needed for digesting all variants of an enum +// This is a control flow indirection needed for digesting all variants of an enum. +// +// All of types (including non-enums) will be processed by this trait, albeit the +// name of this trait. +// User-defined enums usually just need to impl this with namesake derive macro (AbiEnumVisitor). +// +// Note that sometimes this indirection doesn't work for various reasons. For that end, there are +// hacks with marker traits (IgnoreAsHelper/EvenAsOpaque). pub trait AbiEnumVisitor: Serialize { fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult; } pub trait IgnoreAsHelper {} -pub trait EvenAsOpaque {} +pub trait EvenAsOpaque { + const TYPE_NAME_MATCHER: &'static str; +} impl AbiEnumVisitor for T { default fn visit_for_abi(&self, _digester: &mut AbiDigester) -> DigestResult { @@ -489,7 +542,9 @@ impl AbiEnumVisitor for T { impl AbiEnumVisitor for T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { - info!("AbiEnumVisitor for (default): {}", type_name::()); + info!("AbiEnumVisitor for T: {}", type_name::()); + // not calling self.serialize(...) is intentional here as the most generic impl + // consider IgnoreAsHelper and EvenAsOpaque if you're stuck on this.... T::example() .serialize(digester.create_new()) .map_err(DigestError::wrap_by_type::) @@ -501,7 +556,7 @@ impl AbiEnumVisitor for T { // relevant test: TestVecEnum impl AbiEnumVisitor for &T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { - info!("AbiEnumVisitor for (&default): {}", type_name::()); + info!("AbiEnumVisitor for &T: {}", type_name::()); // Don't call self.visit_for_abi(...) to avoid the infinite recursion! T::visit_for_abi(self, digester) } @@ -521,9 +576,13 @@ impl AbiEnumVisitor for &T { // inability of implementing AbiExample for private structs from other crates impl AbiEnumVisitor for &T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { - info!("AbiEnumVisitor for (IgnoreAsOpaque): {}", type_name::()); - let top_scope = type_name::().split("::").next().unwrap(); - self.serialize(digester.create_new_opaque(top_scope)) + let type_name = type_name::(); + let matcher = T::TYPE_NAME_MATCHER; + info!( + "AbiEnumVisitor for (EvenAsOpaque): {}: matcher: {}", + type_name, matcher + ); + self.serialize(digester.create_new_opaque(matcher)) .map_err(DigestError::wrap_by_type::) } } diff --git a/perf/Cargo.toml b/perf/Cargo.toml index aea478da078c35..b62484f4249abd 100644 --- a/perf/Cargo.toml +++ b/perf/Cargo.toml @@ -21,6 +21,8 @@ log = { workspace = true } rand = { workspace = true } rayon = { workspace = true } serde = { workspace = true } +solana-frozen-abi = { workspace = true } +solana-frozen-abi-macro = { workspace = true } solana-metrics = { workspace = true } solana-rayon-threadlimit = { workspace = true } solana-sdk = { workspace = true } @@ -40,6 +42,9 @@ rand_chacha = { workspace = true } solana-logger = { workspace = true } test-case = { workspace = true } +[build-dependencies] +rustc_version = { workspace = true } + [[bench]] name = "sigverify" diff --git a/perf/build.rs b/perf/build.rs index 025c71008f092b..4925ee898eb612 100644 --- a/perf/build.rs +++ b/perf/build.rs @@ -1,3 +1,6 @@ +extern crate rustc_version; +use rustc_version::{version_meta, Channel}; + fn main() { #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] { @@ -8,4 +11,27 @@ fn main() { println!("cargo:rustc-cfg=build_target_feature_avx2"); } } + + // Copied and adapted from + // https://github.com/Kimundi/rustc-version-rs/blob/1d692a965f4e48a8cb72e82cda953107c0d22f47/README.md#example + // Licensed under Apache-2.0 + MIT + match version_meta().unwrap().channel { + Channel::Stable => { + println!("cargo:rustc-cfg=RUSTC_WITHOUT_SPECIALIZATION"); + } + Channel::Beta => { + println!("cargo:rustc-cfg=RUSTC_WITHOUT_SPECIALIZATION"); + } + Channel::Nightly => { + println!("cargo:rustc-cfg=RUSTC_WITH_SPECIALIZATION"); + } + Channel::Dev => { + println!("cargo:rustc-cfg=RUSTC_WITH_SPECIALIZATION"); + // See https://github.com/solana-labs/solana/issues/11055 + // We may be running the custom `rust-bpf-builder` toolchain, + // which currently needs `#![feature(proc_macro_hygiene)]` to + // be applied. + println!("cargo:rustc-cfg=RUSTC_NEEDS_PROC_MACRO_HYGIENE"); + } + } } diff --git a/perf/src/cuda_runtime.rs b/perf/src/cuda_runtime.rs index a2986af1813680..5b44099aecb36c 100644 --- a/perf/src/cuda_runtime.rs +++ b/perf/src/cuda_runtime.rs @@ -54,7 +54,7 @@ fn unpin(mem: *mut T) { // A vector wrapper where the underlying memory can be // page-pinned. Controlled by flags in case user only wants // to pin in certain circumstances. -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Serialize, Deserialize, AbiExample)] pub struct PinnedVec { x: Vec, pinned: bool, diff --git a/perf/src/lib.rs b/perf/src/lib.rs index 8d277d7ad69778..83cefe1f319145 100644 --- a/perf/src/lib.rs +++ b/perf/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] pub mod cuda_runtime; pub mod data_budget; pub mod deduper; @@ -23,6 +24,9 @@ extern crate assert_matches; #[macro_use] extern crate solana_metrics; +#[macro_use] +extern crate solana_frozen_abi_macro; + fn is_rosetta_emulated() -> bool { #[cfg(target_os = "macos")] { diff --git a/perf/src/packet.rs b/perf/src/packet.rs index b030f04dae8ce6..fbb8a437d6bd32 100644 --- a/perf/src/packet.rs +++ b/perf/src/packet.rs @@ -18,7 +18,7 @@ pub const NUM_PACKETS: usize = 1024 * 8; pub const PACKETS_PER_BATCH: usize = 64; pub const NUM_RCVMMSGS: usize = 64; -#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, Serialize, Deserialize, AbiExample)] pub struct PacketBatch { packets: PinnedVec, } diff --git a/perf/src/recycler.rs b/perf/src/recycler.rs index 27c47d0df45103..87c44399e7fbc8 100644 --- a/perf/src/recycler.rs +++ b/perf/src/recycler.rs @@ -57,6 +57,15 @@ impl Default for RecyclerX { } } +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl solana_frozen_abi::abi_example::AbiExample + for RecyclerX> +{ + fn example() -> Self { + Self::default() + } +} + pub trait Reset { fn reset(&mut self); fn warm(&mut self, size_hint: usize); diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 4eccde743b06ea..af0efd7802ef0d 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5199,7 +5199,10 @@ dependencies = [ "nix", "rand 0.8.5", "rayon", + "rustc_version", "serde", + "solana-frozen-abi", + "solana-frozen-abi-macro", "solana-metrics", "solana-rayon-threadlimit", "solana-sdk", diff --git a/sdk/src/packet.rs b/sdk/src/packet.rs index b70d8adae8a4bb..faea9ab4753c67 100644 --- a/sdk/src/packet.rs +++ b/sdk/src/packet.rs @@ -36,7 +36,7 @@ bitflags! { } } -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, AbiExample)] #[repr(C)] pub struct Meta { pub size: usize, @@ -45,6 +45,21 @@ pub struct Meta { pub flags: PacketFlags, } +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::AbiExample for PacketFlags { + fn example() -> Self { + Self::empty() + } +} + +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::IgnoreAsHelper for PacketFlags {} + +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::EvenAsOpaque for PacketFlags { + const TYPE_NAME_MATCHER: &'static str = "::_::InternalBitFlags"; +} + // serde_as is used as a work around because array isn't supported by serde // (and serde_bytes). // @@ -71,7 +86,7 @@ pub struct Meta { // ryoqun's dirty experiments: // https://github.com/ryoqun/serde-array-comparisons #[serde_as] -#[derive(Clone, Eq, Serialize, Deserialize)] +#[derive(Clone, Eq, Serialize, Deserialize, AbiExample)] #[repr(C)] pub struct Packet { // Bytes past Packet.meta.size are not valid to read from.