[Mesa-dev] [PATCH] glsl_compiler: Add binding hash tables to avoid SIGSEVs on linking stage
Brian Paul
brianp at vmware.com
Mon Nov 17 08:25:34 PST 2014
Please split up this patch into:
1. gallium comment fixes
2. linker string fixes
3. hash table code changes
-Brian
On 11/17/2014 07:27 AM, Andres Gomez wrote:
> 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();
>
More information about the mesa-dev
mailing list