[Mesa-dev] [PATCH] glsl_compiler: Add binding hash tables to avoid SIGSEVs on linking stage

Andres Gomez agomez at igalia.com
Mon Nov 17 06:27:47 PST 2014


When using the stand alone compiler, if we try to
link a shader with vertex attributes it will
segfault on linking as the binding hash tables are
not included in the shader program. Obviously, we
cannot make the linking stage succeed without the
bound attributes but we can prevent the crash and
just let the linker spit its own error.

This also adds carriage returns on several linker
errors and fixes a couple of inline comments.
---
 src/gallium/auxiliary/draw/draw_private.h   |  2 +-
 src/gallium/auxiliary/draw/draw_pt_vsplit.c |  2 +-
 src/glsl/linker.cpp                         | 40 ++++++++++++++---------------
 src/glsl/main.cpp                           | 10 ++++++++
 4 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h
index d8dc2ab..37045eb 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -499,7 +499,7 @@ draw_clamp_viewport_idx(int idx)
 /**
  * Adds two unsigned integers and if the addition
  * overflows then it returns the value from
- * from the overflow_value variable.
+ * the overflow_value variable.
  */
 static INLINE unsigned
 draw_overflow_uadd(unsigned a, unsigned b,
diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit.c b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
index eebcabb..8098ade 100644
--- a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
@@ -91,7 +91,7 @@ vsplit_add_cache(struct vsplit_frontend *vsplit, unsigned fetch, unsigned ofbias
 
    hash = fetch % MAP_SIZE;
 
-   /* If the value isn't in the cache of it's an overflow due to the
+   /* If the value isn't in the cache or it's an overflow due to the
     * element bias */
    if (vsplit->cache.fetches[hash] != fetch || ofbias) {
       /* update cache */
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 2d31801..794efdc 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -642,7 +642,7 @@ validate_geometry_shader_emissions(struct gl_context *ctx,
       emit_vertex.run(prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->ir);
       if (emit_vertex.error()) {
          linker_error(prog, "Invalid call %s(%d). Accepted values for the "
-                      "stream parameter are in the range [0, %d].",
+                      "stream parameter are in the range [0, %d].\n",
                       emit_vertex.error_func(),
                       emit_vertex.error_stream(),
                       ctx->Const.MaxVertexStreams - 1);
@@ -676,7 +676,7 @@ validate_geometry_shader_emissions(struct gl_context *ctx,
        */
       if (prog->Geom.UsesStreams && prog->Geom.OutputType != GL_POINTS) {
          linker_error(prog, "EmitStreamVertex(n) and EndStreamPrimitive(n) "
-                      "with n>0 requires point output");
+                      "with n>0 requires point output\n");
       }
    }
 }
@@ -808,7 +808,7 @@ cross_validate_globals(struct gl_shader_program *prog,
 		  linker_error(prog,
 			       "All redeclarations of gl_FragDepth in all "
 			       "fragment shaders in a single program must have "
-			       "the same set of qualifiers.");
+			       "the same set of qualifiers.\n");
 	       }
 
 	       if (var->data.used && layout_differs) {
@@ -817,7 +817,7 @@ cross_validate_globals(struct gl_shader_program *prog,
 			       "qualifier in any fragment shader, it must be "
 			       "redeclared with the same layout qualifier in "
 			       "all fragment shaders that have assignments to "
-			       "gl_FragDepth");
+			       "gl_FragDepth\n");
 	       }
 	    }
 
@@ -948,7 +948,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog)
 						       &sh->UniformBlocks[j]);
 
 	 if (index == -1) {
-	    linker_error(prog, "uniform block `%s' has mismatching definitions",
+	    linker_error(prog, "uniform block `%s' has mismatching definitions\n",
 			 sh->UniformBlocks[j].Name);
 	    return false;
 	 }
@@ -1635,7 +1635,7 @@ link_intrastage_shaders(void *mem_ctx,
 
 	       if ((other_sig != NULL) && other_sig->is_defined
 		   && !other_sig->is_builtin()) {
-		  linker_error(prog, "function `%s' is multiply defined",
+		  linker_error(prog, "function `%s' is multiply defined\n",
 			       f->name);
 		  return NULL;
 	       }
@@ -2086,7 +2086,7 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
             if (attr + slots > max_index) {
                linker_error(prog,
                            "insufficient contiguous locations "
-                           "available for %s `%s' %d %d %d", string,
+                           "available for %s `%s' %d %d %d\n", string,
                            var->name, used_locations, use_mask, attr);
                return false;
             }
@@ -2155,7 +2155,7 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
 
 	 linker_error(prog,
 		      "insufficient contiguous locations "
-		      "available for %s `%s'",
+		      "available for %s `%s'\n",
 		      string, to_assign[i].var->name);
 	 return false;
       }
@@ -2257,7 +2257,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
 	 continue;
 
       if (sh->num_samplers > ctx->Const.Program[i].MaxTextureImageUnits) {
-	 linker_error(prog, "Too many %s shader texture samplers",
+	 linker_error(prog, "Too many %s shader texture samplers\n",
 		      _mesa_shader_stage_to_string(i));
       }
 
@@ -2271,7 +2271,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
                            _mesa_shader_stage_to_string(i));
          } else {
             linker_error(prog, "Too many %s shader default uniform block "
-			 "components",
+			 "components\n",
                          _mesa_shader_stage_to_string(i));
          }
       }
