From 3dd4eab241eea5784e71903b59e78febd5c88a9b Mon Sep 17 00:00:00 2001 From: ROMemories <152802150+ROMemories@users.noreply.github.com> Date: Fri, 29 Nov 2024 09:35:51 +0100 Subject: [PATCH 1/5] fix(cortex-m): stop using `no_mangle` for the scheduler fn This avoids potential collisions of the short symbol `sched`, which would be UB, as denoted by the `no_mangle` attribute being marked `unsafe` in Rust edition 2024. --- src/ariel-os-threads/src/arch/cortex_m.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ariel-os-threads/src/arch/cortex_m.rs b/src/ariel-os-threads/src/arch/cortex_m.rs index 830a49ffd..0a37f632c 100644 --- a/src/ariel-os-threads/src/arch/cortex_m.rs +++ b/src/ariel-os-threads/src/arch/cortex_m.rs @@ -125,7 +125,7 @@ unsafe extern "C" fn PendSV() { unsafe { asm!( " - bl sched + bl {sched} cmp r0, #0 beq 99f @@ -171,6 +171,7 @@ unsafe extern "C" fn PendSV() { 999: .word 0xFFFFFFFD ", + sched = sym sched, options(noreturn) ) }; @@ -190,8 +191,6 @@ unsafe extern "C" fn PendSV() { /// - `r0`: stack-pointer for new thread /// /// This function is called in PendSV. -// TODO: make arch independent, or move to arch -#[no_mangle] unsafe fn sched() -> u128 { loop { if let Some(res) = critical_section::with(|cs| { From 24cc9ad4994b38f744ca950f25a7ea897600630e Mon Sep 17 00:00:00 2001 From: ROMemories <152802150+ROMemories@users.noreply.github.com> Date: Fri, 29 Nov 2024 09:48:15 +0100 Subject: [PATCH 2/5] refactor(thread-macro): remove the unused `no_mangle` parameter If we were to keep it, we would need to mark it unsafe in some way, to reflect the `#[no_mangle]` attribute being marked unsafe in Rust edition 2024. --- src/ariel-os-macros/src/thread.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/ariel-os-macros/src/thread.rs b/src/ariel-os-macros/src/thread.rs index 8bfaf67b1..001c2ae14 100644 --- a/src/ariel-os-macros/src/thread.rs +++ b/src/ariel-os-macros/src/thread.rs @@ -63,12 +63,6 @@ pub fn thread(args: TokenStream, item: TokenStream) -> TokenStream { } }; - let no_mangle_attr = if attrs.no_mangle { - quote! {#[no_mangle]} - } else { - quote! {} - }; - let maybe_wait_for_start_event = if attrs.no_wait { quote! {} } else { @@ -85,7 +79,6 @@ pub fn thread(args: TokenStream, item: TokenStream) -> TokenStream { } = Parameters::from(attrs); let expanded = quote! { - #no_mangle_attr #[inline(always)] #thread_function @@ -143,7 +136,6 @@ mod thread { pub stack_size: Option, pub priority: Option, pub affinity: Option, - pub no_mangle: bool, pub no_wait: bool, } @@ -174,11 +166,6 @@ mod thread { return Ok(()); } - if meta.path.is_ident("no_mangle") { - self.no_mangle = true; - return Ok(()); - } - if meta.path.is_ident("no_wait") { self.no_wait = true; return Ok(()); From 8c69b303c59f88188c604b82d8d39881eb64f4d2 Mon Sep 17 00:00:00 2001 From: ROMemories <152802150+ROMemories@users.noreply.github.com> Date: Fri, 29 Nov 2024 09:58:26 +0100 Subject: [PATCH 3/5] fix(config-macro): add a prefix to FFI fns to avoid symbol collisions --- src/ariel-os-embassy/src/network.rs | 4 ++-- src/ariel-os-embassy/src/usb.rs | 4 ++-- src/ariel-os-macros/src/config.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ariel-os-embassy/src/network.rs b/src/ariel-os-embassy/src/network.rs index 810937ab1..8c9261667 100644 --- a/src/ariel-os-embassy/src/network.rs +++ b/src/ariel-os-embassy/src/network.rs @@ -42,9 +42,9 @@ pub(crate) fn config() -> embassy_net::Config { #[cfg(feature = "override-network-config")] { extern "Rust" { - fn ariel_os_network_config() -> embassy_net::Config; + fn __ariel_os_network_config() -> embassy_net::Config; } - unsafe { ariel_os_network_config() } + unsafe { __ariel_os_network_config() } } } diff --git a/src/ariel-os-embassy/src/usb.rs b/src/ariel-os-embassy/src/usb.rs index 9b8422113..e22fcf893 100644 --- a/src/ariel-os-embassy/src/usb.rs +++ b/src/ariel-os-embassy/src/usb.rs @@ -61,8 +61,8 @@ pub(crate) fn config() -> embassy_usb::Config<'static> { #[cfg(feature = "override-usb-config")] { extern "Rust" { - fn ariel_os_usb_config() -> embassy_usb::Config<'static>; + fn __ariel_os_usb_config() -> embassy_usb::Config<'static>; } - unsafe { ariel_os_usb_config() } + unsafe { __ariel_os_usb_config() } } } diff --git a/src/ariel-os-macros/src/config.rs b/src/ariel-os-macros/src/config.rs index 2c6e2e366..7bc74802e 100644 --- a/src/ariel-os-macros/src/config.rs +++ b/src/ariel-os-macros/src/config.rs @@ -60,11 +60,11 @@ pub fn config(args: TokenStream, item: TokenStream) -> TokenStream { let (config_fn_name, return_type) = match attrs.kind { Some(ConfigKind::Network) => ( - format_ident!("ariel_os_network_config"), + format_ident!("__ariel_os_network_config"), quote! {#ariel_os_crate::reexports::embassy_net::Config}, ), Some(ConfigKind::Usb) => ( - format_ident!("ariel_os_usb_config"), + format_ident!("__ariel_os_usb_config"), quote! {#ariel_os_crate::reexports::embassy_usb::Config<'static>}, ), None => { From 56cac7ec30fddcd75c4cf6f8d4621674225a1edd Mon Sep 17 00:00:00 2001 From: ROMemories <152802150+ROMemories@users.noreply.github.com> Date: Fri, 29 Nov 2024 10:08:28 +0100 Subject: [PATCH 4/5] refactor(thread-executor): use `no_mangle` instead of `export_name` The export name was the same as the local function name and therefore redundant. --- src/ariel-os-embassy/src/thread_executor.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ariel-os-embassy/src/thread_executor.rs b/src/ariel-os-embassy/src/thread_executor.rs index 342a3363a..793430ed3 100644 --- a/src/ariel-os-embassy/src/thread_executor.rs +++ b/src/ariel-os-embassy/src/thread_executor.rs @@ -12,7 +12,8 @@ use embassy_executor::{raw, Spawner}; // doesn't matter. const THREAD_FLAG_WAKEUP: ThreadFlags = 0x01; -#[export_name = "__pender"] +// This name is required by embassy-executor. +#[no_mangle] fn __pender(context: *mut ()) { // SAFETY: `context` is a `ThreadId` passed by `ThreadExecutor::new`. let thread_id = ThreadId::new(context as usize as u8); From 57281297073881a44aa70daab07d92ab5e7be218 Mon Sep 17 00:00:00 2001 From: ROMemories <152802150+ROMemories@users.noreply.github.com> Date: Fri, 29 Nov 2024 10:18:08 +0100 Subject: [PATCH 5/5] fix(executor): add a prefix to the FFI fn to avoid symbol collisions --- src/ariel-os-embassy/src/lib.rs | 2 +- src/ariel-os-rt/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ariel-os-embassy/src/lib.rs b/src/ariel-os-embassy/src/lib.rs index b5ec6c2e3..3eea13bb7 100644 --- a/src/ariel-os-embassy/src/lib.rs +++ b/src/ariel-os-embassy/src/lib.rs @@ -127,7 +127,7 @@ pub(crate) fn init() { } #[cfg(feature = "executor-single-thread")] -#[export_name = "ariel_os_embassy_init"] +#[export_name = "__ariel_os_embassy_init"] fn init() -> ! { debug!("ariel-os-embassy::init(): using single thread executor"); let p = hal::init(); diff --git a/src/ariel-os-rt/src/lib.rs b/src/ariel-os-rt/src/lib.rs index 7e84a9cb3..28928be1d 100644 --- a/src/ariel-os-rt/src/lib.rs +++ b/src/ariel-os-rt/src/lib.rs @@ -88,10 +88,10 @@ fn startup() -> ! { #[cfg(feature = "executor-single-thread")] { extern "Rust" { - fn ariel_os_embassy_init() -> !; + fn __ariel_os_embassy_init() -> !; } debug!("ariel_os_rt::startup() launching single thread executor"); - unsafe { ariel_os_embassy_init() }; + unsafe { __ariel_os_embassy_init() }; } #[cfg(not(any(feature = "threading", feature = "executor-single-thread")))]