Merge #357

357: fix issue #336 r=jonas-schievink a=spookyvision

Introduce special cased symbol names to satisfy the macos linker

Co-authored-by: Anatol Ulrich <anatol.ulrich@ferrous-systems.com>

Author: bors[bot]

.github/workflows/ci.yml

@@ -44,16 +44,19 @@ 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
       # (see https://github.com/rust-embedded/cortex-m-rt/issues/74), so we cannot use
       # `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:

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";

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`
@@ -194,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 = ".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;
             };
         )
@@ -984,13 +988,16 @@ 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(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
@@ -1039,12 +1046,30 @@ 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(macos: bool, prefix: &str, symbol: &str) -> String {
+    let mut sub_section = format!(".{}{}", prefix, symbol);
+
+    if 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(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;
     )

tests/ui.rs

@@ -7,5 +7,12 @@ fn ui() {
     {
         let t = trybuild::TestCases::new();
         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/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");
     }
 }