non UTF8 encoded string in enumerate_instance_extension_properties function

[GummyGun]

There is a bug in the method enumerate_instance_extension_properties() where sometimes the last character in the field ExtensionProperties.extension_name is not properly encoded and may hold non-valid utf8 char.

[GummyGun]

this bug to my findings is only present in the extension VK_EXT_direct_mode_display

[MarijnS95]

Can you share the u8 data that this array contains, i.e. the way that you ran into this issue?

[MarijnS95]

After checking the ICD on my ArchLinux machine with the following snippet:

            for p in entry.enumerate_instance_extension_properties(None).unwrap() {
                let n = ::std::ffi::CStr::from_ptr(p.extension_name.as_ptr());

                if n.to_string_lossy() == "VK_EXT_direct_mode_display" {
                    dbg!(p.extension_name);
                }
            }

I get a sensible result out of the array:

[ash-examples/src/lib.rs:229] p.extension_name = [
    86,
    75,
    95,
    69,
    88,
    84,
    95,
    100,
    105,
    114,
    101,
    99,
    116,
    95,
    109,
    111,
    100,
    101,
    95,
    100,
    105,
    115,
    112,
    108,
    97,
    121,
    0,
    ...,
    0,
]

Note that Ash initially allocates uninitialized memory for these structs, and it's up to the ICD to optionally zero out the whole string buffer after the first NUL character. However, that might even be optional given the description at https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkExtensionProperties.html:

extensionName is an array of VK_MAX_EXTENSION_NAME_SIZE char containing a null-terminated UTF-8 string which is the name of the extension.

Typically (as far as I'm aware) implementations should stop reading data after the first NUL byte/character.

Are you sure that your test case isn't accidentally reading beyond the NUL character?

EDIT: For completeness #746 and its use of CStr::from_bytes_until_nul() might be useful to you. I'm willing to convert the debug APIs to it - and maybe even autogenerate CStr getters on structs utilizing it - once our MSRV allows for it.
You can use it in client code already though.

[GummyGun]

I should use the CStr since it saves me a unsafe block and over all is a better practice and I'm pretty sure CStr would solve the problem. Either way I think it is worth fixing the this awkward behavior.

let extension_list = entry.enumerate_instance_extension_properties(None).unwrap();
 
for extension in &extension_list {
    println!("{:?}", extension);
    println!("{:?}", extension.extension_name);
    if extension.extension_name[255] != 0 {
        panic!("found nonValid utf8");
    }
}

The output of the code above:

ExtensionProperties { extension_name: "VK_EXT_direct_mode_display", spec_version: 1 }
[86, 75, 95, 69, 88, 84, 95, 100, 105, 114, 101, 99, 116, 95, 109, 111, 100, 101, 95, 100, 105, 115, 112, 108, 97, 121, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 58]

This error is not present every run but I find it is pops out about half of the runs I do

[GummyGun]

I will run its equivalent interfacing directly to the vulkan in C to see if I get a different behavior

[MarijnS95]

Either way I think it is worth fixing the this awkward behavior.

What do you want to "fix"? There's no weird behaviour here. The driver provides a valid UTF-8 string and a NUL terminator. There is no requirement to set any bytes to NUL after that, and it seems the driver/ICD is off by one byte when trying that here (or Rust's uninitialized vector allocation happened to do that).

If you're testing on C, don't forget to set the array to uninitialized bytes first, to match ash behaviour.

[MarijnS95]

@GummyGun did you end up checking whether your driver only writes a single NUL-terminator, or does it zero the entire array minus the last item (by accident)?

I should use the CStr since it saves me a unsafe block and over all is a better practice and I'm pretty sure CStr would solve the problem

On that note we've added CStr getters and setters to structs, to make this easier in the future: #831