From 7c636acd5380bd8d72687a9228ecd37e78297bcb Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Fri, 18 Mar 2022 10:58:38 -0700 Subject: [PATCH] turnip: Get autotune off of ralloc destructors. We've wanted to remove destructors from ralloc's API for a long time (it's an extra storage cost per ralloc for a rarely-used feature), and for the suballoc change we'd need to spend more storage on storing the tu_device pointer per result since destructors don't get anything else but the pointer passed into them. Fixes use-after-frees: ================================================================= ==2383==ERROR: AddressSanitizer: heap-use-after-free on address 0xffff88fe1940 at pc 0xffff934f427c bp 0xfffff5481e90 sp 0xfffff5481ea8 WRITE of size 8 at 0xffff88fe1940 thread T0 #0 0xffff934f4278 in list_del ../src/util/list.h:108 #1 0xffff934f4278 in result_destructor ../src/freedreno/vulkan/tu_autotune.c:237 #2 0xffff9377793c in unsafe_free ../src/util/ralloc.c:300 #3 0xffff9377793c in ralloc_free ../src/util/ralloc.c:265 #4 0xffff934f4368 in history_destructor ../src/freedreno/vulkan/tu_autotune.c:229 #5 0xffff9377793c in unsafe_free ../src/util/ralloc.c:300 #6 0xffff9377793c in ralloc_free ../src/util/ralloc.c:265 #7 0xffff934f5990 in tu_autotune_on_submit ../src/freedreno/vulkan/tu_autotune.c:442 [...] 0xffff88fe1940 is located 80 bytes inside of 112-byte region [0xffff88fe18f0,0xffff88fe1960) freed by thread T0 here: #0 0xffff9c1c90d8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127 #1 0xffff934f4368 in history_destructor ../src/freedreno/vulkan/tu_autotune.c:229 #2 0xffff9377793c in unsafe_free ../src/util/ralloc.c:300 #3 0xffff9377793c in ralloc_free ../src/util/ralloc.c:265 #4 0xffff934f5990 in tu_autotune_on_submit ../src/freedreno/vulkan/tu_autotune.c:442 #5 0xffff935cf2ac in tu_queue_submit_locked ../src/freedreno/vulkan/tu_drm.c:997 [...] Part-of: --- src/freedreno/ci/freedreno-a630-fails.txt | 6 ---- src/freedreno/vulkan/tu_autotune.c | 35 +++++++++-------------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/freedreno/ci/freedreno-a630-fails.txt b/src/freedreno/ci/freedreno-a630-fails.txt index 385e0b1d76a..e473c051b8b 100644 --- a/src/freedreno/ci/freedreno-a630-fails.txt +++ b/src/freedreno/ci/freedreno-a630-fails.txt @@ -602,12 +602,6 @@ spec@!opengl 2.1@polygon-stipple-fs,Fail spec@!opengl 3.0@clearbuffer-depth-cs-probe,Timeout spec@!opengl 3.1@primitive-restart-xfb generated,Fail -# VK-GL-CTS 1.3.1.1 uprev -dEQP-VK.ray_query.acceleration_structures.flags.fragment_shader.cpu_built.aabbs.identical_instances.nopadding.0_0_compaction_0_bothgeneric,Crash -dEQP-VK.ray_tracing_pipeline.acceleration_structures.flags.cpu_built.triangles_aop.different_instances_aop.nopadding.fastbuild_0_compaction_0_bottomgeneric,Crash -dEQP-VK.ray_tracing_pipeline.pipeline_library.configurations.multithreaded_compilation.s2_l23,Crash -dEQP-VK.texture.shadow.cube_array.linear_mipmap_linear.greater_or_equal_d24_unorm_s8_uint,Crash - # Failures with unaligned gmem store gmem-unaligned-dEQP-VK.renderpass2.depth_stencil_resolve.image_2d_32_32.samples_2.d24_unorm_s8_uint.depth_zero_stencil_zero_testing_stencil_samplemask,Fail gmem-unaligned-dEQP-VK.renderpass2.depth_stencil_resolve.image_2d_32_32.samples_2.d24_unorm_s8_uint.compatibility_depth_zero_stencil_zero_testing_stencil,Fail diff --git a/src/freedreno/vulkan/tu_autotune.c b/src/freedreno/vulkan/tu_autotune.c index 042d0f06d5e..0874fafa548 100644 --- a/src/freedreno/vulkan/tu_autotune.c +++ b/src/freedreno/vulkan/tu_autotune.c @@ -220,21 +220,17 @@ hash_renderpass_instance(const struct tu_render_pass *pass, } static void -history_destructor(void *h) +free_result(struct tu_renderpass_result *result) { - struct tu_renderpass_history *history = h; - - list_for_each_entry_safe(struct tu_renderpass_result, result, - &history->results, node) { - ralloc_free(result); - } + list_del(&result->node); + free(result); } static void -result_destructor(void *r) +free_history(struct tu_renderpass_history *history) { - struct tu_renderpass_result *result = r; - list_del(&result->node); + tu_autotune_free_results(&history->results); + free(history); } static bool @@ -263,11 +259,9 @@ get_history(struct tu_autotune *at, uint64_t rp_key, uint32_t *avg_samples) static struct tu_renderpass_result * create_history_result(struct tu_autotune *at, uint64_t rp_key) { - struct tu_renderpass_result *result = rzalloc_size(NULL, sizeof(*result)); + struct tu_renderpass_result *result = calloc(1, sizeof(*result)); result->rp_key = rp_key; - ralloc_set_destructor(result, result_destructor); - return result; } @@ -286,8 +280,7 @@ history_add_result(struct tu_renderpass_history *history, */ struct tu_renderpass_result *old_result = list_last_entry(&history->results, struct tu_renderpass_result, node); - list_delinit(&old_result->node); - ralloc_free(old_result); + free_result(old_result); } /* Do calculations here to avoid locking history in tu_autotune_use_bypass */ @@ -343,8 +336,7 @@ queue_pending_results(struct tu_autotune *at, struct tu_cmd_buffer *cmdbuf) list_for_each_entry_safe(struct tu_renderpass_result, result, &cmdbuf->renderpass_autotune_results, node) { /* TODO: copying each result isn't nice */ - struct tu_renderpass_result *copy = ralloc_size(NULL, sizeof(*result)); - ralloc_set_destructor(result, result_destructor); + struct tu_renderpass_result *copy = malloc(sizeof(*result)); *copy = *result; list_addtail(©->node, &at->pending_results); } @@ -376,8 +368,7 @@ tu_autotune_on_submit(struct tu_device *dev, struct hash_entry *entry = _mesa_hash_table_search(at->ht, &result->rp_key); if (!entry) { - history = rzalloc_size(NULL, sizeof(*history)); - ralloc_set_destructor(history, history_destructor); + history = calloc(1, sizeof(*history)); history->key = result->rp_key; list_inithead(&history->results); @@ -439,7 +430,7 @@ tu_autotune_on_submit(struct tu_device *dev, _mesa_hash_table_remove_key(at->ht, &history->key); u_rwlock_wrunlock(&at->ht_lock); - ralloc_free(history); + free_history(history); } return &submission_data->fence_cs; @@ -493,7 +484,7 @@ tu_autotune_fini(struct tu_autotune *at, struct tu_device *dev) hash_table_foreach(at->ht, entry) { struct tu_renderpass_history *history = entry->data; - ralloc_free(history); + free_history(history); } list_for_each_entry_safe(struct tu_submission_data, submission_data, @@ -523,7 +514,7 @@ tu_autotune_free_results(struct list_head *results) { list_for_each_entry_safe(struct tu_renderpass_result, result, results, node) { - ralloc_free(result); + free_result(result); } }