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

[Timothy Arceri]

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

On 17/9/18 7:18 pm, Samuel Pitoiset 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);
>