Backport Do not feed &"" to D3DCompile (#5812) to 0.20 (#5831)

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: Jubilee <46493976+workingjubilee@users.noreply.github.com>
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Co-authored-by: Brezak <bezak.adam@proton.me>
This commit is contained in:
TÖRÖK Attila 2024-06-18 21:46:36 +02:00 committed by GitHub
parent 098c56ff7a
commit 32d21c8e4f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 16 additions and 10 deletions

View File

@ -263,12 +263,7 @@ impl super::Device {
.as_ref() .as_ref()
.map_err(|e| crate::PipelineError::Linkage(stage_bit, format!("{e}")))?; .map_err(|e| crate::PipelineError::Linkage(stage_bit, format!("{e}")))?;
let source_name = stage let source_name = stage.module.raw_name.as_deref();
.module
.raw_name
.as_ref()
.and_then(|cstr| cstr.to_str().ok())
.unwrap_or_default();
// Compile with DXC if available, otherwise fall back to FXC // Compile with DXC if available, otherwise fall back to FXC
let (result, log_level) = if let Some(ref dxc_container) = self.dxc_container { let (result, log_level) = if let Some(ref dxc_container) = self.dxc_container {

View File

@ -1,3 +1,4 @@
use std::ffi::CStr;
use std::ptr; use std::ptr;
pub(super) use dxc::{compile_dxc, get_dxc_container, DxcContainer}; 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( pub(super) fn compile_fxc(
device: &super::Device, device: &super::Device,
source: &str, source: &str,
source_name: &str, source_name: Option<&CStr>,
raw_ep: &std::ffi::CString, raw_ep: &std::ffi::CString,
stage_bit: wgt::ShaderStages, stage_bit: wgt::ShaderStages,
full_stage: String, full_stage: String,
@ -32,13 +33,17 @@ pub(super) fn compile_fxc(
{ {
compile_flags |= d3dcompiler::D3DCOMPILE_DEBUG | d3dcompiler::D3DCOMPILE_SKIP_OPTIMIZATION; compile_flags |= d3dcompiler::D3DCOMPILE_DEBUG | d3dcompiler::D3DCOMPILE_SKIP_OPTIMIZATION;
} }
// If no name has been set, D3DCompile wants the null pointer.
let source_name = source_name.map(|cstr| cstr.as_ptr()).unwrap_or(ptr::null());
let mut error = d3d12::Blob::null(); let mut error = d3d12::Blob::null();
let hr = unsafe { let hr = unsafe {
profiling::scope!("d3dcompiler::D3DCompile"); profiling::scope!("d3dcompiler::D3DCompile");
d3dcompiler::D3DCompile( d3dcompiler::D3DCompile(
source.as_ptr().cast(), source.as_ptr().cast(),
source.len(), source.len(),
source_name.as_ptr().cast(), source_name.cast(),
ptr::null(), ptr::null(),
ptr::null_mut(), ptr::null_mut(),
raw_ep.as_ptr(), raw_ep.as_ptr(),
@ -78,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. // 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")] #[cfg(feature = "dxc_shader_compiler")]
mod dxc { mod dxc {
use std::ffi::CStr;
use std::path::PathBuf; use std::path::PathBuf;
// Destructor order should be fine since _dxil and _dxc don't rely on each other. // Destructor order should be fine since _dxil and _dxc don't rely on each other.
@ -132,7 +138,7 @@ mod dxc {
pub(crate) fn compile_dxc( pub(crate) fn compile_dxc(
device: &crate::dx12::Device, device: &crate::dx12::Device,
source: &str, source: &str,
source_name: &str, source_name: Option<&CStr>,
raw_ep: &str, raw_ep: &str,
stage_bit: wgt::ShaderStages, stage_bit: wgt::ShaderStages,
full_stage: String, full_stage: String,
@ -166,6 +172,10 @@ mod dxc {
Err(e) => return (Err(e), log::Level::Error), 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( let compiled = dxc_container.compiler.compile(
&blob, &blob,
source_name, source_name,
@ -263,6 +273,7 @@ mod dxc {
// These are stubs for when the `dxc_shader_compiler` feature is disabled. // These are stubs for when the `dxc_shader_compiler` feature is disabled.
#[cfg(not(feature = "dxc_shader_compiler"))] #[cfg(not(feature = "dxc_shader_compiler"))]
mod dxc { mod dxc {
use std::ffi::CStr;
use std::path::PathBuf; use std::path::PathBuf;
pub(crate) struct DxcContainer {} pub(crate) struct DxcContainer {}
@ -280,7 +291,7 @@ mod dxc {
pub(crate) fn compile_dxc( pub(crate) fn compile_dxc(
_device: &crate::dx12::Device, _device: &crate::dx12::Device,
_source: &str, _source: &str,
_source_name: &str, _source_name: Option<&CStr>,
_raw_ep: &str, _raw_ep: &str,
_stage_bit: wgt::ShaderStages, _stage_bit: wgt::ShaderStages,
_full_stage: String, _full_stage: String,