Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issue #336 #357

Merged
merged 12 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion firmware/defmt-rtt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
35 changes: 30 additions & 5 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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;
};
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
)
Expand Down
7 changes: 7 additions & 0 deletions tests/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}