mesa: rework how we free gl_shader_program_data

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" <mesa-stable@lists.freedesktop.org>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Neil Roberts <nroberts@igalia.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102177

diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index e2103bcde4974e953f7ea3da0b8700c1c44a1a83..ce2e3df4faec33aa4b32ead7e2d541fa41d9fd6c 100644 (file)
--- 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 97b8ce7ac233f55c22561374ce4679cfbfd943f6..084848e220c5d850f8bf37deb607b54bf5a652f6 100644 (file)
--- 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 aa330638836b27c74dbc06f10323d801eba5fd67..327fd61d422c8d7f5f75194e7bea51a62993cbf0 100644 (file)
--- 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++) {