From a6d3d13fe22b778e3c662b5c8e7cef7d0ad1e5f7 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 13 Jun 2024 19:30:07 -0700 Subject: [PATCH 1/2] Do not feed &"" to D3DCompile A recent change by rustc, now in 1.79-stable, makes empty str constants point to the same location: 0x01. This is an optimization of sorts, not stable behavior. Code must not rely on constants having stable addresses nor should it pass &"" to APIs expecting CStrs or NULL addresses. D3DCompile will segfault if you give it such a pointer, or worse: read random garbage addresses! Pass the NULL pointer to D3DCompile if wgpu lacks a decent CString. refs: - https://learn.microsoft.com/en-us/windows/win32/api/d3dcompiler/nf-d3dcompiler-d3dcompile Co-authored-by: Jan Hohenheim Co-authored-by: Brezak --- wgpu-hal/src/dx12/shader_compilation.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/wgpu-hal/src/dx12/shader_compilation.rs b/wgpu-hal/src/dx12/shader_compilation.rs index 288fc24745..b4afe492b7 100644 --- a/wgpu-hal/src/dx12/shader_compilation.rs +++ b/wgpu-hal/src/dx12/shader_compilation.rs @@ -32,13 +32,21 @@ pub(super) fn compile_fxc( { compile_flags |= d3dcompiler::D3DCOMPILE_DEBUG | d3dcompiler::D3DCOMPILE_SKIP_OPTIMIZATION; } + + // If no name has been set, D3DCompile wants the null pointer. + let source_name = if source_name.is_empty() { + ptr::null() + } else { + source_name.as_ptr() + }; + let mut error = d3d12::Blob::null(); let hr = unsafe { profiling::scope!("d3dcompiler::D3DCompile"); d3dcompiler::D3DCompile( source.as_ptr().cast(), source.len(), - source_name.as_ptr().cast(), + source_name.cast(), ptr::null(), ptr::null_mut(), raw_ep.as_ptr(), From b0f603f2f37f2f9704ad4f5e2b0e595f714d30d6 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 13 Jun 2024 23:57:57 -0700 Subject: [PATCH 2/2] Alter compile_{d,f}xc to take `Option<&CStr>` --- wgpu-hal/src/dx12/device.rs | 7 +------ wgpu-hal/src/dx12/shader_compilation.rs | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index 5625dfca3b..81674ea8b1 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -259,12 +259,7 @@ impl super::Device { .as_ref() .map_err(|e| crate::PipelineError::Linkage(stage_bit, format!("{e}")))?; - let source_name = stage - .module - .raw_name - .as_ref() - .and_then(|cstr| cstr.to_str().ok()) - .unwrap_or_default(); + let source_name = stage.module.raw_name.as_deref(); // Compile with DXC if available, otherwise fall back to FXC let (result, log_level) = if let Some(ref dxc_container) = self.dxc_container { diff --git a/wgpu-hal/src/dx12/shader_compilation.rs b/wgpu-hal/src/dx12/shader_compilation.rs index b4afe492b7..4eb8801990 100644 --- a/wgpu-hal/src/dx12/shader_compilation.rs +++ b/wgpu-hal/src/dx12/shader_compilation.rs @@ -1,3 +1,4 @@ +use std::ffi::CStr; use std::ptr; pub(super) use dxc::{compile_dxc, get_dxc_container, DxcContainer}; @@ -14,7 +15,7 @@ use crate::auxil::dxgi::result::HResult; pub(super) fn compile_fxc( device: &super::Device, source: &str, - source_name: &str, + source_name: Option<&CStr>, raw_ep: &std::ffi::CString, stage_bit: wgt::ShaderStages, full_stage: String, @@ -34,11 +35,7 @@ pub(super) fn compile_fxc( } // If no name has been set, D3DCompile wants the null pointer. - let source_name = if source_name.is_empty() { - ptr::null() - } else { - source_name.as_ptr() - }; + let source_name = source_name.map(|cstr| cstr.as_ptr()).unwrap_or(ptr::null()); let mut error = d3d12::Blob::null(); let hr = unsafe { @@ -86,6 +83,7 @@ pub(super) fn compile_fxc( // The Dxc implementation is behind a feature flag so that users who don't want to use dxc can disable the feature. #[cfg(feature = "dxc_shader_compiler")] mod dxc { + use std::ffi::CStr; use std::path::PathBuf; // Destructor order should be fine since _dxil and _dxc don't rely on each other. @@ -140,7 +138,7 @@ mod dxc { pub(crate) fn compile_dxc( device: &crate::dx12::Device, source: &str, - source_name: &str, + source_name: Option<&CStr>, raw_ep: &str, stage_bit: wgt::ShaderStages, full_stage: String, @@ -174,6 +172,10 @@ mod dxc { Err(e) => return (Err(e), log::Level::Error), }; + let source_name = source_name + .and_then(|cstr| cstr.to_str().ok()) + .unwrap_or(""); + let compiled = dxc_container.compiler.compile( &blob, source_name, @@ -271,6 +273,7 @@ mod dxc { // These are stubs for when the `dxc_shader_compiler` feature is disabled. #[cfg(not(feature = "dxc_shader_compiler"))] mod dxc { + use std::ffi::CStr; use std::path::PathBuf; pub(crate) struct DxcContainer {} @@ -288,7 +291,7 @@ mod dxc { pub(crate) fn compile_dxc( _device: &crate::dx12::Device, _source: &str, - _source_name: &str, + _source_name: Option<&CStr>, _raw_ep: &str, _stage_bit: wgt::ShaderStages, _full_stage: String,