@@ -2284,7 +2284,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
                            "this is non-portable out-of-spec behavior\n",
                            _mesa_shader_stage_to_string(i));
          } else {
-            linker_error(prog, "Too many %s shader uniform components",
+            linker_error(prog, "Too many %s shader uniform components\n",
                          _mesa_shader_stage_to_string(i));
          }
       }
@@ -2302,7 +2302,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
       }
 
       if (total_uniform_blocks > ctx->Const.MaxCombinedUniformBlocks) {
-	 linker_error(prog, "Too many combined uniform blocks (%d/%d)",
+	 linker_error(prog, "Too many combined uniform blocks (%d/%d)\n",
 		      prog->NumUniformBlocks,
 		      ctx->Const.MaxCombinedUniformBlocks);
       } else {
@@ -2310,7 +2310,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
             const unsigned max_uniform_blocks =
                ctx->Const.Program[i].MaxUniformBlocks;
 	    if (blocks[i] > max_uniform_blocks) {
-	       linker_error(prog, "Too many %s uniform blocks (%d/%d)",
+	       linker_error(prog, "Too many %s uniform blocks (%d/%d)\n",
 			    _mesa_shader_stage_to_string(i),
 			    blocks[i],
 			    max_uniform_blocks);
@@ -2338,7 +2338,7 @@ check_image_resources(struct gl_context *ctx, struct gl_shader_program *prog)
 
       if (sh) {
          if (sh->NumImages > ctx->Const.Program[i].MaxImageUniforms)
-            linker_error(prog, "Too many %s shader image uniforms",
+            linker_error(prog, "Too many %s shader image uniforms\n",
                          _mesa_shader_stage_to_string(i));
 
          total_image_units += sh->NumImages;
@@ -2354,11 +2354,11 @@ check_image_resources(struct gl_context *ctx, struct gl_shader_program *prog)
    }
 
    if (total_image_units > ctx->Const.MaxCombinedImageUniforms)
-      linker_error(prog, "Too many combined image uniforms");
+      linker_error(prog, "Too many combined image uniforms\n");
 
    if (total_image_units + fragment_outputs >
        ctx->Const.MaxCombinedImageUnitsAndFragmentOutputs)
-      linker_error(prog, "Too many combined image uniforms and fragment outputs");
+      linker_error(prog, "Too many combined image uniforms and fragment outputs\n");
 }
 
 
@@ -2382,7 +2382,7 @@ reserve_explicit_locations(struct gl_shader_program *prog,
                   max_loc + 1);
 
       if (!prog->UniformRemapTable) {
-         linker_error(prog, "Out of memory during linking.");
+         linker_error(prog, "Out of memory during linking.\n");
          return false;
       }
 
@@ -2412,7 +2412,7 @@ reserve_explicit_locations(struct gl_shader_program *prog,
           */
          linker_error(prog,
                       "location qualifier for uniform %s overlaps"
-                      "previously used location",
+                      "previously used location\n",
                       var->name);
          return false;
       }
@@ -2447,7 +2447,7 @@ check_explicit_uniform_locations(struct gl_context *ctx,
    string_to_uint_map *uniform_map = new string_to_uint_map;
 
    if (!uniform_map) {
-      linker_error(prog, "Out of memory during linking.");
+      linker_error(prog, "Out of memory during linking.\n");
       return;
    }
 
@@ -2719,7 +2719,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
        */
       if (first == MESA_SHADER_FRAGMENT) {
          linker_error(prog, "Transform feedback varyings specified, but "
-                      "no vertex or geometry shader is present.");
+                      "no vertex or geometry shader is present.\n");
          goto done;
       }
 
diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
index 9b36a1f..91e457a 100644
--- a/src/glsl/main.cpp
+++ b/src/glsl/main.cpp
@@ -35,6 +35,7 @@
 #include "glsl_parser_extras.h"
 #include "ir_optimization.h"
 #include "program.h"
+#include "program/hash_table.h"
 #include "loop_analysis.h"
 #include "standalone_scaffolding.h"
 
@@ -357,6 +358,11 @@ main(int argc, char **argv)
    assert(whole_program != NULL);
    whole_program->InfoLog = ralloc_strdup(whole_program, "");
 
+   /* Created just to avoid segmentation faults */
+   whole_program->AttributeBindings = new string_to_uint_map;
+   whole_program->FragDataBindings = new string_to_uint_map;
+   whole_program->FragDataIndexBindings = new string_to_uint_map;
+
    for (/* empty */; argc > optind; optind++) {
       whole_program->Shaders =
 	 reralloc(whole_program, whole_program->Shaders,
@@ -415,6 +421,10 @@ main(int argc, char **argv)
    for (unsigned i = 0; i < MESA_SHADER_STAGES; i++)
       ralloc_free(whole_program->_LinkedShaders[i]);
 
+   delete whole_program->AttributeBindings;
+   delete whole_program->FragDataBindings;
+   delete whole_program->FragDataIndexBindings;
+
    ralloc_free(whole_program);
    _mesa_glsl_release_types();
    _mesa_glsl_release_builtin_functions();
-- 
1.9.1



More information about the mesa-dev mailing list