[Piglit] [PATCH v2] shaders: Test using a bound program after an unsuccessful relink
Timothy Arceri
tarceri at itsqueeze.com
Sun Nov 5 22:49:11 UTC 2017
On 03/11/17 21:27, Neil Roberts wrote:
> Timothy Arceri <tarceri at itsqueeze.com> writes:
>
>> Thanks for the test! The idea is great, the only problem I see is
>> that the minimum GL version seems too high. This test would not run
>> on the i915 driver which is a problem IMO. Can you lower the
>> requirements?
>
> Good point. Here is a v2 that should work with GL2.0. However I’m not
> exactly sure what is the minimun requirements for i915. I read that
> the default version has been lowered to 1.4. Were you suggesting that
> it should go as low as using the GL_ARB_shader_objects extension? I
> guess there are two i915 drivers as well so I’m not sure which one you
> are referring to.
Your right I forgot it was lowered, however dropping the requirements of
this test is still the right thing to do so thanks for that :)
One small comment further down, otherwise:
Acked-by: Timothy Arceri <tarceri at itsqueeze.com>
>
> Regards,
> - Neil
>
> ---------- >8 ---------- (use git am -c to cut here)
>
> If a bound program is relinked unsuccessfully it is supposed to keep
> the executable and the program state such as the uniforms. This adds a
> test for that which updates a uniform, does a failed relink and
> verifies that a render succeeds with the new uniform value. This is
> currently causing a use-after-free error in Mesa. The test has some
> redundant allocations to try and encourage it to fail which it does
> about 98% of the time.
>
> v2: Adapt to work with GL2.0. Change the order of the colours so that
> a successful run ends with green. Delete the program after use.
> ---
> tests/all.py | 1 +
> tests/shaders/CMakeLists.gl.txt | 1 +
> tests/shaders/unsuccessful-relink.c | 203 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 205 insertions(+)
> create mode 100644 tests/shaders/unsuccessful-relink.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 9e63fa9..ae4a995 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -624,6 +624,7 @@ with profile.test_list.group_manager(PiglitGLTest, 'shaders') as g:
> g(['point-vertex-id', 'gl_InstanceID', 'divisor'])
> g(['point-vertex-id', 'gl_VertexID', 'gl_InstanceID', 'divisor'])
> g(['glsl-vs-int-attrib'])
> + g(['unsuccessful-relink'])
> g(['glsl-link-initializer-03'],
> 'GLSL link two programs, global initializer')
> g(['glsl-getactiveuniform-count',
> diff --git a/tests/shaders/CMakeLists.gl.txt b/tests/shaders/CMakeLists.gl.txt
> index 1c06843..7090d94 100644
> --- a/tests/shaders/CMakeLists.gl.txt
> +++ b/tests/shaders/CMakeLists.gl.txt
> @@ -164,5 +164,6 @@ piglit_add_executable (useshaderprogram-bad-program useshaderprogram-bad-program
> piglit_add_executable (useshaderprogram-flushverts-1 useshaderprogram-flushverts-1.c)
> piglit_add_executable (version-mixing version-mixing.c)
> piglit_add_executable (glsl-vs-int-attrib glsl-vs-int-attrib.c)
> +piglit_add_executable (unsuccessful-relink unsuccessful-relink.c)
>
> # vim: ft=cmake:
> diff --git a/tests/shaders/unsuccessful-relink.c b/tests/shaders/unsuccessful-relink.c
> new file mode 100644
> index 0000000..b652fe9
> --- /dev/null
> +++ b/tests/shaders/unsuccessful-relink.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/**
> + * \file unsuccessful-relink.c
> + *
> + * Render using a program with a uniform. Modify the uniform and then
> + * do a relink that will fail. This shouldn’t affect the original
> + * program and it should render with the new uniform value.
> + *
> + * 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.”
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 20;
> + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char vs_source[] =
> + "attribute vec4 piglit_vertex;\n"
> + "void main()\n"
> + "{\n"
> + " gl_Position = piglit_vertex;\n"
> + "}\n";
> +
> +static const char fs_source[] =
> + "uniform vec4 color;\n"
> + "void main()\n"
> + "{\n"
> + " gl_FragColor = color;\n"
> + "}\n";
> +
> +static const float green[] = { 0.0f, 1.0f, 0.0f, 1.0f };
> +static const float purple[] = { 0.5f, 0.0f, 0.5f, 1.0f };
> +
> +static bool
> +try_render(GLuint vertex_attrib,
> + const float *color)
> +{
> + struct vertex { float x, y; };
> + static const struct vertex verts[] = {
> + { -1, -1 }, { 1, -1 },
> + { -1, 1 }, { 1, 1 }
> + };
> + bool pass;
> + GLuint buf;
> +
> + /* This isn’t using piglit_draw_rect because that tries to
> + * call glGetAttribLocation which won’t work on the unlinked
> + * program
> + */
> +
> + glGenBuffers(1, &buf);
> + glBindBuffer(GL_ARRAY_BUFFER, buf);
> + glBufferData(GL_ARRAY_BUFFER, sizeof verts, verts, GL_STATIC_DRAW);
> + glVertexAttribPointer(vertex_attrib, 2, GL_FLOAT,
> + GL_FALSE, sizeof verts[0],
> + (void *) offsetof(struct vertex, x));
> + glEnableVertexAttribArray(vertex_attrib);
> + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> + glBindBuffer(GL_ARRAY_BUFFER, 0);
> + glDeleteBuffers(1, &buf);
> +
> + if (!piglit_check_gl_error(GL_NO_ERROR)) {
> + fprintf(stderr, "error while drawing\n");
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + pass = piglit_probe_rect_rgb(0, 0,
> + piglit_width, piglit_height,
> + color);
> + if (!pass)
> + fprintf(stderr, "render failed\n");
> +
> + return pass;
> +}
> +
> +static void
> +unsuccessful_link(GLuint prog)
> +{
> + GLuint shader;
> + GLint status;
> + int i;
> +
> + /* Add the fs shader again. This should cause a link error
> + * because there would be two main functions in the fragment
> + * stage. */
> + for (i = 0; i < 2; i++) {
> + shader = piglit_compile_shader_text(GL_FRAGMENT_SHADER,
> + fs_source);
> + glAttachShader(prog, shader);
> + }
> +
> + glLinkProgram(prog);
> +
> + glGetProgramiv(prog, GL_LINK_STATUS, &status);
> +
> + if (status) {
> + fprintf(stderr, "Broken shader unexpectedly linked\n");
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + bool pass = true;
> + GLuint prog;
> + GLint color_uniform;
> + GLint vertex_attrib;
> + void *dummy_data[2048];
> + int i;
> +
> + glViewport(0, 0, piglit_width, piglit_height);
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + prog = piglit_build_simple_program(vs_source, fs_source);
> +
> + color_uniform = glGetUniformLocation(prog, "color");
> +
> + if (color_uniform == -1) {
> + fprintf(stderr, "color uniform missing\n");
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + vertex_attrib = glGetAttribLocation(prog, "piglit_vertex");
> +
> + if (vertex_attrib == -1) {
> + fprintf(stderr, "piglit_vertex attrib missing\n");
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + glUseProgram(prog);
> +
> + glUniform4fv(color_uniform, 1, purple);
> +
> + if (!try_render(vertex_attrib, purple))
> + pass = false;
> +
> + glUniform4fv(color_uniform, 1, green);
> +
> + unsuccessful_link(prog);
> +
> + /* This test currently causes a use-after-free error in Mesa
> + * which causes sporadic failures. In order to increase the
> + * chances of making the test fail, we will do lots of little
> + * redundant allocations in the hope of overwriting the data
> + * allocated by Mesa.
Maybe change this to:
"previously allocated and freed by Mesa"
> + */
> + for (i = 0; i < ARRAY_SIZE(dummy_data); i++) {
> + dummy_data[i] = malloc(64);
> + memset(dummy_data[i], 0, 64);
> + }
> + for (i = 0; i < ARRAY_SIZE(dummy_data); i++)
> + free(dummy_data[i]);
> +
> + if (!try_render(vertex_attrib, green))
> + pass = false;
> +
> + piglit_present_results();
> +
> + glDeleteProgram(prog);
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + piglit_require_gl_version(20);
> + piglit_require_GLSL_version(110);
> +}
>
More information about the Piglit
mailing list