Skip to content

optimizes out scheduler entry function in no_std environment #131716

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

Closed
rayanmargham opened this issue Oct 14, 2024 · 9 comments
Closed

optimizes out scheduler entry function in no_std environment #131716

rayanmargham opened this issue Oct 14, 2024 · 9 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. C-gub Category: the reverse of a compiler bug is generally UB

Comments

@rayanmargham
Copy link

this is for a kernel, strict provience is enabled and has NOT been violated. when running on dev mode everything works well, but when running on release mode the compiler optimizes out the kernel scheduler entry function and has the CPU just hlt indefinitely,

I believe its replacing the RIP of the scheduler entry function when i pass it into my cpu_context struct with a similar function called hcf()

because the behavior is quite similar as i am just constantly looping and printing hello onto the screen. adding a variable does not change anything at all. the kernel is quite minimal and has only the components necessary to have a scheduler. the memory allocator has been extensively tested by an aml interrupter called uacpi, which i use via bindings called uacpi-rs
This is the github:
https://github.com/rayanmargham/NyauxKT

I expected to see this happen: I expected the scheduler entry function to run as expected and to print "hi"

Instead, this happened: the cpu indefinitely hlts

Meta

rustc --version --verbose:

rustc 1.83.0-nightly (3ae715c8c 2024-10-07)
binary: rustc
commit-hash: 3ae715c8c63f9aeac47cbf7d8d9dadb3fa32c638
commit-date: 2024-10-07
host: x86_64-unknown-linux-gnu
release: 1.83.0-nightly
LLVM version: 19.1.1

backtrace not possible due to being a no_std environment

@rayanmargham rayanmargham added the C-bug Category: This is a bug. label Oct 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 14, 2024
@jieyouxu jieyouxu added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Oct 15, 2024
@jwong101
Copy link
Contributor

jwong101 commented Oct 15, 2024

I think you need to use ptr::read_volatile, and wrap these requests in an UnsafeCell:

https://github.com/rayanmargham/NyauxKT/blob/415f4c39ddce202a5632bd5eca34f8dbd8899449/kernel/src/main.rs#L27-L29

#[used]
#[link_section = ".requests"]
static FRAMEBUFFER_REQUEST: FramebufferRequest = FramebufferRequest::new();

