From 789d5a6944be286dfae741f77c63105d17045c01 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Sat, 27 Jul 2024 09:07:09 +0800 Subject: [PATCH] Eliminate clippy warnings; Add more comprehensive CIs --- .github/workflows/ci.yml | 53 +++++++++++++++++++-------- src/code_manipulate.rs | 8 +++++ src/lib.rs | 78 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 119 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 207123e..d33e674 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,22 +15,24 @@ jobs: strategy: fail-fast: false matrix: - os: [ - ubuntu-latest, - macos-latest, - windows-latest + target: [ + x86_64-unknown-linux-gnu, + x86_64-unknown-linux-musl, + aarch64-apple-darwin, + x86_64-pc-windows-msvc, + i686-pc-windows-msvc, ] include: - - os: ubuntu-latest - target: x86_64-unknown-linux-gnu - - os: ubuntu-latest - target: x86_64-unknown-linux-musl - - os: macos-latest - target: aarch64-apple-darwin - - os: windows-latest - target: x86_64-pc-windows-msvc - - os: windows-latest - target: i686-pc-windows-msvc + - target: x86_64-unknown-linux-gnu + os: ubuntu-latest + - target: x86_64-unknown-linux-musl + os: ubuntu-latest + - target: aarch64-apple-darwin + os: macos-latest + - target: x86_64-pc-windows-msvc + os: windows-latest + - target: i686-pc-windows-msvc + os: windows-latest steps: - uses: actions/checkout@v4 - name: Install corresponding target @@ -69,3 +71,26 @@ jobs: - uses: actions/checkout@v4 - name: Check code format run: cargo fmt --check + + clippy: + name: Rust Clippy + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest] + steps: + - uses: actions/checkout@v4 + - name: Run clippy + run: cargo clippy -- -Dwarnings + + # Use individual doctest due to https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#doctest-xcompile + doctest: + name: Rustdoc Test + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest] + steps: + - uses: actions/checkout@v4 + - name: Test documentation + run: cargo test --verbose --doc -- --test-threads=1 diff --git a/src/code_manipulate.rs b/src/code_manipulate.rs index c9a0575..a985417 100644 --- a/src/code_manipulate.rs +++ b/src/code_manipulate.rs @@ -9,6 +9,14 @@ pub trait CodeManipulator { /// /// The `addr` is not aligned, you need to align it you self. The length is not too long, usually /// 5 bytes. + /// + /// # Safety + /// + /// This method will do best effort to make the code region writable, and then write the data into + /// the code region. If the code region is still not writable, the data writing will become a UB. + /// Never call this method when there are multi-threads running. Spawn threads after this method + /// is called. This method may manipulate code region memory protection, and if other threads are + /// executing codes in the same code page, it may lead to unexpected behaviors. unsafe fn write_code(addr: *mut core::ffi::c_void, data: &[u8; L]); } diff --git a/src/lib.rs b/src/lib.rs index b19ea65..d21eca8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ #![no_std] #![feature(asm_goto)] #![feature(asm_const)] +#![allow(clippy::needless_doctest_main)] mod arch; pub mod code_manipulate; @@ -72,8 +73,8 @@ impl JumpEntry { } /// Unique reference to associated key - fn key_mut(&self) -> &mut GenericStaticKey { - unsafe { &mut *(self.key_addr() as usize as *mut GenericStaticKey) } + fn key_mut(&self) -> &'static mut GenericStaticKey { + unsafe { &mut *(self.key_addr() as *mut GenericStaticKey) } } /// Whether this jump entry is dummy @@ -161,13 +162,27 @@ impl GenericStaticKey { } /// Enable this static key (make the value to be `true`). Do nothing if current static key is already enabled. - pub fn enable(&mut self) { - unsafe { static_key_update(self, true) } + /// + /// # Safety + /// + /// This method may be UB if called before [`global_init`] or called in parallel. Never call this method when + /// there are multi-threads running. Spawn threads after this method is called. This method may manipulate + /// code region memory protection, and if other threads are executing codes in the same code page, it may + /// lead to unexpected behaviors. + pub unsafe fn enable(&mut self) { + static_key_update(self, true) } /// Disable this static key (make the value to be `false`). Do nothing if current static key is already disabled. - pub fn disable(&mut self) { - unsafe { static_key_update(self, false) } + /// + /// # Safety + /// + /// This method may be UB if called before [`global_init`] or called in parallel. Never call this method when + /// there are multi-threads running. Spawn threads after this method is called. This method may manipulate + /// code region memory protection, and if other threads are executing codes in the same code page, it may + /// lead to unexpected behaviors. + pub unsafe fn disable(&mut self) { + static_key_update(self, false) } } @@ -180,13 +195,50 @@ pub fn jump_entries_count() { } // ---------------------------- Create ---------------------------- +/// Global state to make sure [`global_init`] is called only once +static GLOBAL_INIT_STATE: core::sync::atomic::AtomicUsize = core::sync::atomic::AtomicUsize::new(0); +const UNINITIALIZED: usize = 0; +const INITIALIZING: usize = 1; +const INITIALIZED: usize = 2; + /// Initialize the static keys data. Always call this method at beginning of application, before using any static key related /// functionalities. +/// +/// This function should be called only once. If calling this method multiple times in multi-threads, only the first invocation +/// will take effect. pub fn global_init() { // DUMMY_STATIC_KEY will never changed, and this will always be a NOP. + // Doing this to make sure there are at least one jump entry. if static_branch_unlikely!(DUMMY_STATIC_KEY) { return; } + + // This logic is taken from log::set_logger_inner + match GLOBAL_INIT_STATE.compare_exchange( + UNINITIALIZED, + INITIALIZING, + core::sync::atomic::Ordering::Acquire, + core::sync::atomic::Ordering::Relaxed, + ) { + Ok(UNINITIALIZED) => { + global_init_inner(); + GLOBAL_INIT_STATE.store(INITIALIZED, core::sync::atomic::Ordering::Release); + // Successful init + } + Err(INITIALIZING) => { + while GLOBAL_INIT_STATE.load(core::sync::atomic::Ordering::Relaxed) == INITIALIZING { + core::hint::spin_loop(); + } + // Other has inited + } + _ => { + // Other has inited + } + } +} + +/// Inner function to [`global_init`] +fn global_init_inner() { let jump_entry_start_addr = core::ptr::addr_of_mut!(os::JUMP_ENTRY_START); let jump_entry_stop_addr = core::ptr::addr_of_mut!(os::JUMP_ENTRY_STOP); let jump_entry_len = @@ -285,6 +337,13 @@ macro_rules! define_static_key_true { /// The internal method used for [`GenericStaticKey::enable`] and [`GenericStaticKey::disable`]. /// /// This method will update instructions recorded in each jump entries that associated with thie static key +/// +/// # Safety +/// +/// This method may be UB if called before [`global_init`] or called in parallel. Never call this method when +/// there are multi-threads running. Spawn threads after this method is called. This method may manipulate +/// code region memory protection, and if other threads are executing codes in the same code page, it may +/// lead to unexpected behaviors. unsafe fn static_key_update( key: &mut GenericStaticKey, enabled: bool, @@ -324,6 +383,13 @@ enum JumpLabelType { } /// Update instruction recorded in a single jump entry. This is where magic happens +/// +/// # Safety +/// +/// This method may be UB if called in parallel. Never call this method when +/// there are multi-threads running. Spawn threads after this method is called. This method may manipulate +/// code region memory protection, and if other threads are executing codes in the same code page, it may +/// lead to unexpected behaviors. unsafe fn jump_entry_update(jump_entry: &JumpEntry, enabled: bool) { let jump_label_type = if enabled ^ jump_entry.likely_branch_is_true() { JumpLabelType::Jmp