Re: [Mesa-dev] [PATCH] glsl_compiler: Add binding hash tables to avoid SIGSEVs on linking stage
On Mon, 2014-11-17 at 09:25 -0700, Brian Paul wrote: Please split up this patch into: 1. gallium comment fixes 2. linker string fixes 3. hash table code changes Sure. I thought it was not worth given the small changes but I suppose it is always better to have different topics in different patches. Coming soon ... Thanks for checking! :) -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl_compiler: Add binding hash tables to avoid SIGSEVs on linking stage
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 n0 requires point output);
+ with n0 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,
+
Re: [Mesa-dev] [PATCH] glsl_compiler: Add binding hash tables to avoid SIGSEVs on linking stage
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 n0 requires point output);
+ with n0 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) {
