From 960dd4c4f5f2962458a4488c056efccd84c4192e Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Mon, 25 Jan 2021 19:08:39 +0100 Subject: [PATCH 01/12] fix issue #336 --- macros/src/lib.rs | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index d08c2a3b..129a7bb5 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -18,6 +18,9 @@ use syn::{ WhereClause, WherePredicate, }; +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; + /// Checks if any attribute in `attrs_to_check` is in `reject_list` and returns a compiler error if there's a match /// /// The compiler error will indicate that the attribute conflicts with `attr_name` @@ -177,6 +180,13 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { ) .into() } else { + let section = { + if cfg!(target_os = "macos") { + ".defmt,end_timestamp" + } else { + ".defmt.end.timestamp" + } + }; quote!( const _: () = { #[export_name = "_defmt_timestamp"] @@ -194,7 +204,7 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { // Unique symbol name to prevent multiple `timestamp!` invocations in the crate graph. // Uses `#symname` to ensure it is not discarded by the linker. #[no_mangle] - #[link_section = ".defmt.end.timestamp"] + #[link_section = #section] static __DEFMT_MARKER_TIMESTAMP_WAS_DEFINED: &u8 = &#symname; }; ) @@ -984,7 +994,8 @@ pub fn internp(ts: TokenStream) -> TokenStream { let ls = lit.value(); let sym = symbol::Symbol::new("prim", &ls).mangle(); - let section = format!(".defmt.prim.{}", sym); + + let section = mksection("prim.", &sym); if cfg!(feature = "unstable-test") { quote!({ defmt::export::fetch_add_string_index() as u8 }) @@ -1039,9 +1050,25 @@ pub fn write(ts: TokenStream) -> TokenStream { .into() } +/// work around restrictions on length and allowed characters imposed by macos linker +/// returns (note the comma character for macos): +/// under macos: ".defmt," + 16 character hex digest of symbol's hash +/// otherwise: ".defmt." + prefix + symbol +fn mksection(prefix: &str, symbol: &str) -> String { + let mut sub_section = format!(".{}{}", prefix, symbol); + + if cfg!(target_os = "macos") { + let mut hasher = DefaultHasher::new(); + sub_section.hash(&mut hasher); + sub_section = format!(",{:x}", hasher.finish()); + } + + format!(".defmt{}", sub_section) +} + fn mkstatic(varname: Ident2, string: &str, tag: &str) -> TokenStream2 { let sym = symbol::Symbol::new(tag, string).mangle(); - let section = format!(".defmt.{}", sym); + let section = mksection("", &sym); quote!( #[link_section = #section] From 64648382b28342ada3dfd4e3d44fed09a5662be3 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Mon, 25 Jan 2021 19:14:35 +0100 Subject: [PATCH 02/12] formatting --- macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 129a7bb5..10bdf8ea 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -1062,7 +1062,7 @@ fn mksection(prefix: &str, symbol: &str) -> String { sub_section.hash(&mut hasher); sub_section = format!(",{:x}", hasher.finish()); } - + format!(".defmt{}", sub_section) } From c8a2434ad396f78f5c079a870fed2cbaa659e6f7 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Mon, 25 Jan 2021 19:46:30 +0100 Subject: [PATCH 03/12] defer section name to actual target, not host target --- macros/src/lib.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 10bdf8ea..34be4b4a 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -180,13 +180,6 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { ) .into() } else { - let section = { - if cfg!(target_os = "macos") { - ".defmt,end_timestamp" - } else { - ".defmt.end.timestamp" - } - }; quote!( const _: () = { #[export_name = "_defmt_timestamp"] @@ -204,7 +197,8 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { // Unique symbol name to prevent multiple `timestamp!` invocations in the crate graph. // Uses `#symname` to ensure it is not discarded by the linker. #[no_mangle] - #[link_section = #section] + #[cfg_attr(target_os = "macos", link_section = ".defmt,end_timestamp")] + #[cfg_attr(not(target_os = "macos"), link_section = ".defmt.end_timestamp")] static __DEFMT_MARKER_TIMESTAMP_WAS_DEFINED: &u8 = &#symname; }; ) @@ -995,13 +989,15 @@ pub fn internp(ts: TokenStream) -> TokenStream { let sym = symbol::Symbol::new("prim", &ls).mangle(); - let section = mksection("prim.", &sym); + let section = mksection(false, "prim.", &sym); + let section_macos = mksection(true, "prim.", &sym); if cfg!(feature = "unstable-test") { quote!({ defmt::export::fetch_add_string_index() as u8 }) } else { quote!({ - #[link_section = #section] + #[cfg_attr(target_os = "macos", link_section = #section_macos)] + #[cfg_attr(not(target_os = "macos"), link_section = #section)] #[export_name = #sym] static S: u8 = 0; &S as *const u8 as u8 @@ -1054,10 +1050,10 @@ pub fn write(ts: TokenStream) -> TokenStream { /// returns (note the comma character for macos): /// under macos: ".defmt," + 16 character hex digest of symbol's hash /// otherwise: ".defmt." + prefix + symbol -fn mksection(prefix: &str, symbol: &str) -> String { +fn mksection(macos: bool, prefix: &str, symbol: &str) -> String { let mut sub_section = format!(".{}{}", prefix, symbol); - if cfg!(target_os = "macos") { + if macos { let mut hasher = DefaultHasher::new(); sub_section.hash(&mut hasher); sub_section = format!(",{:x}", hasher.finish()); @@ -1068,10 +1064,12 @@ fn mksection(prefix: &str, symbol: &str) -> String { fn mkstatic(varname: Ident2, string: &str, tag: &str) -> TokenStream2 { let sym = symbol::Symbol::new(tag, string).mangle(); - let section = mksection("", &sym); + let section = mksection(false, "", &sym); + let section_macos = mksection(true, "", &sym); quote!( - #[link_section = #section] + #[cfg_attr(target_os = "macos", link_section = #section_macos)] + #[cfg_attr(not(target_os = "macos"), link_section = #section)] #[export_name = #sym] static #varname: u8 = 0; ) From c63e42969e39fd409c1bc7fbbe5d497e44ef43d1 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Mon, 25 Jan 2021 19:56:20 +0100 Subject: [PATCH 04/12] fix end timestamp link section name --- macros/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 34be4b4a..7ac01980 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -197,8 +197,8 @@ pub fn timestamp(ts: TokenStream) -> TokenStream { // Unique symbol name to prevent multiple `timestamp!` invocations in the crate graph. // Uses `#symname` to ensure it is not discarded by the linker. #[no_mangle] - #[cfg_attr(target_os = "macos", link_section = ".defmt,end_timestamp")] - #[cfg_attr(not(target_os = "macos"), link_section = ".defmt.end_timestamp")] + #[cfg_attr(target_os = "macos", link_section = ".defmt,end.timestamp")] + #[cfg_attr(not(target_os = "macos"), link_section = ".defmt.end.timestamp")] static __DEFMT_MARKER_TIMESTAMP_WAS_DEFINED: &u8 = &#symname; }; ) From fcb86aecd7b9dd45525780f0deba217fe282001b Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Tue, 26 Jan 2021 17:03:38 +0100 Subject: [PATCH 05/12] test that basic usage links successfully --- tests/basic_usage.rs | 15 +++++++++++++++ tests/ui.rs | 1 + 2 files changed, 16 insertions(+) create mode 100644 tests/basic_usage.rs diff --git a/tests/basic_usage.rs b/tests/basic_usage.rs new file mode 100644 index 00000000..b5e5005c --- /dev/null +++ b/tests/basic_usage.rs @@ -0,0 +1,15 @@ +fn main() { + defmt::info!("hello"); +} + +#[defmt::global_logger] +struct Logger; + +unsafe impl defmt::Logger for Logger { + fn acquire() -> Option> { + None + } + unsafe fn release(_writer: core::ptr::NonNull) {} +} + +defmt::timestamp!("{=u32}", 0); diff --git a/tests/ui.rs b/tests/ui.rs index 9316ae83..1bb65f26 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -7,5 +7,6 @@ fn ui() { { let t = trybuild::TestCases::new(); t.compile_fail("tests/ui/*.rs"); + t.pass("tests/basic_usage.rs"); } } From 9ded420eabcf5acd87cade1622abe624a666df1d Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Tue, 26 Jan 2021 17:56:57 +0100 Subject: [PATCH 06/12] fix another link section --- firmware/defmt-rtt/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firmware/defmt-rtt/src/lib.rs b/firmware/defmt-rtt/src/lib.rs index 43f5588f..147b5115 100644 --- a/firmware/defmt-rtt/src/lib.rs +++ b/firmware/defmt-rtt/src/lib.rs @@ -217,7 +217,8 @@ unsafe fn handle() -> &'static Channel { }, }; - #[link_section = ".uninit.defmt-rtt.BUFFER"] + #[cfg_attr(target_os = "macos", link_section = ".uninit,defmt-rtt.BUFFER")] + #[cfg_attr(not(target_os = "macos"), link_section = ".uninit.defmt-rtt.BUFFER")] static mut BUFFER: [u8; SIZE] = [0; SIZE]; static NAME: &[u8] = b"defmt\0"; From 546a4dde36a1eb1629b2655b595a1e98041cf2f6 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Wed, 27 Jan 2021 14:58:28 +0100 Subject: [PATCH 07/12] disable compilation test for now --- tests/ui.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/ui.rs b/tests/ui.rs index 1bb65f26..aad91fe5 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -7,6 +7,10 @@ fn ui() { { let t = trybuild::TestCases::new(); t.compile_fail("tests/ui/*.rs"); - t.pass("tests/basic_usage.rs"); + + // TODO once the corresponding fix in cortex-m-rt has been released, + // () + // re-enable this test and remove the macos special casing in `ci.yml` + // t.pass("tests/basic_usage.rs"); } } From a2af0f14c8f42b269b6af21590a28145a45d7b90 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Wed, 27 Jan 2021 15:02:27 +0100 Subject: [PATCH 08/12] mention the actual cortex-m-rt PRs --- tests/ui.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ui.rs b/tests/ui.rs index aad91fe5..36c0ab6c 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -9,7 +9,8 @@ fn ui() { t.compile_fail("tests/ui/*.rs"); // TODO once the corresponding fix in cortex-m-rt has been released, - // () + // ( https://github.com/rust-embedded/cortex-m-rt/pull/306 + // https://github.com/rust-embedded/cortex-m-rt/pull/307 ) // re-enable this test and remove the macos special casing in `ci.yml` // t.pass("tests/basic_usage.rs"); } From d20ec320f39023186f2689542b109fc9750ec1da Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Wed, 27 Jan 2021 15:37:33 +0100 Subject: [PATCH 09/12] delete basic usage test for now --- tests/basic_usage.rs | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 tests/basic_usage.rs diff --git a/tests/basic_usage.rs b/tests/basic_usage.rs deleted file mode 100644 index b5e5005c..00000000 --- a/tests/basic_usage.rs +++ /dev/null @@ -1,15 +0,0 @@ -fn main() { - defmt::info!("hello"); -} - -#[defmt::global_logger] -struct Logger; - -unsafe impl defmt::Logger for Logger { - fn acquire() -> Option> { - None - } - unsafe fn release(_writer: core::ptr::NonNull) {} -} - -defmt::timestamp!("{=u32}", 0); From a63a541ed39ad2b0d7999ad15462e420205b88f7 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Wed, 27 Jan 2021 15:38:44 +0100 Subject: [PATCH 10/12] dox --- tests/ui.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ui.rs b/tests/ui.rs index 36c0ab6c..c2a1b2ce 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -11,7 +11,8 @@ fn ui() { // TODO once the corresponding fix in cortex-m-rt has been released, // ( https://github.com/rust-embedded/cortex-m-rt/pull/306 // https://github.com/rust-embedded/cortex-m-rt/pull/307 ) - // re-enable this test and remove the macos special casing in `ci.yml` + // re-enable this test (deleted in commit d20ec32) and remove the macos special casing in `ci.yml` + // also check out improved approach re: linker sections in the second cortex-m-rt PR // t.pass("tests/basic_usage.rs"); } } From c7750c50635d4009b16a2860a82f0fa57816f5d0 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Wed, 27 Jan 2021 15:39:13 +0100 Subject: [PATCH 11/12] *correct* dox --- tests/ui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui.rs b/tests/ui.rs index c2a1b2ce..4d7de3ab 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -10,7 +10,7 @@ fn ui() { // TODO once the corresponding fix in cortex-m-rt has been released, // ( https://github.com/rust-embedded/cortex-m-rt/pull/306 - // https://github.com/rust-embedded/cortex-m-rt/pull/307 ) + // https://github.com/rust-embedded/cortex-m-rt/pull/310 ) // re-enable this test (deleted in commit d20ec32) and remove the macos special casing in `ci.yml` // also check out improved approach re: linker sections in the second cortex-m-rt PR // t.pass("tests/basic_usage.rs"); From d91fa125e96f3fc552bd99aee73ddbf2ee3e9ae3 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 28 Jan 2021 19:04:52 +0100 Subject: [PATCH 12/12] change CI config to maybe appease the microsoft linker gods --- .github/workflows/ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86f9ea25..f04f6146 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,8 +44,8 @@ jobs: - name: Check that crates build with unstable-test feature run: RUSTFLAGS='--deny warnings' cargo check --all --features unstable-test shell: bash - - name: Run tests on Ubuntu/Windows - if: matrix.os != 'macOS-latest' + - name: Run tests on Ubuntu + if: matrix.os == 'linux-latest' run: cargo test --workspace --features unstable-test - name: Run tests on macOS # NOTE defmt does not build for macOS because its `cortex-m-rt` dependency doesn't @@ -53,7 +53,10 @@ jobs: # `cargo test --workspace` and have to build the test suites individually instead if: matrix.os == 'macOS-latest' run: cargo test -p defmt -p defmt-decoder -p defmt-parser -p defmt-macros -p defmt-logger -p defmt-print --all-features - + - name: Run tests on Windows + # NOTE there might be a linker bug on the CI's current Windows installation, still investigating + if: matrix.os == 'windows-latest' + run: cargo test -p defmt -p defmt-decoder -p defmt-parser -p defmt-macros -p defmt-logger -p defmt-print --all-features no-std: runs-on: ubuntu-latest steps: