discovery
discovery copied to clipboard
Modifying locals potentially broken with recent Rust
I'm working through the discovery book and just did the gdb
session in this chapter.
What I found is that while info locals
properly reports my change to half_period
, the behaviour did not change. So I went through calls to the delay_ms
methods again and found that the calling parameter is still the original value of 500.
What I am wondering is if this is due to Rust recent changes with respect to constant value optimizations. Could it be that the half_period
(which is a primitive type and only read twice and never written) is optimized away? I can imagine that it would be sufficient to write the value somewhere to the binary and let delay_ms
access it directly without doing the usual stack-frame handling.
My questions are:
- Can you give me hints on how to validate my hypothesis?
- Can we (jointly) fix this by modifying the usage or declaration of
half_period
so that it is really modifiable interactively ingdb
?
I guess fixing this would save others from trying out this example and wondering why it does not work.
I have the same issue.
Stepping through the program in disassemble view (as described in 5.3. Debug it) I found the instructions mov.w r0, #500
and strh.w r0, [r7, #-2]
, which I think store the value of half_period
in memory.
However, right before the subroutine for function delay.delay_ms
is called the raw value is moved to a register mov.w r1, #500
.
I am pretty new to the topic, but as far as I understand my findings, I think your assumption is correct that the compiler is too smart in this case. So far I haven't tried to find a solution or workaround, but I agree that fixing this would help prevent some confusion.
I played around with it some more and found that declaring half_period
as mutable and reassigning its value once solves the issue.
let mut half_period = 0_u16;
half_period = 500_u16;
As much as I think playing around with this feature at that point in the tutorial is very interesting, I am not sure it justifies using strange code like that - unless it is explained in some detail what is happening without it.
Thanks for looking into assembly and confirming my hypothesis. Your code makes sense but is indeed "strange".
I wondered if one could make the variable volatile as it is also done in the register section of the book.
I found the volatile crate, which could be used for that. Still, a developer would not per-se make this variable volatile, as it actually is not expected to change without doing live-rewrite in the debugger. But that would at least provide similar "changability" while having less contrived code.
Maybe leveraging volatile
and explaining the details in a "note"-box would educate the reader and make the example work again.
I also just ran into this issue. @wupdiduh thanks for finding a workaround. I had to modify it a bit:
let mut half_period = 500_u16;
delay.delay_ms(half_period);
half_period = 100;
loop {
I had to use half_period before changing the value, so the compiler wouldn't optimize it away.
From a thread in the Rust forum:
I believe this is a result of MIR constant propagation, which is done by rustc before it generates LLVM IR and runs LLVM's optimization passes. In recent versions of rustc, MIR constant propagation is enabled by default in debug mode because it reduces compile times. You can disable it using the -Zmir-opt-level=0
rustc flag (requires a nightly build of the Rust toolchain).
Unfortunately using -Zmir-opt-level
doesn't work. I've only been able to get around the optimization by doing what @yerke did.
Here are a couple other solutions I came up with.
- Use the volatile crate:
Cargo.toml to add volatile = "0.4.2"
as a dependency:
[dependencies]
aux5 = { path = "auxiliary" }
volatile = "0.4.2"
And the main.rs is now:
#![deny(unsafe_code)]
#![no_main]
#![no_std]
use aux5::{entry, prelude::*, Delay, Leds};
use volatile::Volatile;
#[entry]
fn main() -> ! {
let (mut delay, mut leds): (Delay, Leds) = aux5::init();
let mut half_period = 2000_u16;
let v_half_period = Volatile::new(&mut half_period);
loop {
leds[0].on();
delay.delay_ms(v_half_period.read());
leds[0].off();
delay.delay_ms(v_half_period.read());
}
}
- Use Atomics:
#![deny(unsafe_code)]
#![no_main]
#![no_std]
use aux5::{entry, prelude::*, Delay, Leds};
use core::sync::atomic::{AtomicU16, Ordering};
#[entry]
fn main() -> ! {
let (mut delay, mut leds): (Delay, Leds) = aux5::init();
let half_period = AtomicU16::new(2000_u16);
loop {
leds[0].on();
delay.delay_ms(half_period.load(Ordering::Relaxed));
leds[0].off();
delay.delay_ms(half_period.load(Ordering::Relaxed));
}
}
Change half_period with:
(gdb) set half_period = core::sync::atomic::AtomicU16::new(100)