[Mesa-dev] [PATCH] clover: Refuse to create context with invalid properties

[Francisco Jerez]

Jan Vesely <jan.vesely at rutgers.edu> writes:

> the specs say that clCreateContext reutrns error
> "if platform value specified in properties is not a valid platform"
>
> The orignal approach fials if invalid valu other than NULL pointer is provided.
>
> Fixes piglit cl-api-create-context.
>
Honestly, I don't think this test makes much sense.  It's unreasonable
to expect that the CL will be able to catch any bad pointer you give it
as argument and fail gracefully.  The only reliable solution that comes
to my mind would be to build a global hash table for each CL object type
that keeps track of the valid objects that have been allocated.  That
seems like a lot of effort with the only purpose of finding out if the
user is doing something *very* stupid and very unlikely.

That said, we're already doing three forms of object validation: first,
the pointers provided by the user are compared against NULL; second, we
make sure that the dispatch table pointer is at the correct location in
memory; third, if the object is part of a non-trivial class hierarchy,
as is the case for events and memory objects, we use RTTI to make sure
that the object is of the expected type.  I don't think we want or need
more validation, it would probably be more useful to drop that test from
piglit.

Apparently nVidia's libOpenCL fails the test as well.

Thanks.

> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> ---
>  src/gallium/state_trackers/clover/api/context.cpp | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/context.cpp b/src/gallium/state_trackers/clover/api/context.cpp
> index 7b020a6..67adf8f 100644
> --- a/src/gallium/state_trackers/clover/api/context.cpp
> +++ b/src/gallium/state_trackers/clover/api/context.cpp
> @@ -34,14 +34,19 @@ clCreateContext(const cl_context_properties *d_props, cl_uint num_devs,
>                  void *user_data, cl_int *r_errcode) try {
>     auto props = obj<property_list_tag>(d_props);
>     auto devs = objs(d_devs, num_devs);
> +   cl_platform_id platform;
> +   cl_uint num_platforms;
>  
>     if (!pfn_notify && user_data)
>        throw error(CL_INVALID_VALUE);
> +   
> +   int ret = clGetPlatformIDs(1, &platform, &num_platforms);
> +   if (ret || !num_platforms)
> +      throw error(CL_INVALID_PLATFORM);
>  
>     for (auto &prop : props) {
> -      if (prop.first == CL_CONTEXT_PLATFORM)
> -         obj(prop.second.as<cl_platform_id>());
> -      else
> +      if (prop.first != CL_CONTEXT_PLATFORM ||
> +	  prop.second.as<cl_platform_id>() != platform)
>           throw error(CL_INVALID_PROPERTY);
>     }
>  
> -- 
> 1.8.3.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131021/42b46e9e/attachment-0001.pgp>