From df0202feafd7c1737c428e31634e9c20af30de74 Mon Sep 17 00:00:00 2001 From: "Juan A. Suarez Romero" Date: Tue, 27 Apr 2021 12:08:50 +0200 Subject: [PATCH] util/hash_table: do not leak u64 struct key For non 64bit devices the key stored in hash_table_u64 is wrapped in hash_key_u64 structure, which is never free. This commit fixes this issue by just removing the user-defined `delete_function` parameter in hash_table_u64_{destroy,clear} (which nobody is using) and using instead a delete function to free this structure. Fixes: 608257cf82f ("i965: Fix INTEL_DEBUG=bat") Reviewed-by: Ian Romanick Signed-off-by: Juan A. Suarez Romero Part-of: (cherry picked from commit e532a47f76cc8d763e2534c61b27dce0f5bc86a0) Conflicts: src/microsoft/compiler/dxil_nir.c --- .pick_status.json | 2 +- src/freedreno/ir3/ir3_compiler_nir.c | 2 +- src/gallium/drivers/lima/ir/pp/nir.c | 4 +- .../winsys/radeon/drm/radeon_drm_winsys.c | 2 +- src/mesa/drivers/dri/i965/brw_batch.c | 4 +- src/mesa/main/shader_query.cpp | 2 +- src/mesa/main/shaderobj.c | 2 +- src/mesa/main/texturebindless.c | 8 +-- src/panfrost/lib/decode_common.c | 2 +- src/panfrost/midgard/mir_squeeze.c | 2 +- src/util/fossilize_db.c | 2 +- src/util/hash_table.c | 66 +++++-------------- src/util/hash_table.h | 6 +- 13 files changed, 36 insertions(+), 68 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index a7ede526528..7a169cd9140 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -94,7 +94,7 @@ "description": "util/hash_table: do not leak u64 struct key", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "master_sha": null, "because_sha": "608257cf82f49109c8f1a2bab1d6e30fa14f9ba7" }, diff --git a/src/freedreno/ir3/ir3_compiler_nir.c b/src/freedreno/ir3/ir3_compiler_nir.c index 5fd2ecaa01e..4db7e87cb14 100644 --- a/src/freedreno/ir3/ir3_compiler_nir.c +++ b/src/freedreno/ir3/ir3_compiler_nir.c @@ -2751,7 +2751,7 @@ emit_block(struct ir3_context *ctx, nir_block *nblock) ctx->addr0_ht[i] = NULL; } - _mesa_hash_table_u64_destroy(ctx->addr1_ht, NULL); + _mesa_hash_table_u64_destroy(ctx->addr1_ht); ctx->addr1_ht = NULL; nir_foreach_instr (instr, nblock) { diff --git a/src/gallium/drivers/lima/ir/pp/nir.c b/src/gallium/drivers/lima/ir/pp/nir.c index 8790ec49142..b6ab9d4f2ce 100644 --- a/src/gallium/drivers/lima/ir/pp/nir.c +++ b/src/gallium/drivers/lima/ir/pp/nir.c @@ -972,12 +972,12 @@ bool ppir_compile_nir(struct lima_fs_compiled_shader *prog, struct nir_shader *n ppir_print_shader_db(nir, comp, debug); - _mesa_hash_table_u64_destroy(comp->blocks, NULL); + _mesa_hash_table_u64_destroy(comp->blocks); ralloc_free(comp); return true; err_out0: - _mesa_hash_table_u64_destroy(comp->blocks, NULL); + _mesa_hash_table_u64_destroy(comp->blocks); ralloc_free(comp); return false; } diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c index 7c3e99404f9..08ee7e7e5cf 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c @@ -636,7 +636,7 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws) _mesa_hash_table_destroy(ws->bo_names, NULL); _mesa_hash_table_destroy(ws->bo_handles, NULL); - _mesa_hash_table_u64_destroy(ws->bo_vas, NULL); + _mesa_hash_table_u64_destroy(ws->bo_vas); mtx_destroy(&ws->bo_handles_mutex); mtx_destroy(&ws->vm32.mutex); mtx_destroy(&ws->vm64.mutex); diff --git a/src/mesa/drivers/dri/i965/brw_batch.c b/src/mesa/drivers/dri/i965/brw_batch.c index 92d6ec0f93e..4db72dc4ec8 100644 --- a/src/mesa/drivers/dri/i965/brw_batch.c +++ b/src/mesa/drivers/dri/i965/brw_batch.c @@ -276,7 +276,7 @@ brw_batch_reset(struct brw_context *brw) batch->state_base_address_emitted = false; if (batch->state_batch_sizes) - _mesa_hash_table_u64_clear(batch->state_batch_sizes, NULL); + _mesa_hash_table_u64_clear(batch->state_batch_sizes); /* Always add workaround_bo which contains a driver identifier to be * recorded in error states. @@ -345,7 +345,7 @@ brw_batch_free(struct brw_batch *batch) brw_bo_unreference(batch->batch.bo); brw_bo_unreference(batch->state.bo); if (batch->state_batch_sizes) { - _mesa_hash_table_u64_destroy(batch->state_batch_sizes, NULL); + _mesa_hash_table_u64_destroy(batch->state_batch_sizes); intel_batch_decode_ctx_finish(&batch->decoder); } } diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index cb86fdd32c2..240f6e89e8c 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -1951,7 +1951,7 @@ _mesa_create_program_resource_hash(struct gl_shader_program *shProg) { /* Rebuild resource hash. */ if (shProg->data->ProgramResourceHash) - _mesa_hash_table_u64_destroy(shProg->data->ProgramResourceHash, NULL); + _mesa_hash_table_u64_destroy(shProg->data->ProgramResourceHash); shProg->data->ProgramResourceHash = _mesa_hash_table_u64_create(shProg); diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c index 3a3483c0182..27eab80b94a 100644 --- a/src/mesa/main/shaderobj.c +++ b/src/mesa/main/shaderobj.c @@ -358,7 +358,7 @@ _mesa_clear_shader_program_data(struct gl_context *ctx, } if (shProg->data && shProg->data->ProgramResourceHash) { - _mesa_hash_table_u64_destroy(shProg->data->ProgramResourceHash, NULL); + _mesa_hash_table_u64_destroy(shProg->data->ProgramResourceHash); shProg->data->ProgramResourceHash = NULL; } diff --git a/src/mesa/main/texturebindless.c b/src/mesa/main/texturebindless.c index 939c01dd021..18f82c5d2ec 100644 --- a/src/mesa/main/texturebindless.c +++ b/src/mesa/main/texturebindless.c @@ -378,8 +378,8 @@ _mesa_init_resident_handles(struct gl_context *ctx) void _mesa_free_resident_handles(struct gl_context *ctx) { - _mesa_hash_table_u64_destroy(ctx->ResidentTextureHandles, NULL); - _mesa_hash_table_u64_destroy(ctx->ResidentImageHandles, NULL); + _mesa_hash_table_u64_destroy(ctx->ResidentTextureHandles); + _mesa_hash_table_u64_destroy(ctx->ResidentImageHandles); } /** @@ -397,10 +397,10 @@ void _mesa_free_shared_handles(struct gl_shared_state *shared) { if (shared->TextureHandles) - _mesa_hash_table_u64_destroy(shared->TextureHandles, NULL); + _mesa_hash_table_u64_destroy(shared->TextureHandles); if (shared->ImageHandles) - _mesa_hash_table_u64_destroy(shared->ImageHandles, NULL); + _mesa_hash_table_u64_destroy(shared->ImageHandles); mtx_destroy(&shared->HandlesMutex); } diff --git a/src/panfrost/lib/decode_common.c b/src/panfrost/lib/decode_common.c index cd4be5b7027..b0dd34aae9b 100644 --- a/src/panfrost/lib/decode_common.c +++ b/src/panfrost/lib/decode_common.c @@ -212,7 +212,7 @@ pandecode_next_frame(void) void pandecode_close(void) { - _mesa_hash_table_u64_destroy(mmap_table, NULL); + _mesa_hash_table_u64_destroy(mmap_table); util_dynarray_fini(&ro_mappings); pandecode_dump_file_close(); } diff --git a/src/panfrost/midgard/mir_squeeze.c b/src/panfrost/midgard/mir_squeeze.c index 10fa9a6eb97..aa230f836db 100644 --- a/src/panfrost/midgard/mir_squeeze.c +++ b/src/panfrost/midgard/mir_squeeze.c @@ -83,5 +83,5 @@ mir_squeeze_index(compiler_context *ctx) ctx->blend_input = find_or_allocate_temp(ctx, map, ctx->blend_input); ctx->blend_src1 = find_or_allocate_temp(ctx, map, ctx->blend_src1); - _mesa_hash_table_u64_destroy(map, NULL); + _mesa_hash_table_u64_destroy(map); } diff --git a/src/util/fossilize_db.c b/src/util/fossilize_db.c index 292389b44a4..b77d7bcfa9b 100644 --- a/src/util/fossilize_db.c +++ b/src/util/fossilize_db.c @@ -304,7 +304,7 @@ foz_destroy(struct foz_db *foz_db) } if (foz_db->mem_ctx) { - _mesa_hash_table_u64_destroy(foz_db->index_db, NULL); + _mesa_hash_table_u64_destroy(foz_db->index_db); ralloc_free(foz_db->mem_ctx); simple_mtx_destroy(&foz_db->mtx); } diff --git a/src/util/hash_table.c b/src/util/hash_table.c index 079427bd951..a0cebdb250b 100644 --- a/src/util/hash_table.c +++ b/src/util/hash_table.c @@ -768,65 +768,35 @@ _mesa_hash_table_u64_create(void *mem_ctx) return ht; } -void -_mesa_hash_table_u64_clear(struct hash_table_u64 *ht, - void (*delete_function)(struct hash_entry *entry)) +static void +_mesa_hash_table_u64_delete_key(struct hash_entry *entry) { - if (!ht) + if (sizeof(void *) == 8) return; - if (ht->deleted_key_data) { - if (delete_function) { - struct hash_table *table = ht->table; - struct hash_entry entry; + struct hash_key_u64 *_key = (struct hash_key_u64 *)entry->key; - /* Create a fake entry for the delete function. */ - if (sizeof(void *) == 8) { - entry.hash = table->key_hash_function(table->deleted_key); - } else { - struct hash_key_u64 _key = { .value = (uintptr_t)table->deleted_key }; - entry.hash = table->key_hash_function(&_key); - } - entry.key = table->deleted_key; - entry.data = ht->deleted_key_data; - - delete_function(&entry); - } - ht->deleted_key_data = NULL; - } - - if (ht->freed_key_data) { - if (delete_function) { - struct hash_table *table = ht->table; - struct hash_entry entry; - - /* Create a fake entry for the delete function. */ - if (sizeof(void *) == 8) { - entry.hash = table->key_hash_function(uint_key(FREED_KEY_VALUE)); - } else { - struct hash_key_u64 _key = { .value = (uintptr_t)FREED_KEY_VALUE }; - entry.hash = table->key_hash_function(&_key); - } - entry.key = uint_key(FREED_KEY_VALUE); - entry.data = ht->freed_key_data; - - delete_function(&entry); - } - ht->freed_key_data = NULL; - } - - _mesa_hash_table_clear(ht->table, delete_function); + if (_key) + free(_key); } void -_mesa_hash_table_u64_destroy(struct hash_table_u64 *ht, - void (*delete_function)(struct hash_entry *entry)) +_mesa_hash_table_u64_clear(struct hash_table_u64 *ht) { if (!ht) return; - _mesa_hash_table_u64_clear(ht, delete_function); - _mesa_hash_table_destroy(ht->table, delete_function); + _mesa_hash_table_clear(ht->table, _mesa_hash_table_u64_delete_key); +} + +void +_mesa_hash_table_u64_destroy(struct hash_table_u64 *ht) +{ + if (!ht) + return; + + _mesa_hash_table_u64_clear(ht); + _mesa_hash_table_destroy(ht->table, NULL); free(ht); } diff --git a/src/util/hash_table.h b/src/util/hash_table.h index ff3b3bef576..b61721a5df8 100644 --- a/src/util/hash_table.h +++ b/src/util/hash_table.h @@ -172,8 +172,7 @@ struct hash_table_u64 * _mesa_hash_table_u64_create(void *mem_ctx); void -_mesa_hash_table_u64_destroy(struct hash_table_u64 *ht, - void (*delete_function)(struct hash_entry *entry)); +_mesa_hash_table_u64_destroy(struct hash_table_u64 *ht); void _mesa_hash_table_u64_insert(struct hash_table_u64 *ht, uint64_t key, @@ -186,8 +185,7 @@ void _mesa_hash_table_u64_remove(struct hash_table_u64 *ht, uint64_t key); void -_mesa_hash_table_u64_clear(struct hash_table_u64 *ht, - void (*delete_function)(struct hash_entry *entry)); +_mesa_hash_table_u64_clear(struct hash_table_u64 *ht); #ifdef __cplusplus } /* extern C */