Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CL_MEM_USE_HOST_PTR can result in undefined behaviour #52

Closed
awused opened this issue Jul 22, 2022 · 4 comments
Closed

CL_MEM_USE_HOST_PTR can result in undefined behaviour #52

awused opened this issue Jul 22, 2022 · 4 comments
Assignees
Labels
invalid This doesn't seem right

Comments

@awused
Copy link

awused commented Jul 22, 2022

CL_MEM_USE_HOST_PTR is useful for performance but has additional requirements that need to be upheld by the program for it to be used safely. Looking at the example in https://github.com/kenba/opencl3/blob/main/examples/basic.rs, switching x to CL_MEM_USE_HOST_PTR like this will result in an error from OpenCL, which is the best case.

    let mut x = Buffer::<cl_float>::create(
        &context,
        CL_MEM_READ_ONLY | CL_MEM_USE_HOST_PTR,
        ARRAY_SIZE,
        ptr::null_mut(),
    )?;

The allocation pointed to by the host pointer may be too small, unaligned, or of the wrong type, or just not live long enough. There are also a set of alignment rules that need to be checked for this to be safe. See https://registry.khronos.org/OpenCL/sdk/2.0/docs/man/xhtml/dataTypes.html

    let img = image::open("test.png").unwrap().into_rgba8();
    let format = cl_image_format {
        image_channel_order: CL_RGBA,
        image_channel_data_type: CL_UNSIGNED_INT8,
    };
    let desc = cl_image_desc {
        image_type: CL_MEM_OBJECT_IMAGE2D,
        image_width: img.width() as _,
        image_height: img.height() as _,
        image_depth: 1,
        image_array_size: 1,
        image_row_pitch: 0,
        image_slice_pitch: 0,
        num_mip_levels: 0,
        num_samples: 0,
        buffer: ptr::null_mut(),
    };
    let mut img = img.into_vec();
    let unaligned_image = Image::create(
        &context,
        CL_MEM_READ_ONLY | CL_MEM_USE_HOST_PTR,
        &format,
        &desc,
        // This does not have the proper alignment and may cause undefined behaviour or not work.
        ptr::addr_of_mut!(img[1]) as _,
    )?;
    // Can drop img and still use _unaligned_image
    drop(img);
    let _still_alive = unaligned_image;
@kenba kenba self-assigned this Jul 22, 2022
@kenba kenba added the invalid This doesn't seem right label Jul 23, 2022
@kenba
Copy link
Owner

kenba commented Jul 23, 2022

This is an OpenCL issue, it is not a bug that has been introduced by this library.
Please read and understand the OpenCL manual before trying to use advanced features like CL_MEM_USE_HOST_PTR.

@kenba kenba closed this as completed Jul 23, 2022
@awused
Copy link
Author

awused commented Jul 23, 2022

I do understand the OpenCL documentation around CL_MEM_USE_HOST_PTR which is why I was so surprised that this code didn't require unsafe anywhere on my part. I know the bug hasn't been introduced, per se, by this library, but I would expect it to require a use of unsafe somewhere.

@kenba
Copy link
Owner

kenba commented Aug 9, 2022

@awused is right. Any OpenCL functions that can lead to undefined behaviour in Rust should be declared unsafe in this library.

Note: they should also be declared unsafe in the cl3 library.

@kenba
Copy link
Owner

kenba commented Sep 10, 2022

Changes published to crates.io.

@kenba kenba closed this as completed Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants