ac/llvm,radeonsi: fix memory leaks triggered by ac_nir_translate() errors

Patrick Lerda · 1ace · commit 8428accd423f · 2024-03-24T23:04:54.000Z
For instance, this issue is triggered with "piglit/bin/glslparsertest tests/spec/arb_bindless_texture/compiler/images/arith-bound-image.frag pass 3.30 GL_ARB_bindless_texture GL_ARB_shader_image_load_store": Direct leak of 176 byte(s) in 1 object(s) allocated from: #0 0x7f84c3fbe9a7 in calloc (/usr/lib64/libasan.so.6+0xb19a7) #1 0x7f84ba7e0801 in ac_nir_translate ../src/amd/llvm/ac_nir_to_llvm.c:4391 #2 0x7f84ba53fdf4 in si_llvm_translate_nir ../src/gallium/drivers/radeonsi/si_shader_llvm.c:759 #3 0x7f84ba542bb7 in si_llvm_compile_shader ../src/gallium/drivers/radeonsi/si_shader_llvm.c:836 #4 0x7f84ba337b8e in si_compile_shader ../src/gallium/drivers/radeonsi/si_shader.c:2874 #5 0x7f84ba43a7c1 in si_init_shader_selector_async ../src/gallium/drivers/radeonsi/si_state_shaders.cpp:3176 #6 0x7f84b81c3448 in util_queue_thread_func ../src/util/u_queue.c:309 #7 0x7f84b821ea6a in impl_thrd_routine ../src/c11/impl/threads_posix.c:67 #8 0x7f84c2fea38a (/lib64/libc.so.6+0x8438a) Direct leak of 136 byte(s) in 1 object(s) allocated from: #0 0x7f84c3fbff57 in operator new(unsigned long) (/usr/lib64/libasan.so.6+0xb2f57) #1 0x7f84b1a5f749 in LLVMCreateBuilderInContext (/usr/local/lib64/libLLVM-17.so+0xc84749) #2 0x7f84ba7817b0 in ac_llvm_context_init ../src/amd/llvm/ac_llvm_build.c:54 #3 0x7f84ba542b7a in si_llvm_context_init ../src/gallium/drivers/radeonsi/si_shader_llvm.c:120 #4 0x7f84ba542b7a in si_llvm_compile_shader ../src/gallium/drivers/radeonsi/si_shader_llvm.c:832 #5 0x7f84ba337b8e in si_compile_shader ../src/gallium/drivers/radeonsi/si_shader.c:2874 #6 0x7f84ba43a7c1 in si_init_shader_selector_async ../src/gallium/drivers/radeonsi/si_state_shaders.cpp:3176 #7 0x7f84b81c3448 in util_queue_thread_func ../src/util/u_queue.c:309 #8 0x7f84b821ea6a in impl_thrd_routine ../src/c11/impl/threads_posix.c:67 #9 0x7f84c2fea38a (/lib64/libc.so.6+0x8438a) Indirect leak of 176 byte(s) in 1 object(s) allocated from: #0 0x7f84c3fbe7ef in __interceptor_malloc (/usr/lib64/libasan.so.6+0xb17ef) #1 0x7f84b81b9b3f in ralloc_size ../src/util/ralloc.c:118 #2 0x7f84b81b9fee in rzalloc_size ../src/util/ralloc.c:152 #3 0x7f84b81b9fee in rzalloc_array_size ../src/util/ralloc.c:232 #4 0x7f84b81b05c7 in _mesa_hash_table_init ../src/util/hash_table.c:163 #5 0x7f84b81b05c7 in _mesa_hash_table_create ../src/util/hash_table.c:186 #6 0x7f84ba7e06ae in ac_nir_translate ../src/amd/llvm/ac_nir_to_llvm.c:4381 #7 0x7f84ba53fdf4 in si_llvm_translate_nir ../src/gallium/drivers/radeonsi/si_shader_llvm.c:759 #8 0x7f84ba542bb7 in si_llvm_compile_shader ../src/gallium/drivers/radeonsi/si_shader_llvm.c:836 #9 0x7f84ba337b8e in si_compile_shader ../src/gallium/drivers/radeonsi/si_shader.c:2874 #10 0x7f84ba43a7c1 in si_init_shader_selector_async ../src/gallium/drivers/radeonsi/si_state_shaders.cpp:3176 #11 0x7f84b81c3448 in util_queue_thread_func ../src/util/u_queue.c:309 #12 0x7f84b821ea6a in impl_thrd_routine ../src/c11/impl/threads_posix.c:67 #13 0x7f84c2fea38a (/lib64/libc.so.6+0x8438a) Indirect leak of 176 byte(s) in 1 object(s) allocated from: #0 0x7f84c3fbe7ef in __interceptor_malloc (/usr/lib64/libasan.so.6+0xb17ef) #1 0x7f84b81b9b3f in ralloc_size ../src/util/ralloc.c:118 #2 0x7f84b81b9fee in rzalloc_size ../src/util/ralloc.c:152 #3 0x7f84b81b9fee in rzalloc_array_size ../src/util/ralloc.c:232 #4 0x7f84b81b05c7 in _mesa_hash_table_init ../src/util/hash_table.c:163 #5 0x7f84b81b05c7 in _mesa_hash_table_create ../src/util/hash_table.c:186 #6 0x7f84ba7e06e4 in ac_nir_translate ../src/amd/llvm/ac_nir_to_llvm.c:4382 #7 0x7f84ba53fdf4 in si_llvm_translate_nir ../src/gallium/drivers/radeonsi/si_shader_llvm.c:759 #8 0x7f84ba542bb7 in si_llvm_compile_shader ../src/gallium/drivers/radeonsi/si_shader_llvm.c:836 #9 0x7f84ba337b8e in si_compile_shader ../src/gallium/drivers/radeonsi/si_shader.c:2874 #10 0x7f84ba43a7c1 in si_init_shader_selector_async ../src/gallium/drivers/radeonsi/si_state_shaders.cpp:3176 #11 0x7f84b81c3448 in util_queue_thread_func ../src/util/u_queue.c:309 #12 0x7f84b821ea6a in impl_thrd_routine ../src/c11/impl/threads_posix.c:67 #13 0x7f84c2fea38a (/lib64/libc.so.6+0x8438a) Indirect leak of 128 byte(s) in 1 object(s) allocated from: #0 0x7f84c3fbe7ef in __interceptor_malloc (/usr/lib64/libasan.so.6+0xb17ef) #1 0x7f84b81b9b3f in ralloc_size ../src/util/ralloc.c:118 #2 0x7f84b81b046c in _mesa_hash_table_create ../src/util/hash_table.c:182 #3 0x7f84ba7e06e4 in ac_nir_translate ../src/amd/llvm/ac_nir_to_llvm.c:4382 #4 0x7f84ba53fdf4 in si_llvm_translate_nir ../src/gallium/drivers/radeonsi/si_shader_llvm.c:759 #5 0x7f84ba542bb7 in si_llvm_compile_shader ../src/gallium/drivers/radeonsi/si_shader_llvm.c:836 #6 0x7f84ba337b8e in si_compile_shader ../src/gallium/drivers/radeonsi/si_shader.c:2874 #7 0x7f84ba43a7c1 in si_init_shader_selector_async ../src/gallium/drivers/radeonsi/si_state_shaders.cpp:3176 #8 0x7f84b81c3448 in util_queue_thread_func ../src/util/u_queue.c:309 #9 0x7f84b821ea6a in impl_thrd_routine ../src/c11/impl/threads_posix.c:67 #10 0x7f84c2fea38a (/lib64/libc.so.6+0x8438a) Indirect leak of 128 byte(s) in 1 object(s) allocated from: #0 0x7f84c3fbe7ef in __interceptor_malloc (/usr/lib64/libasan.so.6+0xb17ef) #1 0x7f84b81b9b3f in ralloc_size ../src/util/ralloc.c:118 #2 0x7f84b81b046c in _mesa_hash_table_create ../src/util/hash_table.c:182 #3 0x7f84ba7e06ae in ac_nir_translate ../src/amd/llvm/ac_nir_to_llvm.c:4381 #4 0x7f84ba53fdf4 in si_llvm_translate_nir ../src/gallium/drivers/radeonsi/si_shader_llvm.c:759 #5 0x7f84ba542bb7 in si_llvm_compile_shader ../src/gallium/drivers/radeonsi/si_shader_llvm.c:836 #6 0x7f84ba337b8e in si_compile_shader ../src/gallium/drivers/radeonsi/si_shader.c:2874 #7 0x7f84ba43a7c1 in si_init_shader_selector_async ../src/gallium/drivers/radeonsi/si_state_shaders.cpp:3176 #8 0x7f84b81c3448 in util_queue_thread_func ../src/util/u_queue.c:309 #9 0x7f84b821ea6a in impl_thrd_routine ../src/c11/impl/threads_posix.c:67 #10 0x7f84c2fea38a (/lib64/libc.so.6+0x8438a) SUMMARY: AddressSanitizer: 920 byte(s) leaked in 6 allocation(s). Fixes: d92d35c ("ac/llvm: add a return value to ac_nir_translate") Signed-off-by: Patrick Lerda <patrick9876@free.fr> Reviewed-by: Marek Olšák <marek.olsak@amd.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28099> (cherry picked from commit 0fd907f)
diff --git a/.pick_status.json b/.pick_status.json
@@ -1114,7 +1114,7 @@
         "description": "ac/llvm,radeonsi: fix memory leaks triggered by ac_nir_translate() errors",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "d92d35c9db6da78e1e49e9a851a6c3227a93305c",
         "notes": null
