-
Notifications
You must be signed in to change notification settings - Fork 218
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
Critical section causes unexpected behaviour in --release
mode
#505
Comments
Update: I was running |
ufmt::uwriteln!
with a u64 in a critical section sometimes disables interrupts--release
mode
I've found a workaround by adding avr_device::interrupt::free(
#[inline(never)]
|_| {
ufmt::uwriteln!(
serial,
"Counter: {}, interrupts enabled: {}",
counter,
int_en
)
.unwrap_infallible();
},
); |
I'm suspecting this is a compilation / linker issue rather than crate-level issue. [profile.release]
panic = "abort"
codegen-units = 1
debug = true
lto = true
opt-level = "s" to [profile.release]
panic = "abort"
codegen-units = 2 # anything > 1
debug = true
opt-level = "s" my example works again. Similarly, if I enable either [profile.release]
panic = "abort"
codegen-units = 1
debug = true
lto = true
opt-level = "s"
overflow-checks = true # enable either one (or both)
debug-assertions = true # enable either one (or both) |
Paging @Patryk27, maybe you know something here? Otherwise we'll need to look at the generated assembly to find out where this is falling apart. My first guess would be that some u64 intrinsics are not working correctly... |
Judging by the behavior, this does look like a codegen bug, but I don't recall anything that could cause it (in this case the intrinsics should be fine, but always worth to take a look there, considering the history 😄) - I'll also try to take a look during the week. |
Minimized: #![no_std]
#![no_main]
use avr_device::interrupt;
use panic_halt as _;
#[arduino_hal::entry]
fn main() -> ! {
let dp = arduino_hal::Peripherals::take().unwrap();
let pins = arduino_hal::pins!(dp);
let mut serial = arduino_hal::default_serial!(dp, pins, 57600);
unsafe {
interrupt::enable();
}
let irq = interrupt::disable_save();
let counter = core::hint::black_box(123u64);
_ = ufmt::uwriteln!(serial, "{}", counter);
if irq.enabled() {
_ = ufmt::uwriteln!(serial, "irq? 1");
} else {
_ = ufmt::uwriteln!(serial, "irq? 0");
}
loop {
//
}
} As it is, it prints |
Alright, this one seems to be a proper bug in the LLVM codegen (i.e. doesn't seem to be related to the Minimized even more (by inlining stuff done by #![no_std]
#![no_main]
use avr_device::interrupt;
use panic_halt as _;
#[arduino_hal::entry]
fn main() -> ! {
unsafe {
interrupt::enable();
}
let irq = interrupt::disable_save();
let counter = core::hint::black_box(12u64); // at least a two-digit number is required to trigger the bug
let mut buf = [0; 3];
core::hint::black_box(fmt_num(&mut buf, counter));
if irq.enabled() {
report_irq_enabled();
} else {
report_irq_disabled();
}
loop {
//
}
}
fn fmt_num(buf: &mut [u8], mut n: u64) -> &[u8] { // corresponds to uxx!() from ufmt
let mut i = 0;
loop {
unsafe {
*buf.get_unchecked_mut(i) = (n % 10) as u8;
}
n /= 10;
if n == 0 {
break;
} else {
i += 1;
}
}
if i > 2 { // branch not taken, but required for the bug to occur
panic!("ayy");
}
buf
}
// separated from `main()` to make following the assembly easier
#[inline(never)]
fn report_irq_enabled() {
let dp = arduino_hal::Peripherals::take().unwrap();
let pins = arduino_hal::pins!(dp);
let mut serial = arduino_hal::default_serial!(dp, pins, 57600);
serial.write_byte(b'E');
serial.write_byte(b'\n');
}
// ditto
#[inline(never)]
fn report_irq_disabled() {
let dp = arduino_hal::Peripherals::take().unwrap();
let pins = arduino_hal::pins!(dp);
let mut serial = arduino_hal::default_serial!(dp, pins, 57600);
serial.write_byte(b'D');
serial.write_byte(b'\n');
} If we inspect the assembly (e.g. by running
Tracking further 🔍 |
Status: got it! -- llvm/llvm-project#85277 Once the fix is merged to LLVM, I should be able to push it to rustc within a couple of days. |
Update to LLVM 18.1.2 Fixes rust-lang#122476. Also contains fixes for Rahix/avr-hal#505 and llvm/llvm-project#83362. r? `@cuviper`
fyi, toolchain's already fixed (compiling with |
Nice, then it's time to bump the compiler version again :) |
I've come across inconsistent behavior when attempting to output a
u64
to serial usingufmt
from within a critical section.Originally this came up because I was using a modified version of a
print!
macro from #115, and I was noticing that my interrupts would stop after a few iterations.Edit: this is with the
--release
flag. Removing it seems to solve the issue, although I'd still be curious to know why.(near)Minimal test-case:
Which outputs:
Changing inner type
If I change
INT_COUNTER
's inner type tou32
, or I don't print the counter, interrupts stay enabled and the counter correctly increments.Adding a condition
Even stranger still, if I add a condition to whether I print the counter (with
u64
), IT STARTS WORKING AGAIN...Any advice would be appreciated :)
Dependencies
Tested on
uno
andmega2560
.The text was updated successfully, but these errors were encountered: