[Mesa-dev] [PATCH v2 04/22] clover: Disallow creating libraries from other libraries

[Francisco Jerez]

Pierre Moreau <pierre.morrow at free.fr> writes:

> If creating a library, do not allow non-compiled object in it, as
> executables are not allowed, and libraries would make it really hard to
> enforce the "-enable-link-options" flag.
>
> Signed-off-by: Pierre Moreau <pierre.morrow at free.fr>
> ---
>  src/gallium/state_trackers/clover/api/program.cpp | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/program.cpp b/src/gallium/state_trackers/clover/api/program.cpp
> index 6044179587..8f0b103a4d 100644
> --- a/src/gallium/state_trackers/clover/api/program.cpp
> +++ b/src/gallium/state_trackers/clover/api/program.cpp
> @@ -251,9 +251,13 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
>  namespace {
>     ref_vector<device>
>     validate_link_devices(const ref_vector<program> &progs,
> -                         const ref_vector<device> &all_devs) {
> +                         const ref_vector<device> &all_devs,
> +                         const std::string &opts) {
>        std::vector<device *> devs;
>  
> +      const std::string flag = "-create-library";
> +      const bool create_library = opts.find("-create-library") != std::string::npos;
> +
>        for (auto &dev : all_devs) {
>           const auto has_binary = [&](const program &prog) {
>              const auto t = prog.build(dev).binary_type();
> @@ -261,10 +265,19 @@ namespace {
>                     t == CL_PROGRAM_BINARY_TYPE_LIBRARY;
>           };
>  
> +         // If creating a library, do not allow non-compiled object in it, as
> +         // executables are not allowed, and libraries would make it really
> +         // hard to enforce the "-enable-link-options".
> +         if (create_library && any_of([&](const program &prog) {
> +                  const auto t = prog.build(dev).binary_type();
> +                  return t != CL_PROGRAM_BINARY_TYPE_COMPILED_OBJECT;
> +               }, progs))
> +            throw error(CL_INVALID_OPERATION);
> +

Do you have any spec quote justifying this?  "Hard" is hardly a
justification for emitting an unexpected API error ;).  If doing such a
thing would rely on unimplemented behavior somewhere else it would
probably be cleaner to assert-fail wherever an implementation is missing
rather than returning an API error under conditions not expected by the
application.

>           // According to the CL 1.2 spec, when "all programs specified [..]
>           // contain a compiled binary or library for the device [..] a link is
>           // performed",
> -         if (all_of(has_binary, progs))
> +         else if (all_of(has_binary, progs))
>              devs.push_back(&dev);
>  
>           // otherwise if "none of the programs contain a compiled binary or
> @@ -290,7 +303,7 @@ clLinkProgram(cl_context d_ctx, cl_uint num_devs, const cl_device_id *d_devs,
>     auto prog = create<program>(ctx);
>     auto devs = validate_link_devices(progs,
>                                       (d_devs ? objs(d_devs, num_devs) :
> -                                      ref_vector<device>(ctx.devices())));
> +                                      ref_vector<device>(ctx.devices())), opts);
>  
>     validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
>  
> -- 
> 2.16.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180123/29ce336b/attachment.sig>