mesa/glsl: delete previously linked shaders earlier when linking

This moves the delete linked shaders call to
_mesa_clear_shader_program_data() which makes sure we delete them
before returning due to any validation problems.

It also reduces some code duplication.

From the OpenGL 4.5 Core spec:

   "If LinkProgram failed, any information about a previous link of
   that program object is lost. Thus, a failed link does not restore
   the old state of program.

   ...

   If one of these commands is called with a program for which
   LinkProgram failed, no error is generated unless otherwise noted.
   Implementations may return information on variables and interface
   blocks that would have been active had the program been linked
   successfully. In cases where the link failed because the program
   required too many resources, these commands may help applications
   determine why limits were exceeded."

Therefore it's expected that we shouldn't be able to query the
program that failed to link and retrieve information about a
previously successful link.

Before this change the linker was doing validation before freeing
the previously linked shaders and therefore could exit on failure
before they were freed.

This change also fixes an issue in compat profile where a program
with no shaders attached is expect to fall back to fixed function
but was instead trying to relink IR from a previous link.

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97715
Cc: "13.0" <mesa-stable@lists.freedesktop.org>

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 03b866f29130df4ea0d2ca501672cc381bd67269..463c1f6c21f029da01677845ef2356962cc7c9ae 100644 (file)
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -4795,14 +4795,6 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
                    "type of shader\n");
    }
 
