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

[Samuel Pitoiset]

On 9/17/18 11:22 AM, Jason Ekstrand wrote:
> 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.

Yes, you are right. Please ignore this patch.

> 
> On Mon, Sep 17, 2018 at 4:18 AM Samuel Pitoiset 
> <samuel.pitoiset at gmail.com <mailto: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
>     <mailto: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 <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>