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

AVR: Miscompilation with Trait Object in Option #74743

Closed
Rahix opened this issue Jul 25, 2020 · 4 comments
Closed

AVR: Miscompilation with Trait Object in Option #74743

Rahix opened this issue Jul 25, 2020 · 4 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-AVR Target: AVR processors (ATtiny, ATmega, etc.) requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Rahix
Copy link

Rahix commented Jul 25, 2020

While playing around with async I stumbled upon an apparent miscomplation. I've tried to reduce it as much as possible and am now left with this:

#![no_std]
#![no_main]
#![feature(llvm_asm)]

#[inline(never)]
#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
    loop {
        unsafe { llvm_asm!("nop" :::: "volatile") };
    }
}

fn func() -> () {
    unsafe { llvm_asm!("nop" :::: "volatile") };
}

pub struct Func<'a>(pub &'a dyn Fn());

#[no_mangle]
pub extern fn main() -> ! {
    let mut t = Some(Func(&func));

    loop {
        if let Some(t) = t.take() {
            (t.0)();
        }
    }
}

Compiling and running under simavr, I see this code panicking:

#0  core::panicking::panic (expr=...) at /home/rahix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/panicking.rs:50
#1  0x00000244 in core::ptr::swap_nonoverlapping_one (x=0x800adf, y=0x800ae3) at src/main.rs:25
#2  core::mem::swap (x=0x800adf, y=0x800ae3) at /home/rahix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/mem/mod.rs:690
#3  core::mem::replace (dest=0x800adf, src=...) at /home/rahix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/mem/mod.rs:815
#4  core::mem::take (dest=0x800adf) at /home/rahix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/mem/mod.rs:751
#5  core::option::Option<T>::take (self=0x800adf) at /home/rahix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/option.rs:892
#6  avr_async::main () at src/main.rs:24

I should also note that while reducing the code, the error manifested itself in many different ways; it was not always panicking like shown above.

Meta

rustc --version --verbose:

rustc 1.47.0-nightly (5ef299eb9 2020-07-24)
binary: rustc
commit-hash: 5ef299eb9805b4c86b227b718b39084e8bf24454
commit-date: 2020-07-24
host: x86_64-unknown-linux-gnu
release: 1.47.0-nightly
LLVM version: 10.0

cc @dylanmckay

@Rahix Rahix added the C-bug Category: This is a bug. label Jul 25, 2020
@jonas-schievink jonas-schievink added the O-AVR Target: AVR processors (ATtiny, ATmega, etc.) label Jul 25, 2020
@jonas-schievink jonas-schievink added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. labels Nov 7, 2020
@Rahix
Copy link
Author

Rahix commented Nov 21, 2022

I think it needs to be checked whether this bug is still present in current versions. I have the suspicion that it was fixed as part of some LLVM patches related to function pointers.

@workingjubilee
Copy link
Member

We can check, but can you explain the relevance of the llvm_asm! usages? (we can probably replicate the equivalent in current inline assembly, but I'm curious if these are just being used as "black boxes"...)

@Rahix
Copy link
Author

Rahix commented Feb 20, 2023

Well, essentially just as black boxes/optimization barriers. But as the issue was so unstable in what ways it manifested, I'd keep the nops using asm!().

@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@Patryk27
Copy link
Contributor

fwiw, this has been fixed a long time ago - I think this issue can be safely closed 🙂

@oli-obk oli-obk closed this as completed Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-AVR Target: AVR processors (ATtiny, ATmega, etc.) requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants