[Mesa-dev] [PATCH] radv: do not segfault when the SPIRV entry point is NULL

[Jason Ekstrand]

It's not the job of the driver to check these things.  Developers should be
running with validation layers enabled which are supposed to catch this.
There are so many other ways things can go wrong if the user provides bad
input and this only fixes one tiny one.

On Mon, Sep 17, 2018 at 4:18 AM Samuel Pitoiset <samuel.pitoiset at gmail.com>
wrote:

> And return a better error value.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107954
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/amd/vulkan/radv_pipeline.c | 32 +++++++++++++++++++++++++-------
>  src/amd/vulkan/radv_shader.c   |  3 +++
>  2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_pipeline.c
> b/src/amd/vulkan/radv_pipeline.c
> index ae269c32c49..f1e3c6f9fe9 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -1983,8 +1983,8 @@ merge_tess_info(struct shader_info *tes_info,
>         tes_info->tess.point_mode |= tcs_info->tess.point_mode;
>  }
>
> -static
> -void radv_create_shaders(struct radv_pipeline *pipeline,
> +static VkResult
> +radv_create_shaders(struct radv_pipeline *pipeline,
>                           struct radv_device *device,
>                           struct radv_pipeline_cache *cache,
>                           struct radv_pipeline_key key,
> @@ -2023,7 +2023,7 @@ void radv_create_shaders(struct radv_pipeline
> *pipeline,
>
>         if (radv_create_shader_variants_from_pipeline_cache(device, cache,
> hash, pipeline->shaders) &&
>             (!modules[MESA_SHADER_GEOMETRY] || pipeline->gs_copy_shader)) {
> -               return;
> +               return VK_SUCCESS;
>         }
>
>         if (!modules[MESA_SHADER_FRAGMENT] &&
> !modules[MESA_SHADER_COMPUTE]) {
> @@ -2056,6 +2056,9 @@ void radv_create_shaders(struct radv_pipeline
> *pipeline,
>                                                     stage ?
> stage->pSpecializationInfo : NULL,
>                                                     flags);
>
> +               if (!nir[i])
> +                       goto fail;
> +
>                 /* We don't want to alter meta shaders IR directly so
> clone it
>                  * first.
>                  */
> @@ -2202,6 +2205,14 @@ void radv_create_shaders(struct radv_pipeline
> *pipeline,
>
>         if (fs_m.nir)
>                 ralloc_free(fs_m.nir);
> +
> +       return VK_SUCCESS;
> +
> +fail:
> +       for (int i = 0; i < MESA_SHADER_STAGES; ++i)
> +               ralloc_free(nir[i]);
> +       ralloc_free(fs_m.nir);
> +       return VK_ERROR_INVALID_SHADER_NV;
>  }
>
>  static uint32_t
> @@ -3448,9 +3459,11 @@ radv_pipeline_init(struct radv_pipeline *pipeline,
>                 pStages[stage] = &pCreateInfo->pStages[i];
>         }
>
> -       radv_create_shaders(pipeline, device, cache,
> -                           radv_generate_graphics_pipeline_key(pipeline,
> pCreateInfo, &blend, has_view_index),
> -                           pStages, pCreateInfo->flags);
> +       result = radv_create_shaders(pipeline, device, cache,
> +
> radv_generate_graphics_pipeline_key(pipeline, pCreateInfo, &blend,
> has_view_index),
> +                                    pStages, pCreateInfo->flags);
> +       if (result != VK_SUCCESS)
> +               return result;
>
>         pipeline->graphics.spi_baryc_cntl =
> S_0286E0_FRONT_FACE_ALL_BITS(1);
>         radv_pipeline_init_multisample_state(pipeline, &blend,
> pCreateInfo);
> @@ -3683,7 +3696,12 @@ static VkResult radv_compute_pipeline_create(
>         assert(pipeline->layout);
>
>         pStages[MESA_SHADER_COMPUTE] = &pCreateInfo->stage;
> -       radv_create_shaders(pipeline, device, cache, (struct
> radv_pipeline_key) {0}, pStages, pCreateInfo->flags);
> +       result = radv_create_shaders(pipeline, device, cache,
> +                                    (struct radv_pipeline_key) {0},
> pStages, pCreateInfo->flags);
> +       if (result != VK_SUCCESS) {
> +               radv_pipeline_destroy(device, pipeline, pAllocator);
> +               return result;
> +       }
>
>         pipeline->user_data_0[MESA_SHADER_COMPUTE] =
> radv_pipeline_stage_to_user_data_0(pipeline, MESA_SHADER_COMPUTE,
> device->physical_device->rad_info.chip_class);
>         pipeline->need_indirect_descriptor_sets |=
> pipeline->shaders[MESA_SHADER_COMPUTE]->info.need_indirect_descriptor_sets;
> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
> index 51e0b7d65fc..9fd7b478a3e 100644
> --- a/src/amd/vulkan/radv_shader.c
> +++ b/src/amd/vulkan/radv_shader.c
> @@ -230,6 +230,9 @@ radv_shader_compile_to_nir(struct radv_device *device,
>                                            spec_entries, num_spec_entries,
>                                            stage, entrypoint_name,
>                                            &spirv_options, &nir_options);
> +               if (!entry_point)
> +                       return NULL;
> +
>                 nir = entry_point->shader;
>                 assert(nir->info.stage == stage);
>                 nir_validate_shader(nir);
> --
> 2.19.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180917/68dbc4ea/attachment.html>