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

Upgrade Rust nightly #370

Merged
merged 1 commit into from
May 25, 2017
Merged

Upgrade Rust nightly #370

merged 1 commit into from
May 25, 2017

Conversation

ppannuto
Copy link
Member

@ppannuto ppannuto commented May 4, 2017

Okay.. so this has been a truly miserable experience:

Once upon a time I swear I found a page that documented the "target specs" file (i.e. sam4l.json), but that page is lost to the annals of somewhere and is un-google-able. Rust has updated how they handle target specs which required a fair bit of stumbling in the dark :/

The key issue seems to be this one rust-lang/rust#35021 which in a particularly sick joke changed the no-compiler-rt flag from doing what we want to being a silent no-op. The effect is that now we need to pass -nostartflags to the linker.

Rust's added support for multiple "linker flavors", so you have to choose one. That's fine, but now in order to pass any arguments to the linker, you have to include the flavor specifier again. This would almost make sense if you could have multiple flavors, but you can't, there can only be one running linker flavor, so what's the point of scoping to arguments? And back on the real theme here, why does Cargo silently accept and ignore arguments that it's not going to do anything with :( ? Anyways, this is what you want:

    "linker-flavor": "gcc",
    "pre-link-args": {
        "gcc": [
            "-nostartfiles",
            "-mcpu=cortex-m4",
            "-mthumb",
            "-mfloat-abi=soft",
            "-Wl,-gc-sections",
            "-Tlayout.ld"
        ]
    },

Great, now we've gotten rid of undefined references from _start and friends, now we get to the other problem created from that issue linked above. Rust has decided to port all of the compiler intrinsics (all the code that the compiler can emit while it's working, stuff like __aeabi_memclr4 and friends), as they're not yet done, we have undefined references again.

But! There's hope, they have a fallback system that will simply plug in the old C versions of each of the intrinsics whenever a rust one is missing. Unfortunately this doesn't work because we're building for a "sam4l" target, and the C compiler doesn't have a clue how to emit intrinsics for a "sam4l":

TARGET_CC = None
CC = None
HOST = Some("x86_64-apple-darwin")
CROSS_COMPILE = None
TARGET = Some("sam4l.json")
HOST = Some("x86_64-apple-darwin")
CFLAGS_sam4l.json = None
CFLAGS_sam4l.json = None
TARGET_CFLAGS = None
CFLAGS = None
PROFILE = Some("release")
running: "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-fno-builtin" "-fvisibility=hidden" "-fomit-frame-pointer" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/Volumes/code/helena-project/tock/boards/hail/target/sam4l/release/build/compiler_builtins-a004f846c1f47c1c/out/./compiler-rt/lib/builtins/arm/aeabi_cdcmp.o" "-c" "./compiler-rt/lib/builtins/arm/aeabi_cdcmp.S"
cargo:warning=./compiler-rt/lib/builtins/arm/aeabi_cdcmp.S:27:9: error: unknown directive
cargo:warning=        .syntax unified

It doesn't/can't choose the right compiler or flags because it doesn't have a clue what a sam4l is. While there's probably a way to tell it that "sam4l" target should use "thumbv7em-linux-eabi" intrinsics, I'm officially tired and really frustrated. We'll pick this up again tomorrow.

@ppannuto ppannuto added blocked Waiting on something, like a different PR or a dependency. kernel labels May 4, 2017
@ppannuto
Copy link
Member Author

Update: rust-lang/cc-rs#158

@alevy alevy mentioned this pull request May 24, 2017
@alevy
Copy link
Member

alevy commented May 24, 2017

/cc maybe @japaric can save us!

@alevy alevy added needs-squash and removed blocked Waiting on something, like a different PR or a dependency. labels May 25, 2017
@alevy alevy changed the title WIP: bump rust nightly Upgrade Rust nightly May 25, 2017
@alevy
Copy link
Member

alevy commented May 25, 2017

Thanks to @japaric, btw, for explaining how to get this working!

I haven't tested extensively yet, but otherwise I think this is ready.

@bradjc
Copy link
Contributor

bradjc commented May 25, 2017

My suggestion:

