Linux: DXC BSTR now resembles its Windows counterpart with explicit size

Since microsoft/DirectXShaderCompiler#3250
Traverse-Research · Nov 25, 2020 · 1c80cf8 · 1c80cf8
1 parent fc0a483
commit 1c80cf8
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 24 deletions.
diff --git a/src/os.rs b/src/os.rs
@@ -6,13 +6,14 @@ mod os_defs {
     };
 
     pub use winapi::um::combaseapi::CoTaskMemFree;
-    pub use winapi::um::oleauto::SysFreeString;
+    pub use winapi::um::oleauto::{SysFreeString, SysStringLen};
 }
 
 #[cfg(not(windows))]
 mod os_defs {
     pub type CHAR = i8;
-    pub type WCHAR = u32;
+    pub type UINT = u32;
+    pub type WCHAR = widestring::WideChar;
     pub type OLECHAR = WCHAR;
     pub type LPSTR = *mut CHAR;
     pub type LPWSTR = *mut WCHAR;
@@ -25,13 +26,35 @@ mod os_defs {
     #[allow(non_snake_case)]
     pub unsafe fn CoTaskMemFree(p: *mut libc::c_void) {
         // https://github.com/microsoft/DirectXShaderCompiler/blob/a8d9780046cb64a1cea842fa6fc28a250e3e2c09/include/dxc/Support/WinAdapter.h#L46
-        libc::free(p)
+        if !p.is_null() {
+            libc::free(p)
+        }
     }
 
     #[allow(non_snake_case)]
     pub unsafe fn SysFreeString(p: BSTR) {
-        // https://github.com/microsoft/DirectXShaderCompiler/blob/a8d9780046cb64a1cea842fa6fc28a250e3e2c09/include/dxc/Support/WinAdapter.h#L48-L50
-        libc::free(p as _)
+        // https://github.com/microsoft/DirectXShaderCompiler/blob/2ade6f84d6b95bfd96eec1d6d15e3aa3b519d180/lib/DxcSupport/WinAdapter.cpp#L73-L76
+        if !p.is_null() {
+            libc::free((p as *mut UINT).offset(-1) as *mut _)
+        }
+    }
+
+    /// Returns the size of `p` in bytes, without terminating NULL character
+    #[allow(non_snake_case)]
+    pub unsafe fn SysStringByteLen(p: BSTR) -> UINT {
+        // The first four bytes before the pointer contain the length prefix:
+        // https://docs.microsoft.com/en-us/previous-versions/windows/desktop/automat/bstr
+        if p.is_null() {
+            0
+        } else {
+            *(p as *const UINT).offset(-1)
+        }
+    }
+
+    /// Returns the size of `p` in characters, without terminating NULL character
+    #[allow(non_snake_case)]
+    pub unsafe fn SysStringLen(p: BSTR) -> UINT {
+        SysStringByteLen(p) / std::mem::size_of::<OLECHAR>() as UINT
     }
 }
 

diff --git a/src/utils.rs b/src/utils.rs
@@ -1,10 +1,7 @@
-use crate::os::{SysFreeString, BSTR, HRESULT, LPSTR, LPWSTR, WCHAR};
+use crate::os::{SysFreeString, SysStringLen, BSTR, HRESULT, LPSTR, LPWSTR, WCHAR};
 use crate::wrapper::*;
 use thiserror::Error;
 
-#[cfg(windows)]
-use winapi::um::oleauto::SysStringLen;
-
 pub(crate) fn to_wide(msg: &str) -> Vec<WCHAR> {
     widestring::WideCString::from_str(msg).unwrap().into_vec()
 }
@@ -17,7 +14,6 @@ pub(crate) fn from_wide(wide: LPWSTR) -> String {
     }
 }
 
-#[cfg(windows)]
 pub(crate) fn from_bstr(string: BSTR) -> String {
     unsafe {
         let len = SysStringLen(string) as usize;
@@ -31,20 +27,6 @@ pub(crate) fn from_bstr(string: BSTR) -> String {
     }
 }
 
-#[cfg(not(windows))]
-pub(crate) fn from_bstr(string: BSTR) -> String {
-    // TODO (Marijn): This does NOT cover embedded NULLs
-
-    // BSTR contains its size in the four bytes preceding the pointer, in order to contain NULL bytes:
-    // https://docs.microsoft.com/en-us/previous-versions/windows/desktop/automat/bstr
-    // DXC on non-Windows does not adhere to that and simply allocates a buffer without prepending the size:
-    // https://github.com/microsoft/DirectXShaderCompiler/blob/a8d9780046cb64a1cea842fa6fc28a250e3e2c09/include/dxc/Support/WinAdapter.h#L49-L50
-    let result = from_wide(string as LPWSTR);
-
-    unsafe { SysFreeString(string) };
-    result
-}
-
 pub(crate) fn from_lpstr(string: LPSTR) -> String {
     unsafe {
         let len = (0..).take_while(|&i| *string.offset(i) != 0).count();