diff --git a/src/amd/llvm/ac_llvm_build.c b/src/amd/llvm/ac_llvm_build.c
@@ -119,6 +119,8 @@ void ac_llvm_context_dispose(struct ac_llvm_context *ctx)
    free(ctx->flow->stack);
    free(ctx->flow);
    ctx->flow = NULL;
+
+   LLVMDisposeBuilder(ctx->builder);
 }
 
 int ac_get_llvm_num_components(LLVMValueRef value)
diff --git a/src/amd/llvm/ac_nir_to_llvm.c b/src/amd/llvm/ac_nir_to_llvm.c
@@ -4365,6 +4365,7 @@ bool ac_nir_translate(struct ac_llvm_context *ac, struct ac_shader_abi *abi,
 {
    struct ac_nir_context ctx = {0};
    struct nir_function *func;
+   bool ret;
 
    ctx.ac = *ac;
    ctx.abi = abi;
@@ -4394,18 +4395,16 @@ bool ac_nir_translate(struct ac_llvm_context *ac, struct ac_shader_abi *abi,
    if (gl_shader_stage_is_compute(nir->info.stage))
       setup_shared(&ctx, nir);
 
-   if (!visit_cf_list(&ctx, &func->impl->body))
-      return false;
-
-   phi_post_pass(&ctx);
+   if ((ret = visit_cf_list(&ctx, &func->impl->body)))
+      phi_post_pass(&ctx);
 
    free(ctx.ssa_defs);
    ralloc_free(ctx.defs);
    ralloc_free(ctx.phis);
    if (ctx.abi->kill_ps_if_inf_interp)
       ralloc_free(ctx.verified_interp);
 
-   return true;
+   return ret;
 }
 
 /* Fixup the HW not emitting the TCS regs if there are no HS threads. */
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -262,7 +262,6 @@ static void
 ac_llvm_finalize_module(struct radv_shader_context *ctx, LLVMPassManagerRef passmgr)
 {
    LLVMRunPassManager(passmgr, ctx->ac.module);
-   LLVMDisposeBuilder(ctx->ac.builder);
 
    ac_llvm_context_dispose(&ctx->ac);
 }
diff --git a/src/gallium/drivers/radeonsi/si_shader_llvm.c b/src/gallium/drivers/radeonsi/si_shader_llvm.c
@@ -239,7 +239,6 @@ void si_llvm_optimize_module(struct si_shader_context *ctx)
 
    /* Run the pass */
    LLVMRunPassManager(ctx->compiler->passmgr, ctx->ac.module);
-   LLVMDisposeBuilder(ctx->ac.builder);
 }
 
 void si_llvm_dispose(struct si_shader_context *ctx)

Original file line number	Diff line number	Diff line change
`@@ -119,6 +119,8 @@ void ac_llvm_context_dispose(struct ac_llvm_context *ctx)`
`119`	`119`	`free(ctx->flow->stack);`
`120`	`120`	`free(ctx->flow);`
`121`	`121`	`ctx->flow = NULL;`
	`122`	`+`
	`123`	`+ LLVMDisposeBuilder(ctx->builder);`
`122`	`124`	`}`
`123`	`125`
`124`	`126`	`int ac_get_llvm_num_components(LLVMValueRef value)`
Original file line number	Diff line number	Diff line change
`@@ -262,7 +262,6 @@ static void`
`262`	`262`	`ac_llvm_finalize_module(struct radv_shader_context *ctx, LLVMPassManagerRef passmgr)`
`263`	`263`	`{`
`264`	`264`	`LLVMRunPassManager(passmgr, ctx->ac.module);`
`265`		`- LLVMDisposeBuilder(ctx->ac.builder);`
`266`	`265`
`267`	`266`	`ac_llvm_context_dispose(&ctx->ac);`
`268`	`267`	`}`
Original file line number	Diff line number	Diff line change
`@@ -239,7 +239,6 @@ void si_llvm_optimize_module(struct si_shader_context *ctx)`
`239`	`239`
`240`	`240`	`/* Run the pass */`
`241`	`241`	`LLVMRunPassManager(ctx->compiler->passmgr, ctx->ac.module);`
`242`		`- LLVMDisposeBuilder(ctx->ac.builder);`
`243`	`242`	`}`
`244`	`243`
`245`	`244`	`void si_llvm_dispose(struct si_shader_context *ctx)`