diff --git a/boards/Makefile.common b/boards/Makefile.common
index 356eba87..6c77c151 100644
--- a/boards/Makefile.common
+++ b/boards/Makefile.common
@@ -41,6 +41,14 @@ ifneq ($(shell RUSTUP_TOOLCHAIN=$(RUSTUP_TOOLCHAIN) rustc --version), $(RUSTC_VE
   endif
 endif
 
+# Check that xargo is installed
+CHECK := $(strip $(firstword $(shell $(XARGO) --version 2>&1)))
+ifneq ($(CHECK), xargo)
+  $(warning You do not seem to have xargo installed.)
+  $(warning Installing xargo now.)
+  DUMMY := $(shell cd /tmp && cargo install xargo; cd -)
+endif
+
 # Dump configuration for verbose builds
 ifneq ($(V),)
   $(info )

@ppannuto
Copy link
Member Author

This has been rebased and squashed ; I think it should be good to go

  * Use Xargo to get automatically built core
  * Remove dependency on rust-core
  * Remove cutom targets; add RUSTFLAGS where needed
  * Update use of Unique and NonZero
  * Add "mem" feature to "compiler_builtins" crate
  * auto-install xargo if needed
@bradjc bradjc merged commit e5b5c2f into master May 25, 2017
@alevy alevy deleted the bump-nightly branch May 25, 2017 23:55
ppannuto added a commit that referenced this pull request May 26, 2017
This should've been part of #370 / 751141b
@ppannuto
Copy link
Member Author

@alevy did you have this working on the NRF? It compiles, but it seems to hard fault with some immediacy.

First trick, in the current code, all the ISRs are weakly aliased to the Dummy_Handler. In that case, gdb will report the last aliased symbol alphabetically, so don't get tricked into thinking that you're in the WDT_Handler, and going down the rabbit hole of trying to figure out where the watchdog is doing anything. As this code will be going away with #369, I threw this quick hack in to see which ISR tripped:

diff --git a/chips/nrf51/src/crt1.c b/chips/nrf51/src/crt1.c
index bf6d773c..dbe9ad9e 100644
--- a/chips/nrf51/src/crt1.c
+++ b/chips/nrf51/src/crt1.c
@@ -20,19 +20,40 @@ extern uint32_t _ezero;
 extern uint32_t _srelocate;
 extern uint32_t _erelocate;

+void reset_handler(void);
+
 void Dummy_Handler(void)
 {
        while (1) {
        }
 }

-void reset_handler(void);
-void NMI_Handler(void) __attribute__ ((weak, alias("Dummy_Handler")));
-void HardFault_Handler(void)
-    __attribute__ ((weak, alias("Dummy_Handler")));
-void SVC_Handler(void) __attribute__ ((weak, alias("Dummy_Handler")));
-void PendSV_Handler(void) __attribute__ ((weak, alias("Dummy_Handler")));
-void SysTick_Handler(void) __attribute__ ((weak, alias("Dummy_Handler")));
+void NMI_Handler(void) __attribute__ ((weak));
+void NMI_Handler(void) {
+       while (1) {
+       }
+}
+
+void HardFault_Handler(void) __attribute__ ((weak));
+void HardFault_Handler(void) {
+       while (1) {
+       }
+}
+void SVC_Handler(void) __attribute__ ((weak));
+void SVC_Handler(void) {
+       while (1) {
+       }
+}
+void PendSV_Handler(void) __attribute__ ((weak));
+void PendSV_Handler(void) {
+       while (1) {
+       }
+}
+void SysTick_Handler(void) __attribute__ ((weak));
+void SysTick_Handler(void) {
+       while (1) {
+       }
+}
 PERIPHERAL_INTERRUPT_HANDLERS

With no apps loaded, this'll eventually end up looping at 0x000089bc, or:

000089bc <HardFault_Handler>:
    89bc:	e7fe      	b.n	89bc <HardFault_Handler>
    89be:	46c0      	nop			; (mov r8, r8)

Keeping on keeping on, info registers reports lr 0xfffffff9, so the relevant stack is the MSP, msp 0x20001838. Checking out the stacked registers:

(gdb) x/x 0x20001838 + 4*0
0x20001838:	0x20002270  <-- r0
(gdb) x/x 0x20001838 + 4*1
0x2000183c:	0x00000000  <-- r1
(gdb) x/x 0x20001838 + 4*2
0x20001840:	0x00000000  <-- r2
(gdb) x/x 0x20001838 + 4*3
0x20001844:	0x200022a7  <-- r3
(gdb) x/x 0x20001838 + 4*4
0x20001848:	0x20000ad4  <-- r12
(gdb) x/x 0x20001838 + 4*5
0x2000184c:	0xffffffff  <-- lr
(gdb) x/x 0x20001838 + 4*6
0x20001850:	0x00007f02  <-- pc
(gdb) x/x 0x20001838 + 4*7
0x20001854:	0x01000000  <-- psr

From this we learn that we're nominally at the top-level (lr of f's), so reset_handler or more likely something inlined into it. That PC professes to be part of

00007c98 <<char as core::fmt::Debug>::fmt>:
...
    7ef2:	9903      	ldr	r1, [sp, #12]
    7ef4:	4608      	mov	r0, r1
    7ef6:	b007      	add	sp, #28
    7ef8:	bdf0      	pop	{r4, r5, r6, r7, pc}   <----------------- Note this is the fn return
    7efa:	46c0      	nop			; (mov r8, r8)
    7efc:	48cb      	ldr	r0, [pc, #812]	; (822c <core::fmt::write+0x1d4>)
    7efe:	fffd 5929 	vtbl.8	d21, {d13-d14}, d25
    7f02:	fffd fffe 			; <UNDEFINED> instruction: 0xfffdfffe
    7f06:	001f      	movs	r7, r3
    7f08:	b81e      			; <UNDEFINED> instruction: 0xb81e

In practice, that's a garbage PC (that address is data used by that function, not an instruction, so ignore that "decoding"). So most likely it was a garbage return to the top-level reset_handler that got us into this situation. We get in a state like this likely because of stack corruption one frame prior, but continuing to try to chase frames when the PC is corrupt gets real tricky.

I'm going to drop this for tonight, but wanted to give you some place to start if you dig into what broke for the nrf tomorrow.

frenicth pushed a commit to frenicth/tock that referenced this pull request May 31, 2017
This should've been part of tock#370 / 751141b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants