Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

[RFC] [breaking-change] don't depend on GCC by default #91

Closed
japaric opened this issue Aug 20, 2018 · 8 comments · Fixed by #95
Closed

[RFC] [breaking-change] don't depend on GCC by default #91

japaric opened this issue Aug 20, 2018 · 8 comments · Fixed by #95
Labels
needs-decision RFC This issue needs your input!
Milestone

Comments

@japaric
Copy link
Member

japaric commented Aug 20, 2018

It's very likely that the thumb targets will soon switch to rust-lld as their default linker (see
rust-embedded/wg#160). If we want to achieve the goal of not depending on GCC, by default, to
build Cortex-M applications then we also have to make core crates like this one not depend on GCC by
default. This RFC is exactly about that.

I propose that in the next minor (breaking) release, v0.6.0, we add an opt-in Cargo feature named
"exception-frame" that changes the signature of the HardFault handler as it's shown below:

// "exception-frame" disabled
#[exception(HardFault)]
fn handler(/* no `ExceptionFrame` here */) -> ! {
    // ..
}

// "exception-frame" enabled
#[exception(HardFault)]
fn handler(ef: &ExceptionFrame) -> ! {
    // ..
}

The rationale for this change is that providing ExceptionFrame information to the handler requires
an external assembly file and that
makes this crate depend on GCC. By making this information opt-in this crate would, by default, work
w/o GCC being present. If the user wants the ExceptionFrame information they'll need to have GCC
installed.

cc @rust-embedded/cortex-m @hannobraun @crawford

@japaric japaric added needs-decision RFC This issue needs your input! labels Aug 20, 2018
@japaric japaric added this to the RC1 milestone Aug 20, 2018
@hannobraun
Copy link
Member

Sounds great!

@korken89
Copy link
Contributor

I think this is a good compromise.

@thejpster
Copy link
Contributor

Seems reasonable to me.

@adamgreig
Copy link
Member

This will also affect #77.

Will it possible to grab the &ExceptionFrame without external or inline assembly using the proposed __arm_rsr in rust-embedded/wg/pull/184 to read MSP and call the user handler with it? The same idea could be extended to implement #77 in pure Rust without the external assembly.

Some quick experimenting suggests this does work, but I worry about how fragile it might be.

@japaric
Copy link
Member Author

japaric commented Aug 21, 2018

@adamgreig I believe that if we implement that in Rust it would not be reliable. If it's all in Rust then the compiler can inline UserHardFault into HardFault which can cause the a function prelude to be generated and the prelude would modify SP before it's read.

We could mark UserHardFault as inline(never) and that may make it more reliable.

This will also affect #77.

#77 seems complex enough that a Rust implementation would very likely not work properly when unoptimized. In that mode the compiler makes more use of the stack which would modify SP before it's read.

@adamgreig
Copy link
Member

It's a shame rust-lang/rust/issues/32408 isn't stabalised yet or we could just use #[naked]. It's in nightly.

Anyway, I think your suggestion of #[inline(never) on UserHardFault, and a HardFault defined in lib/src.rs which just reads MSP, casts to a &ExceptionFrame, and passes to UserHardFault, which does the same type-checking and then calls the user-provided function as currently, might work. I'll have a play tonight with some long, stack-using user handlers and see how well it works. If so we could retain this feature for stable rust without any asm which would be nice.

#77

I guess we can investigate after resolving the issue here.

@adamgreig
Copy link
Member

I've disabled asm.s, and then implemented my own HardFault (which could live in cortex-m-rt/src/lib.rs once msp can be read from core::arch::arm):

#[no_mangle]
pub unsafe extern "C" fn HardFault() -> ! {
    extern "C" { fn UserHardFault(eh: &ExceptionFrame) -> !; }
    let ef: &ExceptionFrame = &*(cortex_m::register::msp::read() as *const _);
    UserHardFault(ef);
}

exception!(HardFault, hf);

#[inline(always)]
fn hf(ef: &ExceptionFrame) -> ! {
    if ef.pc == 0x0700_0000 {
        loop { unsafe { core::ptr::read_volatile(0x0000_1234 as *const u32); } }
    } else {
        loop { unsafe { core::ptr::read_volatile(0x0000_1238 as *const u32); } }
    }
}