unsafe extern "C" fn kmain() -> ! {
    // All limine requests must also be referenced in a called function, otherwise they may be
    // removed by the linker.

    use NyauxKT::{elf::symbol, timers::init_timers};
    assert!(BASE_REVISION.is_supported());

    if let Some(framebuffer_response) = FRAMEBUFFER_REQUEST.get_response() {
    ...
}

Since there's no interior mutability, these get marked as const, so LLVM will assume that they have the same value that you provided in the initializer(which defaults to None in this case:

https://github.com/Jess4Tech/limine-protocol/blob/cc76e5829708a4af1dcbcec42ca74360a51d7ee6/src/requests/mod.rs#L45-L61

// the macro doesn't add any interior mutability to the generated struct
limine_request! {
    #[repr(C)]
    #[derive(Debug)]
    /// Request a framebuffer
    pub struct FramebufferRequest: [0x9d58_27dc_d881_dd75, 0xa314_8604_f6fa_b11b] {
        /// Response pointer
        pub response: Option<NonNull<FramebufferResponse>>,
    }
}

    pub unsafe fn get_response(&self) -> Option<&FramebufferResponse> {
        Some(self.response?.as_ref())
    }

@jwong101
Copy link
Contributor

Also, I don't think any optimizations will break this currently, but for:

https://github.com/rayanmargham/NyauxKT/blob/415f4c39ddce202a5632bd5eca34f8dbd8899449/kernel/src/sched/mod.rs#L127

    wrmsr(0xC0000101, Box::into_raw(pp).addr() as u64);

ptr::addr discards the provenance of the allocated Box which is the opposite of what you want. If you want to expose the provenance of a pointer, you can use a regular as cast or ptr::expose_provenance if you want to be explicit about it.

@saethlin
Copy link
Member

this is for a kernel, strict provience is enabled and has NOT been violated.

I truly do not know what you mean by this statement. Strict Provenance is a style of Rust, and the kind of code you are writing is one of the small domains that is well-known to not be able to use it. In addition, a quick search of the linked repo turns up some int-to-pointer casts so indeed, this repo is not using strict provenance. You have enabled the feature with the same name, which just gives you access to some nice pointer APIs and does nothing else.

@saethlin saethlin added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. labels Oct 15, 2024
@Noratrieb Noratrieb added C-gub Category: the reverse of a compiler bug is generally UB and removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Oct 15, 2024
@rayanmargham
Copy link
Author

rayanmargham commented Oct 15, 2024

Hello,

this is for a kernel, strict provience is enabled and has NOT been violated.

I truly do not know what you mean by this statement. Strict Provenance is a style of Rust, and the kind of code you are writing is one of the small domains that is well-known to not be able to use it. In addition, a quick search of the linked repo turns up some int-to-pointer casts so indeed, this repo is not using strict provenance. You have enabled the feature with the same name, which just gives you access to some nice pointer APIs and does nothing else.

removing all int to ptr casts by using ptr::expose_provenance() shows no change.

@rayanmargham
Copy link
Author

I do believe this may be a compiler bug.

I think you need to use ptr::read_volatile, and wrap these requests in an UnsafeCell:

https://github.com/rayanmargham/NyauxKT/blob/415f4c39ddce202a5632bd5eca34f8dbd8899449/kernel/src/main.rs#L27-L29

#[used]
#[link_section = ".requests"]
static FRAMEBUFFER_REQUEST: FramebufferRequest = FramebufferRequest::new();

unsafe extern "C" fn kmain() -> ! {
// All limine requests must also be referenced in a called function, otherwise they may be
// removed by the linker.

use NyauxKT::{elf::symbol, timers::init_timers};
assert!(BASE_REVISION.is_supported());

if let Some(framebuffer_response) = FRAMEBUFFER_REQUEST.get_response() {
...

}
Since there's no interior mutability, these get marked as const, so LLVM will assume that they have the same value that you provided in the initializer(which defaults to None in this case:

https://github.com/Jess4Tech/limine-protocol/blob/cc76e5829708a4af1dcbcec42ca74360a51d7ee6/src/requests/mod.rs#L45-L61

// the macro doesn't add any interior mutability to the generated struct
limine_request! {
#[repr(C)]
#[derive(Debug)]
/// Request a framebuffer
pub struct FramebufferRequest: [0x9d58_27dc_d881_dd75, 0xa314_8604_f6fa_b11b] {
/// Response pointer
pub response: Option<NonNull>,
}
}

pub unsafe fn get_response(&self) -> Option<&FramebufferResponse> {
    Some(self.response?.as_ref())
}

This is using the limine crate, please report this to limine-rs but this is not whats affecting my kernel atm

@jasondyoungberg
Copy link

I think you need to use ptr::read_volatile, and wrap these requests in an UnsafeCell:
[...]
Since there's no interior mutability, these get marked as const, so LLVM will assume that they have the same value that you provided in the initializer(which defaults to None in this case:

It does use interior mutability, and they are using the types correctly. If there is an issue it's on my end, but I don't see anything wrong.
https://github.com/jasondyoungberg/limine-rs/blob/f748ad3bc98a4d4a6e23fdef7cae6e5d03542f56/src/request.rs#L113

@rayanmargham
Copy link
Author

Not a compiler bug, was simply me being stupid and passing registers by value. thanks for the discussion :)

@jwong101
Copy link
Contributor

It does use interior mutability, and they are using the types correctly. If there is an issue it's on my end, but I don't see anything wrong. https://github.com/jasondyoungberg/limine-rs/blob/f748ad3bc98a4d4a6e23fdef7cae6e5d03542f56/src/request.rs#L113

Ah, I was looking at the wrong crate that was my mistake.

Regardless, I think you need to watch out for this:

https://github.com/rayanmargham/NyauxKT/blob/bc79cb96e3fe5dc950f2196bdcb696148f97b5cd/kernel/src/mem/vmm.rs#L409

I'm not sure if int2ptr of 0 has provenance and/or pick up exposed provenance(all other constant integers should be fine IIRC). Currently, LLVM will compile the following stores to that pointer as poison. In any case, you can just do new_guy.base as *mut u8 instead of (0 as *mut u8).with_addr(new_guy.base), and that should just work.

@saethlin
Copy link
Member

I'm not sure if int2ptr of 0 has provenance and/or pick up exposed provenance(all other constant integers should be fine IIRC). Currently, LLVM will compile the following stores to that pointer as poison. In any case, you can just do new_guy.base as *mut u8 instead of (0 as *mut u8).with_addr(new_guy.base), and that should just work.

I am pretty sure this is a miscompile in rustc caused by codegen implicitly const-propagating away the cast from integer to pointer, which is wrong. Filed #131741.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. C-gub Category: the reverse of a compiler bug is generally UB
Projects
None yet
Development

No branches or pull requests

7 participants