On Fri, Mar 10, 2017 at 01:38:15PM +0100, Iago Toral Quiroga wrote:
> This situation can happen if we failed to allocate memory for the shader.
>
> v2:
> - We shouldn't see NULL shaders in anv_shader_bin_ref so we should not check
> for that (Jason). Make sure that callers don't attempt to call this
> function with a NULL shader and assert that this never happens (Iago).
> ---
> src/intel/vulkan/anv_pipeline_cache.c | 5 ++++-
> src/intel/vulkan/anv_private.h | 5 ++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_pipeline_cache.c
> b/src/intel/vulkan/anv_pipeline_cache.c
> index a8ea80f..d6c8413 100644
> --- a/src/intel/vulkan/anv_pipeline_cache.c
> +++ b/src/intel/vulkan/anv_pipeline_cache.c
> @@ -308,7 +308,8 @@ anv_pipeline_cache_upload_kernel(struct
> anv_pipeline_cache *cache,
> pthread_mutex_unlock(&cache->mutex);
>
> /* We increment refcount before handing it to the caller */
> - anv_shader_bin_ref(bin);
> + if (bin)
> + anv_shader_bin_ref(bin);
>
> return bin;
> } else {
> @@ -546,6 +547,8 @@ VkResult anv_MergePipelineCaches(
> struct hash_entry *entry;
> hash_table_foreach(src->cache, entry) {
> struct anv_shader_bin *bin = entry->data;
> + assert(bin);
> +
> if (_mesa_hash_table_search(dst->cache, bin->key))
> continue;
>
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 27c1923..aa2b6d7 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1543,13 +1543,16 @@ anv_shader_bin_destroy(struct anv_device *device,
> struct anv_shader_bin *shader)
> static inline void
> anv_shader_bin_ref(struct anv_shader_bin *shader)
> {
> - assert(shader->ref_cnt >= 1);
> + assert(shader && shader->ref_cnt >= 1);
> __sync_fetch_and_add(&shader->ref_cnt, 1);
> }
>
> static inline void
> anv_shader_bin_unref(struct anv_device *device, struct anv_shader_bin
> *shader)
> {
> + if (!shader)
> + return;
> +
There are two callers in anv_pipeline.c: anv_DestroyPipeline and
anv_pipeline_init. Both check against NULL before calling.
In anv_pipeline_cache.c::anv_pipeline_cache_finish() this is only called if
item is found in cache.
Finally lookup_blorp_shader() and upload_blorp_shader() call this if and only
if earlier reference is found in cache. Assert instead?
> assert(shader->ref_cnt >= 1);
> if (__sync_fetch_and_add(&shader->ref_cnt, -1) == 1)
> anv_shader_bin_destroy(device, shader);
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