I marked UserHardFault as #[inline(never)] in the exception!() macro.

I can then trigger a HardFault with:

fn crasher() {
    let x: extern "C" fn() = unsafe { core::mem::transmute(0x07000000 as *const u32) };
    x();
}

The generated code in release is identical to the current external assembly solution except for the trailing UDF in HardFault:

080003b2 <HardFault>:
 80003b2:	f3ef 8008 	mrs	r0, MSP
 80003b6:	f000 f804 	bl	80003c2 <UserHardFault>
 80003ba:	defe      	udf	#254	; 0xfe

080003c2 <UserHardFault>:
 80003c2:	6980      	ldr	r0, [r0, #24]
 80003c4:	f1b0 6fe0 	cmp.w	r0, #117440512	; 0x7000000
 80003c8:	d103      	bne.n	80003d2 <UserHardFault+0x10>
 80003ca:	f241 2034 	movw	r0, #4660	; 0x1234
 80003ce:	6801      	ldr	r1, [r0, #0]
 80003d0:	e7fd      	b.n	80003ce <UserHardFault+0xc>
 80003d2:	f241 2038 	movw	r0, #4664	; 0x1238
 80003d6:	6801      	ldr	r1, [r0, #0]
 80003d8:	e7fd      	b.n	80003d6 <UserHardFault+0x14>

and in debug, we branch to read MSR but without touching the stack:

0800021a <_ZN8cortex_m8register3msp4read17he228d82a29c1abfbE>:
 800021a:       f3ef 8008       mrs     r0, MSP
 800021e:       4770            bx      lr

080003f6 <HardFault>:
 80003f6:	f7ff ff10 	bl	800021a <_ZN8cortex_m8register3msp4read17he228d82a29c1abfbE>
 80003fa:	f000 f832 	bl	8000462 <UserHardFault>
 80003fe:	defe      	udf	#254	; 0xfe

08000462 <UserHardFault>:
 8000462:	6980      	ldr	r0, [r0, #24]
 8000464:	f1b0 6fe0 	cmp.w	r0, #117440512	; 0x7000000
 8000468:	d104      	bne.n	8000474 <UserHardFault+0x12>
 800046a:	f241 2034 	movw	r0, #4660	; 0x1234
 800046e:	f7ff fec8 	bl	8000202 <_ZN4core3ptr13read_volatile17ha191faccd0dd90d3E>
 8000472:	e7fa      	b.n	800046a <UserHardFault+0x8>
 8000474:	f241 2038 	movw	r0, #4664	; 0x1238
 8000478:	f7ff fec3 	bl	8000202 <_ZN4core3ptr13read_volatile17ha191faccd0dd90d3E>
 800047c:	e7fa      	b.n	8000474 <UserHardFault+0x12>

It seems to work as required. Even with a bunch of silliness I can't get it to try and touch the stack in the UserHardFault prologue, but anyway since that method is inline(never) I wouldn't expect it to matter.

That all said I have discovered that in debug mode my gdb crashes during a HardFault unless the debugger is set to intercept it, both on current master and with this change. Fine on release. Maybe a gdb bug, who knows...

@japaric
Copy link
Member Author

japaric commented Aug 25, 2018

I have proposed an alternative that doesn't require a breaking change or adding a new Cargo feature in RFC #95.

bors bot added a commit that referenced this issue Aug 26, 2018
95: [RFC] remove build dependency on arm-none-eabi-gcc (binary blob alternative) r=japaric a=japaric

Before this commit we used gcc to assemble external assembly files into object
files that we linked into our Rust program.

This commit drops the dependency on gcc by shipping the already assembled object
files with this crate source code.

---

This is an alternative to RFC #91 that doesn't require a breaking change or
adding a new Cargo feature and can be implemented right now.

See #91 for the rationale of dropping the dependency on gcc.

This approach can be applied to other Cortex-M crates like cortex-m-semihosting
and cortex-m (would subsume RFC rust-embedded/cortex-m#107).

This seems like an overall better approach to me, but before I go opening more
PRs I want to hear your thoughts, @rust-embedded/cortex-m

closes #91

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
@bors bors bot closed this as completed in #95 Aug 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-decision RFC This issue needs your input!
Projects
None yet
5 participants