-   for (unsigned int i = 0; i < MESA_SHADER_STAGES; i++) {
-      if (prog->_LinkedShaders[i] != NULL) {
-         _mesa_delete_linked_shader(ctx, prog->_LinkedShaders[i]);
-      }
-
-      prog->_LinkedShaders[i] = NULL;
-   }
-
    /* Link all shaders for a particular stage and validate the result.
     */
    for (int stage = 0; stage < MESA_SHADER_STAGES; stage++) {

diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp
index 055c433436a343b1e48fb01f502bc82d71352c47..f0964900938c89276bda0d25b9a7b7c8c7ac5063 100644 (file)
--- a/src/compiler/glsl/standalone.cpp
+++ b/src/compiler/glsl/standalone.cpp
@@ -421,7 +421,7 @@ standalone_compile_shader(const struct standalone_options *_options,
    }
 
    if ((status == EXIT_SUCCESS) && options->do_link)  {
-      _mesa_clear_shader_program_data(whole_program);
+      _mesa_clear_shader_program_data(ctx, whole_program);
 
       link_shaders(ctx, whole_program);
       status = (whole_program->LinkStatus) ? EXIT_SUCCESS : EXIT_FAILURE;

diff --git a/src/compiler/glsl/standalone_scaffolding.cpp b/src/compiler/glsl/standalone_scaffolding.cpp
index 35e40d6c91b044d62c8ba779c08f2c15e6a0fab5..d229368ed340da9fbd83be7595ccc10081c1998e 100644 (file)
--- a/src/compiler/glsl/standalone_scaffolding.cpp
+++ b/src/compiler/glsl/standalone_scaffolding.cpp
@@ -123,8 +123,16 @@ _mesa_delete_linked_shader(struct gl_context *ctx,
 }
 
 void
-_mesa_clear_shader_program_data(struct gl_shader_program *shProg)
+_mesa_clear_shader_program_data(struct gl_context *ctx,
+                                struct gl_shader_program *shProg)
 {
+   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+      if (shProg->_LinkedShaders[i] != NULL) {
+         _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[i]);
+         shProg->_LinkedShaders[i] = NULL;
+      }
+   }
+
    shProg->NumUniformStorage = 0;
    shProg->UniformStorage = NULL;
    shProg->NumUniformRemapTable = 0;

diff --git a/src/compiler/glsl/standalone_scaffolding.h b/src/compiler/glsl/standalone_scaffolding.h
index b56dd3e8707bf58c15b73ee9e19335b450f42e83..2c0454837a53ed6594250e16efb8835051421502 100644 (file)
--- a/src/compiler/glsl/standalone_scaffolding.h
+++ b/src/compiler/glsl/standalone_scaffolding.h
@@ -56,7 +56,8 @@ _mesa_delete_linked_shader(struct gl_context *ctx,
                            struct gl_linked_shader *sh);
 
 extern "C" void
-_mesa_clear_shader_program_data(struct gl_shader_program *);
+_mesa_clear_shader_program_data(struct gl_context *ctx,
+                                struct gl_shader_program *);
 
 extern "C" void
 _mesa_shader_debug(struct gl_context *ctx, GLenum type, GLuint *id,

diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 136ac7b46a6409ddd069041f0b4b3a81ec6384a7..8fd574e5121d01569fa2f2fa575f15b9f7944122 100644 (file)
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -291,12 +291,18 @@ _mesa_new_shader_program(GLuint name)
  * Clear (free) the shader program state that gets produced by linking.
  */
 void
-_mesa_clear_shader_program_data(struct gl_shader_program *shProg)
+_mesa_clear_shader_program_data(struct gl_context *ctx,
+                                struct gl_shader_program *shProg)
 {
-   unsigned i;
+   for (gl_shader_stage sh = 0; sh < MESA_SHADER_STAGES; sh++) {
+      if (shProg->_LinkedShaders[sh] != NULL) {
+         _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[sh]);
+         shProg->_LinkedShaders[sh] = NULL;
+      }
+   }
 
    if (shProg->UniformStorage) {
-      for (i = 0; i < shProg->NumUniformStorage; ++i)
+      for (unsigned i = 0; i < shProg->NumUniformStorage; ++i)
          _mesa_uniform_detach_all_driver_storage(&shProg->UniformStorage[i]);
       ralloc_free(shProg->UniformStorage);
       shProg->NumUniformStorage = 0;
@@ -347,11 +353,10 @@ _mesa_free_shader_program_data(struct gl_context *ctx,
                                struct gl_shader_program *shProg)
 {
    GLuint i;
-   gl_shader_stage sh;
 
    assert(shProg->Type == GL_SHADER_PROGRAM_MESA);
 
-   _mesa_clear_shader_program_data(shProg);
+   _mesa_clear_shader_program_data(ctx, shProg);
 
    if (shProg->AttributeBindings) {
       string_to_uint_map_dtor(shProg->AttributeBindings);
@@ -385,14 +390,6 @@ _mesa_free_shader_program_data(struct gl_context *ctx,
    shProg->TransformFeedback.VaryingNames = NULL;
    shProg->TransformFeedback.NumVarying = 0;
 
-
-   for (sh = 0; sh < MESA_SHADER_STAGES; sh++) {
-      if (shProg->_LinkedShaders[sh] != NULL) {
-        _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[sh]);
-        shProg->_LinkedShaders[sh] = NULL;
-      }
-   }
-
    free(shProg->Label);
    shProg->Label = NULL;
 }

diff --git a/src/mesa/main/shaderobj.h b/src/mesa/main/shaderobj.h
index 814a7f11552e8f18b67a315c7a826763eb46accd..12497328f986328b28c84b9f0f85361f85003760 100644 (file)
--- a/src/mesa/main/shaderobj.h
+++ b/src/mesa/main/shaderobj.h
@@ -99,7 +99,8 @@ extern struct gl_shader_program *
 _mesa_new_shader_program(GLuint name);
 
 extern void
-_mesa_clear_shader_program_data(struct gl_shader_program *shProg);
+_mesa_clear_shader_program_data(struct gl_context *ctx,
+                                struct gl_shader_program *shProg);
 
 extern void
 _mesa_free_shader_program_data(struct gl_context *ctx,

diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 5776d15c8a66c695179b7f2423d6220b795c05a3..f4c2ad6770c1f90d50914e525d9917734ae394cf 100644 (file)
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -3049,7 +3049,7 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
 {
    unsigned int i;
 
-   _mesa_clear_shader_program_data(prog);
+   _mesa_clear_shader_program_data(ctx, prog);
 
    prog->LinkStatus = GL_TRUE;