rusticl/program: store log as a CString

Reviewed-by: @LingMan
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/41905>

src/gallium/frontends/rusticl/api/program.rs

@@ -100,7 +100,7 @@ unsafe impl CLInfoObj<cl_program_build_info, cl_device_id> for cl_program {
         match q {
             CL_PROGRAM_BINARY_TYPE => v.write::<cl_program_binary_type>(prog.bin_type(dev)),
             CL_PROGRAM_BUILD_GLOBAL_VARIABLE_TOTAL_SIZE => v.write::<usize>(0),
-            CL_PROGRAM_BUILD_LOG => v.write::<&str>(&prog.log(dev)),
+            CL_PROGRAM_BUILD_LOG => v.write::<&CStr>(&prog.log(dev)),
             CL_PROGRAM_BUILD_OPTIONS => v.write::<&str>(&prog.options(dev)),
             CL_PROGRAM_BUILD_STATUS => v.write::<cl_build_status>(prog.status(dev)),
             // CL_INVALID_VALUE if param_name is not one of the supported values

src/gallium/frontends/rusticl/core/program.rs

@@ -14,6 +14,8 @@ use mesa_rust::compiler::clc::*;
 use mesa_rust::compiler::nir::*;
 use mesa_rust::util::disk_cache::*;
 use mesa_rust_gen::*;
+use mesa_rust_util::string::CStringExt;
+use mesa_rust_util::string::Join;
 use rusticl_llvm_gen::*;
 use rusticl_opencl_gen::*;
 
@@ -142,7 +144,7 @@ impl ProgramBuild {
                     convert_spirv_to_nir(build, kernel_name, &args, &mut self.spec_constants, dev)
                 else {
                     build.status = CL_BUILD_ERROR;
-                    build.log = "Internal compilation error".to_owned();
+                    build.log = c"Internal compilation error".to_owned();
                     return;
                 };
                 kernel_info_set.insert(build_result.kernel_info);
@@ -195,7 +197,7 @@ pub struct DeviceProgramBuild {
     spirv: Option<spirv::SPIRVBin>,
     status: cl_build_status,
     options: String,
-    log: String,
+    log: CString,
     bin_type: cl_program_binary_type,
     pub kernels: HashMap<CString, Arc<NirKernelBuilds>>,
 }
@@ -266,7 +268,7 @@ impl DeviceProgramBuild {
 
         if let Some(log) = log {
             for line in log {
-                eprintln!("{}", line);
+                eprintln!("{line:?}");
             }
         };
 
@@ -504,7 +506,7 @@ impl Program {
         self.build_info().dev_build(dev).status
     }
 
-    pub fn log(&self, dev: &Device) -> String {
+    pub fn log(&self, dev: &Device) -> CString {
         self.build_info().dev_build(dev).log.clone()
     }
 
@@ -646,7 +648,7 @@ impl Program {
         let (spirv, log) = match &self.src {
             ProgramSourceType::Il(spirv) => {
                 if Platform::dbg().allow_invalid_spirv {
-                    (Some(spirv.clone()), String::new())
+                    (Some(spirv.clone()), CString::default())
                 } else {
                     spirv.clone_on_validate(&val_options)
                 }
@@ -692,7 +694,7 @@ impl Program {
                 if Platform::dbg().validate_spirv {
                     if let Some(spirv) = spirv {
                         let (res, spirv_msgs) = spirv.validate(&val_options);
-                        (res.then_some(spirv), format!("{}\n{}", msgs, spirv_msgs))
+                        (res.then_some(spirv), [msgs, spirv_msgs].join(c"\n"))
                     } else {
                         (None, msgs)
                     }
@@ -854,7 +856,7 @@ impl Program {
             if let Some(spirv) = spirv {
                 let val_options = clc_validator_options(device);
                 let (res, spirv_msgs) = spirv.validate(&val_options);
-                (res.then_some(spirv), format!("{}\n{}", log, spirv_msgs))
+                (res.then_some(spirv), [log, spirv_msgs].join(c"\n"))
             } else {
                 (None, log)
             }
@@ -863,7 +865,7 @@ impl Program {
         };
 
         build.spirv = spirv;
-        build.log.push_str(&log);
+        build.log.push_cstr(&log);
 
         if build.spirv.is_some() {
             build.status = CL_BUILD_SUCCESS as cl_build_status;
@@ -942,7 +944,7 @@ fn debug_logging(p: &Program, devs: &[&Device]) {
         for dev in devs {
             let msg = p.log(dev);
             if !msg.is_empty() {
-                eprintln!("{}", msg);
+                eprintln!("{msg:?}");
             }
         }
     }

src/gallium/frontends/rusticl/mesa/compiler/clc/spirv.rs

@@ -58,9 +58,15 @@ impl Debug for CLCHeader<'_> {
 }
 
 unsafe fn callback_impl(data: *mut c_void, msg: *const c_char) {
-    let data = data as *mut Vec<String>;
+    if msg.is_null() {
+        return;
+    }
+
+    let data = data as *mut Vec<CString>;
     let msgs = unsafe { data.as_mut() }.unwrap();
-    msgs.push(c_string_to_string(msg));
+
+    // SAFETY: msg is a valid C string.
+    msgs.push(unsafe { CStr::from_ptr(msg) }.to_owned());
 }
 
 unsafe extern "C" fn spirv_msg_callback(data: *mut c_void, msg: *const c_char) {
@@ -82,7 +88,7 @@ unsafe extern "C" fn spirv_to_nir_msg_callback(
     }
 }
 
-fn create_clc_logger(msgs: &mut Vec<String>) -> clc_logger {
+fn create_clc_logger(msgs: &mut Vec<CString>) -> clc_logger {
     clc_logger {
         priv_: ptr::from_mut(msgs).cast(),
         error: Some(spirv_msg_callback),
@@ -105,7 +111,7 @@ impl SPIRVBin {
         features: clc_optional_features,
         spirv_extensions: &[&CStr],
         address_bits: u32,
-    ) -> (Option<Self>, String) {
+    ) -> (Option<Self>, CString) {
         let mut hash_key = None;
         let has_includes = args.iter().any(|a| a.as_bytes()[0..2] == *b"-I");
 
@@ -130,7 +136,7 @@ impl SPIRVBin {
 
                 let mut key = cache.gen_key(&key);
                 if let Some(data) = cache.get(&mut key) {
-                    return (Some(Self::from_bin(&data)), String::from(""));
+                    return (Some(Self::from_bin(&data)), CString::default());
                 }
 
                 hash_key = Some(key);
@@ -163,7 +169,7 @@ impl SPIRVBin {
             c_compatible: false,
             address_bits: address_bits,
         };
-        let mut msgs: Vec<String> = Vec::new();
+        let mut msgs = Vec::new();
         let logger = create_clc_logger(&mut msgs);
         let mut out = clc_binary::default();
 
@@ -187,11 +193,11 @@ impl SPIRVBin {
             None
         };
 
-        (res, msgs.join(""))
+        (res, msgs.join(c""))
     }
 
     // TODO cache linking, parsing is around 25% of link time
-    pub fn link(spirvs: &[&SPIRVBin], library: bool) -> (Option<Self>, String) {
+    pub fn link(spirvs: &[&SPIRVBin], library: bool) -> (Option<Self>, CString) {
         let bins: Vec<_> = spirvs.iter().map(|s| ptr::from_ref(&s.spirv)).collect();
 
         let linker_args = clc_linker_args {
@@ -200,7 +206,7 @@ impl SPIRVBin {
             create_library: library as u32,
         };
 
-        let mut msgs: Vec<String> = Vec::new();
+        let mut msgs = Vec::new();
         let logger = create_clc_logger(&mut msgs);
 
         let mut out = clc_binary::default();
@@ -218,18 +224,18 @@ impl SPIRVBin {
             spirv: out,
             info: info,
         });
-        (res, msgs.join(""))
+        (res, msgs.join(c""))
     }
 
-    pub fn validate(&self, options: &clc_validator_options) -> (bool, String) {
+    pub fn validate(&self, options: &clc_validator_options) -> (bool, CString) {
-        let mut msgs: Vec<String> = Vec::new();
+        let mut msgs = Vec::new();
         let logger = create_clc_logger(&mut msgs);
         let res = unsafe { clc_validate_spirv(&self.spirv, &logger, options) };
 
-        (res, msgs.join(""))
+        (res, msgs.join(c""))
     }
 
-    pub fn clone_on_validate(&self, options: &clc_validator_options) -> (Option<Self>, String) {
+    pub fn clone_on_validate(&self, options: &clc_validator_options) -> (Option<Self>, CString) {
         let (res, msgs) = self.validate(options);
         (res.then(|| self.clone()), msgs)
     }
@@ -289,7 +295,7 @@ impl SPIRVBin {
         library: bool,
         clc_shader: *const nir_shader,
         options: SPIRVToNirOptions,
-        log: Option<&mut Vec<String>>,
+        log: Option<&mut Vec<CString>>,
     ) -> spirv_to_nir_options {
         let global_addr_format;
         let offset_addr_format;
@@ -331,7 +337,7 @@ impl SPIRVBin {
         spirv_to_nir_options: SPIRVToNirOptions,
         libclc: &NirShader,
         spec_constants: &mut nir_spirv_specialization,
-        log: Option<&mut Vec<String>>,
+        log: Option<&mut Vec<CString>>,
     ) -> Option<NirShader> {
         let spirv_options =
             Self::get_spirv_options(false, libclc.get_nir(), spirv_to_nir_options, log);