From d785f9ba76be199a47652f5841260a42390f611b Mon Sep 17 00:00:00 2001 From: Dylan McKay Date: Sun, 26 Jul 2020 23:58:37 +1200 Subject: [PATCH 1/5] [AVR] Replace broken 'avr-unknown-unknown' target with 'avr-unknown-gnu-atmega328' target The `avr-unknown-unknown` target has never worked correctly, always trying to invoke the host linker and failing. It aimed to be a mirror of AVR-GCC's default handling of the `avr-unknown-unknown' triple (assume bare minimum chip features, silently skip linking runtime libraries, etc). This behaviour is broken-by-default as it will cause a miscompiled executable when flashed. This patch improves the AVR builtin target specifications to instead expose only a 'avr-unknown-gnu-atmega328' target. This target system is `gnu`, as it uses the AVR-GCC frontend along with avr-binutils. The target triple ABI is 'atmega328'. In the future, it should be possible to replace the dependency on AVR-GCC and binutils by using the in-progress AVR LLD and compiler-rt support. Perhaps at that point it would make sense to add an 'avr-unknown-unknown-atmega328' target as a better default when implemented. There is no current intention to add in-tree AVR target specifications for other AVR microcontrollers - this one can serve as a reference implementation for other devices via `rustc --print target-spec-json avr-unknown-gnu-atmega328p`. There should be no users of the existing 'avr-unknown-unknown' Rust target as a custom target specification JSON has always been recommended, and the avr-unknown-unknown target could never pass the linking step anyway. --- src/librustc_target/spec/avr_gnu_base.rs | 34 +++++++++++++++++++ .../spec/avr_unknown_gnu_atmega328.rs | 5 +++ .../spec/avr_unknown_unknown.rs | 17 ---------- src/librustc_target/spec/mod.rs | 3 +- 4 files changed, 41 insertions(+), 18 deletions(-) create mode 100644 src/librustc_target/spec/avr_gnu_base.rs create mode 100644 src/librustc_target/spec/avr_unknown_gnu_atmega328.rs delete mode 100644 src/librustc_target/spec/avr_unknown_unknown.rs diff --git a/src/librustc_target/spec/avr_gnu_base.rs b/src/librustc_target/spec/avr_gnu_base.rs new file mode 100644 index 0000000000000..5fcaaaacbeed9 --- /dev/null +++ b/src/librustc_target/spec/avr_gnu_base.rs @@ -0,0 +1,34 @@ +use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; + +/// A base target for AVR devices using the GNU toolchain. +/// +/// Requires GNU avr-gcc and avr-binutils on the host system. +pub fn target(target_cpu: String) -> TargetResult { + Ok(Target { + arch: "avr".to_string(), + data_layout: "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8".to_string(), + llvm_target: "avr-unknown-unknown".to_string(), + target_endian: "little".to_string(), + target_pointer_width: "16".to_string(), + linker_flavor: LinkerFlavor::Gcc, + target_os: "unknown".to_string(), + target_env: "".to_string(), + target_vendor: "unknown".to_string(), + target_c_int_width: 16.to_string(), + options: TargetOptions { + cpu: target_cpu.clone(), + exe_suffix: ".elf".to_string(), + linker: Some("avr-gcc".to_owned()), + pre_link_args: vec![( + LinkerFlavor::Gcc, + vec!["-Os".to_owned(), format!("-mmcu={}", target_cpu)], + )] + .into_iter() + .collect(), + late_link_args: vec![(LinkerFlavor::Gcc, vec!["-lc".to_owned(), "-lgcc".to_owned()])] + .into_iter() + .collect(), + ..super::freestanding_base::opts() + }, + }) +} diff --git a/src/librustc_target/spec/avr_unknown_gnu_atmega328.rs b/src/librustc_target/spec/avr_unknown_gnu_atmega328.rs new file mode 100644 index 0000000000000..5d22598b57b87 --- /dev/null +++ b/src/librustc_target/spec/avr_unknown_gnu_atmega328.rs @@ -0,0 +1,5 @@ +use crate::spec::TargetResult; + +pub fn target() -> TargetResult { + super::avr_gnu_base::target("atmega328".to_owned()) +} diff --git a/src/librustc_target/spec/avr_unknown_unknown.rs b/src/librustc_target/spec/avr_unknown_unknown.rs deleted file mode 100644 index f90a8def0aa2f..0000000000000 --- a/src/librustc_target/spec/avr_unknown_unknown.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::spec::{LinkerFlavor, Target, TargetResult}; - -pub fn target() -> TargetResult { - Ok(Target { - llvm_target: "avr-unknown-unknown".to_string(), - target_endian: "little".to_string(), - target_pointer_width: "16".to_string(), - data_layout: "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8".to_string(), - arch: "avr".to_string(), - linker_flavor: LinkerFlavor::Gcc, - target_os: "unknown".to_string(), - target_env: "".to_string(), - target_vendor: "unknown".to_string(), - target_c_int_width: 16.to_string(), - options: super::freestanding_base::opts(), - }) -} diff --git a/src/librustc_target/spec/mod.rs b/src/librustc_target/spec/mod.rs index fa29ff3f8d80f..b16351c063a4f 100644 --- a/src/librustc_target/spec/mod.rs +++ b/src/librustc_target/spec/mod.rs @@ -51,6 +51,7 @@ mod android_base; mod apple_base; mod apple_sdk_base; mod arm_base; +mod avr_gnu_base; mod cloudabi_base; mod dragonfly_base; mod freebsd_base; @@ -581,7 +582,7 @@ supported_targets! { ("aarch64-fuchsia", aarch64_fuchsia), ("x86_64-fuchsia", x86_64_fuchsia), - ("avr-unknown-unknown", avr_unknown_unknown), + ("avr-unknown-gnu-atmega328", avr_unknown_gnu_atmega328), ("x86_64-unknown-l4re-uclibc", x86_64_unknown_l4re_uclibc), From 53b940c74c4f56ef075ee3c71f3cb157aaff65af Mon Sep 17 00:00:00 2001 From: Dylan McKay Date: Fri, 31 Jul 2020 03:57:20 +1200 Subject: [PATCH 2/5] [AVR] Remove unnecessary arguments passed to the linker for GNU target In general, linking with libc is not required, only libgcc is needed. As suggested in the code review, a better option for libc support is by building it into rust-lang/libc directly. This also removes the '-Os' argument to the linker, which is a NOP. --- src/librustc_target/spec/avr_gnu_base.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/librustc_target/spec/avr_gnu_base.rs b/src/librustc_target/spec/avr_gnu_base.rs index 5fcaaaacbeed9..878f0e9dcba7d 100644 --- a/src/librustc_target/spec/avr_gnu_base.rs +++ b/src/librustc_target/spec/avr_gnu_base.rs @@ -19,13 +19,12 @@ pub fn target(target_cpu: String) -> TargetResult { cpu: target_cpu.clone(), exe_suffix: ".elf".to_string(), linker: Some("avr-gcc".to_owned()), - pre_link_args: vec![( - LinkerFlavor::Gcc, - vec!["-Os".to_owned(), format!("-mmcu={}", target_cpu)], + pre_link_args: vec![(LinkerFlavor::Gcc, + vec![format!("-mmcu={}", target_cpu)], )] .into_iter() .collect(), - late_link_args: vec![(LinkerFlavor::Gcc, vec!["-lc".to_owned(), "-lgcc".to_owned()])] + late_link_args: vec![(LinkerFlavor::Gcc, vec!["-lgcc".to_owned()])] .into_iter() .collect(), ..super::freestanding_base::opts() From dc2023801281dd21f04c97fc4ec073c02b2abd46 Mon Sep 17 00:00:00 2001 From: Dylan McKay Date: Fri, 31 Jul 2020 04:04:26 +1200 Subject: [PATCH 3/5] [AVR] Merge the 'freestanding' base target spec into AVR base target spec The 'freestanding' module was only ever used for AVR. It was an unnecessary layer of abstraction. This commit merges the 'freestanding_base' module into 'avr_gnu_base'. --- src/librustc_target/spec/avr_gnu_base.rs | 24 ++++++++++++-- src/librustc_target/spec/freestanding_base.rs | 31 ------------------- src/librustc_target/spec/mod.rs | 1 - 3 files changed, 21 insertions(+), 35 deletions(-) delete mode 100644 src/librustc_target/spec/freestanding_base.rs diff --git a/src/librustc_target/spec/avr_gnu_base.rs b/src/librustc_target/spec/avr_gnu_base.rs index 878f0e9dcba7d..ff559c2bfd684 100644 --- a/src/librustc_target/spec/avr_gnu_base.rs +++ b/src/librustc_target/spec/avr_gnu_base.rs @@ -18,16 +18,34 @@ pub fn target(target_cpu: String) -> TargetResult { options: TargetOptions { cpu: target_cpu.clone(), exe_suffix: ".elf".to_string(), + linker: Some("avr-gcc".to_owned()), - pre_link_args: vec![(LinkerFlavor::Gcc, - vec![format!("-mmcu={}", target_cpu)], + dynamic_linking: false, + executables: true, + linker_is_gnu: true, + has_rpath: false, + position_independent_executables: false, + eh_frame_header: false, + pre_link_args: vec![( + LinkerFlavor::Gcc, + vec![ + format!("-mmcu={}", target_cpu), + // We want to be able to strip as much executable code as possible + // from the linker command line, and this flag indicates to the + // linker that it can avoid linking in dynamic libraries that don't + // actually satisfy any symbols up to that point (as with many other + // resolutions the linker does). This option only applies to all + // following libraries so we're sure to pass it as one of the first + // arguments. + "-Wl,--as-needed".to_string(), + ], )] .into_iter() .collect(), late_link_args: vec![(LinkerFlavor::Gcc, vec!["-lgcc".to_owned()])] .into_iter() .collect(), - ..super::freestanding_base::opts() + ..TargetOptions::default() }, }) } diff --git a/src/librustc_target/spec/freestanding_base.rs b/src/librustc_target/spec/freestanding_base.rs deleted file mode 100644 index c338856228dc6..0000000000000 --- a/src/librustc_target/spec/freestanding_base.rs +++ /dev/null @@ -1,31 +0,0 @@ -use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions}; -use std::default::Default; - -pub fn opts() -> TargetOptions { - let mut args = LinkArgs::new(); - - args.insert( - LinkerFlavor::Gcc, - vec![ - // We want to be able to strip as much executable code as possible - // from the linker command line, and this flag indicates to the - // linker that it can avoid linking in dynamic libraries that don't - // actually satisfy any symbols up to that point (as with many other - // resolutions the linker does). This option only applies to all - // following libraries so we're sure to pass it as one of the first - // arguments. - "-Wl,--as-needed".to_string(), - ], - ); - - TargetOptions { - dynamic_linking: false, - executables: true, - linker_is_gnu: true, - has_rpath: false, - pre_link_args: args, - position_independent_executables: false, - eh_frame_header: false, - ..Default::default() - } -} diff --git a/src/librustc_target/spec/mod.rs b/src/librustc_target/spec/mod.rs index b16351c063a4f..d6e8b304380ca 100644 --- a/src/librustc_target/spec/mod.rs +++ b/src/librustc_target/spec/mod.rs @@ -55,7 +55,6 @@ mod avr_gnu_base; mod cloudabi_base; mod dragonfly_base; mod freebsd_base; -mod freestanding_base; mod fuchsia_base; mod haiku_base; mod hermit_base; From a0905ceff9e8a80aa39bb38dda2e74bddebe613d Mon Sep 17 00:00:00 2001 From: Dylan McKay Date: Fri, 31 Jul 2020 18:41:25 +1200 Subject: [PATCH 4/5] [AVR] Rename the last few remaining references from 'avr-unknown-unknown' to 'avr-unknown-gnu-atmega328' --- library/panic_unwind/src/lib.rs | 2 +- library/std/build.rs | 2 +- src/test/codegen/avr/avr-func-addrspace.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/panic_unwind/src/lib.rs b/library/panic_unwind/src/lib.rs index 7d14893c4cc2e..95675630837c2 100644 --- a/library/panic_unwind/src/lib.rs +++ b/library/panic_unwind/src/lib.rs @@ -65,7 +65,7 @@ cfg_if::cfg_if! { // - os=none ("bare metal" targets) // - os=uefi // - nvptx64-nvidia-cuda - // - avr-unknown-unknown + // - arch=avr #[path = "dummy.rs"] mod real_imp; } diff --git a/library/std/build.rs b/library/std/build.rs index 04bfed12153ec..a787e6d43fc99 100644 --- a/library/std/build.rs +++ b/library/std/build.rs @@ -83,7 +83,7 @@ fn main() { // - os=none ("bare metal" targets) // - mipsel-sony-psp // - nvptx64-nvidia-cuda - // - avr-unknown-unknown + // - arch=avr // - tvos (aarch64-apple-tvos, x86_64-apple-tvos) // - uefi (x86_64-unknown-uefi, i686-unknown-uefi) // - JSON targets diff --git a/src/test/codegen/avr/avr-func-addrspace.rs b/src/test/codegen/avr/avr-func-addrspace.rs index 6d25ca56f1488..0f15729158df1 100644 --- a/src/test/codegen/avr/avr-func-addrspace.rs +++ b/src/test/codegen/avr/avr-func-addrspace.rs @@ -1,4 +1,4 @@ -// compile-flags: -O --target=avr-unknown-unknown --crate-type=rlib +// compile-flags: -O --target=avr-unknown-gnu-atmega328 --crate-type=rlib // needs-llvm-components: avr // This test validates that function pointers can be stored in global variables From c9ead8c895593452677a229fd19909c83283b33f Mon Sep 17 00:00:00 2001 From: Dylan McKay Date: Mon, 24 Aug 2020 20:43:11 +1200 Subject: [PATCH 5/5] [AVR] Replace 'avr-unknown-unknown' with 'avr-unknown-gnu-atmega328' in platform-support.md --- src/doc/rustc/src/platform-support.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index 21874853839bd..546eb74b2cb5d 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -165,7 +165,7 @@ target | std | host | notes `armv7-wrs-vxworks-eabihf` | ? | | `armv7a-none-eabihf` | * | | ARM Cortex-A, hardfloat `armv7s-apple-ios` | ✓[^apple] | | -`avr-unknown-unknown` | ? | | AVR +`avr-unknown-gnu-atmega328` | ✗ | | AVR. Requires `-Z build-std=core` `hexagon-unknown-linux-musl` | ? | | `i386-apple-ios` | ✓[^apple] | | 32-bit x86 iOS `i686-apple-darwin` | ✓ | ✓ | 32-bit OSX (10.7+, Lion+)