[Mesa-dev] [PATCH] mesa/shaderapi: Do a dry run of linking an in-use program

[Neil Roberts]

If an in-use program is unsuccessfully linked, the GL spec requires
that the executable and the uniform state of the program should remain
until a new program is bound. Previously this sort of worked in Mesa
except that it would free the uniform state before attempting to link.
At least on i965 this would usually continue to work because it
accesses the uniform values via a dangling pointer. However it was
causing sporadic failures in one of the CTS tests.

To fix it, when an in-use program is about to be linked, it will now
make a copy of the program and first try to link that instead. If it
works it will let the link continue, otherwise it will copy the status
to the new program and abandon the link. That way the link status and
info log is preserved without freeing the uniform state.

This isn’t very efficient but it would probably quite a big overhaul
to fix it properly and it doesn’t seem worth it because I can’t
imagine that any performance-sensitive application would be hitting
this very strange corner case of the API.

Fixes: KHR-GL46.sepshaderobjs.StateInteraction

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102177
---
 src/mesa/main/shaderapi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 7282435..3f7fe02 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1134,6 +1134,58 @@ _mesa_compile_shader(struct gl_context *ctx, struct gl_shader *sh)
    }
 }
 
+static bool
+try_link_in_use_program(struct gl_context *ctx,
+                        struct gl_shader_program *shProg)
+{
+   struct gl_shader_program *shProgCopy = _mesa_new_shader_program(0);
+   bool ret;
+   int i;
+
+   /* If the program is in use then try linking a copy of the program first.
+    * If it won’t work then we’ll copy the link status to the old program and
+    * abandon the linking. Otherwise we let it relink as normal. This is done
+    * because relinking destroys the uniform state of the program but the GL
+    * spec actually requires that the state be preserved when the program is
+    * in use and the link fails. This isn’t terribly efficent but in practice
+    * it is probably not worth optimising because it’s a rather strange corner
+    * case of the spec.
+    *
+    * GLSL 4.6 spec section 7.3:
+    *
+    * “If a program object that is active for any shader stage is re-linked
+    *  unsuccessfully, the link status will be set to FALSE, but any existing
+    *  executables and associated state will remain part of the current
+    *  rendering state until a subsequent call to UseProgram,
+    *  UseProgramStages, or BindProgramPipeline removes them from use.”
+    */
+
+   for (i = 0; i < shProg->NumShaders; i++)
+      attach_shader(ctx, shProgCopy, shProg->Shaders[i]);
+
+   shProgCopy->BinaryRetreivableHint = shProg->BinaryRetreivableHint;
+   shProgCopy->SeparateShader = shProg->SeparateShader;
+
+   _mesa_glsl_link_shader(ctx, shProgCopy);
+
+   if (shProgCopy->data->LinkStatus) {
+      ret = true;
+   } else {
+      shProg->data->LinkStatus = shProgCopy->data->LinkStatus;
+      shProg->data->Validated = shProgCopy->data->Validated;
+      shProg->data->Version = shProgCopy->data->Version;
+      shProg->IsES = shProgCopy->IsES;
+      ralloc_free(shProg->data->InfoLog);
+      shProg->data->InfoLog =
+         ralloc_strdup(shProg->data, shProgCopy->data->InfoLog);
+
+      ret = false;
+   }
+
+   _mesa_reference_shader_program(ctx, &shProgCopy, NULL);
+
+   return ret;
+}
 
 /**
  * Link a program's shaders.
@@ -1159,12 +1211,16 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg,
    }
 
    unsigned programs_in_use = 0;
-   if (ctx->_Shader)
+   if (ctx->_Shader) {
       for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) {
          if (ctx->_Shader->CurrentProgram[stage] &&
              ctx->_Shader->CurrentProgram[stage]->Id == shProg->Name) {
             programs_in_use |= 1 << stage;
          }
+      }
+
+      if (programs_in_use && !try_link_in_use_program(ctx, shProg))
+         return;
    }
 
    FLUSH_VERTICES(ctx, 0);
-- 
2.9.5