From 6a72eba755fea15a0d97abb913a6315d9d32e274 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 8 Nov 2017 10:57:21 +1100 Subject: [PATCH] mesa: rework how we free gl_shader_program_data MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When I introduced gl_shader_program_data one of the intentions was to fix a bug where a failed linking attempt freed data required by a currently active program. However I seem to have failed to finish hooking up the final steps required to have the data hang around. Here we create a fresh instance of gl_shader_program_data every time we link. gl_program has a reference to gl_shader_program_data so it will be freed once the program is no longer active. Cc: "17.2 17.3" Reviewed-by: Tapani Pälli Reviewed-by: Neil Roberts Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102177 --- src/mesa/main/shaderobj.c | 55 ++++++++++------------------------------- src/mesa/main/shaderobj.h | 3 +++ src/mesa/program/ir_to_mesa.cpp | 2 ++ 3 files changed, 18 insertions(+), 42 deletions(-) diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c index e2103bc..ce2e3df 100644 --- a/src/mesa/main/shaderobj.c +++ b/src/mesa/main/shaderobj.c @@ -193,7 +193,6 @@ _mesa_lookup_shader_err(struct gl_context *ctx, GLuint name, const char *caller) /*** Shader Program object functions ***/ /**********************************************************************/ - void _mesa_reference_shader_program_data(struct gl_context *ctx, struct gl_shader_program_data **ptr, @@ -209,6 +208,12 @@ _mesa_reference_shader_program_data(struct gl_context *ctx, if (p_atomic_dec_zero(&oldData->RefCount)) { assert(ctx); + assert(oldData->NumUniformStorage == 0 || + oldData->UniformStorage); + + for (unsigned i = 0; i < oldData->NumUniformStorage; ++i) + _mesa_uniform_detach_all_driver_storage(&oldData->UniformStorage[i]); + ralloc_free(oldData); } @@ -259,13 +264,15 @@ _mesa_reference_shader_program_(struct gl_context *ctx, } } -static struct gl_shader_program_data * -create_shader_program_data() +struct gl_shader_program_data * +_mesa_create_shader_program_data() { struct gl_shader_program_data *data; data = rzalloc(NULL, struct gl_shader_program_data); - if (data) + if (data) { data->RefCount = 1; + data->InfoLog = ralloc_strdup(data, ""); + } return data; } @@ -286,8 +293,6 @@ init_shader_program(struct gl_shader_program *prog) prog->TransformFeedback.BufferMode = GL_INTERLEAVED_ATTRIBS; exec_list_make_empty(&prog->EmptyUniformLocations); - - prog->data->InfoLog = ralloc_strdup(prog->data, ""); } /** @@ -300,7 +305,7 @@ _mesa_new_shader_program(GLuint name) shProg = rzalloc(NULL, struct gl_shader_program); if (shProg) { shProg->Name = name; - shProg->data = create_shader_program_data(); + shProg->data = _mesa_create_shader_program_data(); if (!shProg->data) { ralloc_free(shProg); return NULL; @@ -325,17 +330,6 @@ _mesa_clear_shader_program_data(struct gl_context *ctx, } } - shProg->data->linked_stages = 0; - - if (shProg->data->UniformStorage) { - for (unsigned i = 0; i < shProg->data->NumUniformStorage; ++i) - _mesa_uniform_detach_all_driver_storage(&shProg->data-> - UniformStorage[i]); - ralloc_free(shProg->data->UniformStorage); - shProg->data->NumUniformStorage = 0; - shProg->data->UniformStorage = NULL; - } - if (shProg->UniformRemapTable) { ralloc_free(shProg->UniformRemapTable); shProg->NumUniformRemapTable = 0; @@ -347,29 +341,7 @@ _mesa_clear_shader_program_data(struct gl_context *ctx, shProg->UniformHash = NULL; } - assert(shProg->data->InfoLog != NULL); - ralloc_free(shProg->data->InfoLog); - shProg->data->InfoLog = ralloc_strdup(shProg->data, ""); - - ralloc_free(shProg->data->UniformBlocks); - shProg->data->UniformBlocks = NULL; - shProg->data->NumUniformBlocks = 0; - - ralloc_free(shProg->data->ShaderStorageBlocks); - shProg->data->ShaderStorageBlocks = NULL; - shProg->data->NumShaderStorageBlocks = 0; - - if (shProg->data->AtomicBuffers) { - ralloc_free(shProg->data->AtomicBuffers); - shProg->data->AtomicBuffers = NULL; - shProg->data->NumAtomicBuffers = 0; - } - - if (shProg->data->ProgramResourceList) { - ralloc_free(shProg->data->ProgramResourceList); - shProg->data->ProgramResourceList = NULL; - shProg->data->NumProgramResourceList = 0; - } + _mesa_reference_shader_program_data(ctx, &shProg->data, NULL); } @@ -432,7 +404,6 @@ _mesa_delete_shader_program(struct gl_context *ctx, struct gl_shader_program *shProg) { _mesa_free_shader_program_data(ctx, shProg); - _mesa_reference_shader_program_data(ctx, &shProg->data, NULL); ralloc_free(shProg); } diff --git a/src/mesa/main/shaderobj.h b/src/mesa/main/shaderobj.h index 97b8ce7..084848e 100644 --- a/src/mesa/main/shaderobj.h +++ b/src/mesa/main/shaderobj.h @@ -100,6 +100,9 @@ _mesa_lookup_shader_program_err(struct gl_context *ctx, GLuint name, extern struct gl_shader_program * _mesa_new_shader_program(GLuint name); +extern struct gl_shader_program_data * +_mesa_create_shader_program_data(void); + extern void _mesa_clear_shader_program_data(struct gl_context *ctx, struct gl_shader_program *shProg); diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index aa33063..327fd61 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -3067,6 +3067,8 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog) _mesa_clear_shader_program_data(ctx, prog); + prog->data = _mesa_create_shader_program_data(); + prog->data->LinkStatus = linking_success; for (i = 0; i < prog->NumShaders; i++) { -- 2.7.4