[Piglit] [PATCH] arb_vertex_program: Verify that unsupported functions generate errors
Albert Freeman
albertwdfreeman at gmail.com
Thu Sep 10 08:22:34 PDT 2015
On 9 September 2015 at 23:48, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Once upon a time, Mesa accidentally enable GL_ARB_vertex_program in Core
> Profile. We fixed that, but continued to export the functions. We
> eventually fixed that too. It turns out that, especially in the piglit
> framework, it's really hard to verify that a function is not there.
>
> The piglit framework will detect that a function is being called that is
> not supported by the implementation, and it will print a friendly
> message. We don't want that. We want to call the function specifically
> because we know it's not supported. As a result, glXGetProcAddress has
> to be used directly.
>
> Using glXGetProcAddress poses two problems. First, it can either return
> NULL or it can return a pointer to a dispatch trampoile function. Some
> closed-source libGL implementations will return NULL for functions that
> they and the associated drivers have never heard of. The open-source
> libGL implementations will never return NULL because the application
> might later load a new driver that supports the requested function. Try
> glXGetProcAddress((const GLubyte *)"glHamsandwich") on your favorite
> libGL.
>
> Second, the trampoline function may or may not call a valid function.
> Depending on the driver and the libGL the dispatch table entry used by
> the trampoline function may:
>
> 1. Point at garbage.
>
> 2. Point at a default function that always sets GL_INVALID_OPERATION.
>
> 3. Point at a real implementation of the function that may or may not
> set GL_INVALID_OPERATION.
>
> The only way to determine which you've got is by calling the trampoline
> function. In case #1, this will result in some signal (probably either
> SIGSEGV or SIGLL) that leads to program termination. By the definition
> of the GL spec, this is actually success, so crashing is not a good way
> to reflect that. To deal with this a combination of signal and
> sigsetjmp are used.
>
> For each function to be tested,
>
> 1. Call glXGetProcAddress on the function. If glXGetProcAddress returns
> NULL, the function passes.
>
> 2. Set signal handlers for every reasonable signal that calling outer
> space might generate. The signal handler will siglongjmp back to the
> caller.
>
> 3. Use sigsetjmp to set the location to which the signal handler should
> return.
>
> 4. Call the function. If GL_INVALID_OPERATION is set, the function
> passes. If the signal handler is invoked, the call to glGetError will
> never happen. If any other condition is set, the function fails.
>
> This particular test only calls a subset of the GL_ARB_vertex_program
> functions. The remaining functions require that a valid vertex program
> be bound. If all the functions included in this test are successful,
> either the functions would be too (with possible false positives) or an
> api-errors test should detect their failures. Either way, I didn't see
> much point in trying them.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
> tests/all.py | 1 +
> tests/spec/arb_vertex_program/CMakeLists.gl.txt | 4 +
> tests/spec/arb_vertex_program/unsupported.c | 111 ++++++++++++++++++++++++
> 3 files changed, 116 insertions(+)
> create mode 100644 tests/spec/arb_vertex_program/unsupported.c
>
> diff --git a/tests/all.py b/tests/all.py
> index a1eba91..a44ceb8 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -2643,6 +2643,7 @@ with profile.group_manager(
> g(['vp-address-04'], run_concurrent=False)
> g(['vp-bad-program'], run_concurrent=False)
> g(['vp-max-array'], run_concurrent=False)
> + g(['arb_vertex_program-unsupported'], 'unsupported')
>
> with profile.group_manager(
> PiglitGLTest,
> diff --git a/tests/spec/arb_vertex_program/CMakeLists.gl.txt b/tests/spec/arb_vertex_program/CMakeLists.gl.txt
> index 197b20e..fb3339a 100644
> --- a/tests/spec/arb_vertex_program/CMakeLists.gl.txt
> +++ b/tests/spec/arb_vertex_program/CMakeLists.gl.txt
> @@ -9,6 +9,10 @@ link_libraries (
> ${OPENGL_glu_LIBRARY}
> )
>
> +IF(PIGLIT_BUILD_GLX_TESTS)
> +piglit_add_executable (arb_vertex_program-unsupported unsupported.c)
> +ENDIF(PIGLIT_BUILD_GLX_TESTS)
> +
> piglit_add_executable (arb_vertex_program-getenv4d-with-error getenv4d-with-error.c)
> piglit_add_executable (arb_vertex_program-getlocal4d-with-error getlocal4d-with-error.c)
> piglit_add_executable (arb_vertex_program-getlocal4f-max getlocal4f-max.c)
> diff --git a/tests/spec/arb_vertex_program/unsupported.c b/tests/spec/arb_vertex_program/unsupported.c
> new file mode 100644
> index 0000000..f2cf039
> --- /dev/null
> +++ b/tests/spec/arb_vertex_program/unsupported.c
> @@ -0,0 +1,111 @@
> +/*
> + * Copyright © 2015 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 unsupported.c
> + * Verify that GL_INVALID_OPERATION is generated in core profile context that
> + * doesn't advertise the GL_ARB_vertex_program extension.
> + */
> +#include "piglit-util-gl.h"
> +#include <GL/glx.h>
> +#include <signal.h>
> +#include <setjmp.h>
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_core_version = 31;
> + config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char source[] =
> + "!!ARBvp1.0\n"
> + "MOV result.position, vertex.position;\n"
> + "END\n"
> + ;
> +
> +static sigjmp_buf env;
> +
> +static void
> +handler(int s)
> +{
> + siglongjmp(env, s);
> +}
> +
> +#define ATTEMPT_FUNCTION(type, name, parameters) \
> + do { \
> + type proc = (type) \
> + glXGetProcAddress((const GLubyte *) # name ); \
> + \
> + printf("%s...\n", # name); \
> + if (proc == NULL) { \
> + printf(" GetProcAddress returned NULL.\n"); \
> + } else { \
> + int s = sigsetjmp(env, true); \
> + if (s == 0) { \
> + printf(" Calling...\n"); \
> + proc parameters ; \
> + pass = piglit_check_gl_error(GL_INVALID_OPERATION) \
> + && pass; \
> + } else { \
> + printf(" Got signal %d.\n", s); \
> + } \
> + } \
> + } while (false)
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + bool pass = true;
> + GLuint prog = 0;
> +
> + piglit_require_not_extension("GL_ARB_vertex_program");
> + piglit_require_not_extension("GL_ARB_fragment_program");
> +
> + signal(SIGSEGV, handler);
> + signal(SIGILL, handler);
> + signal(SIGFPE, handler);
> + signal(SIGBUS, handler);
> +
> + ATTEMPT_FUNCTION(PFNGLGENPROGRAMSARBPROC, glGenProgramsARB,
> + (1, &prog));
> + ATTEMPT_FUNCTION(PFNGLBINDPROGRAMARBPROC, glBindProgramARB,
> + (GL_VERTEX_PROGRAM_ARB, 1));
> + ATTEMPT_FUNCTION(PFNGLPROGRAMSTRINGARBPROC, glProgramStringARB,
> + (GL_VERTEX_PROGRAM_ARB,
> + GL_PROGRAM_FORMAT_ASCII_ARB,
> + strlen(source),
> + source));
> + ATTEMPT_FUNCTION(PFNGLDELETEPROGRAMSARBPROC, glDeleteProgramsARB,
> + (1, &prog));
> + ATTEMPT_FUNCTION(PFNGLISPROGRAMARBPROC, glIsProgramARB,
> + (prog));
> +
> + piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + return PIGLIT_FAIL;
> +}
> --
> 2.1.0
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
The test passes on my r600-evergreen (with the functions all returning
GL_INVALID_OPERATION).
"piglit_require_not_extension("GL_ARB_fragment_program");":
Wouldn't that cause a problem if the driver supports
GL_ARB_fragment_program and not GL_ARB_vertex_program (although its
not like that is going to happen)?
And if spelling/grammer issues matter:
In the code you used "SIGILL" while in the commit message you said "SIGLL".
7th word into the commit message: "enable" is not "enabled".
I find it weird how the specification (GL 4.5 core and compatibility)
only seems to mention GL_INVALID_OPERATION with relation to missing
functions when the functions have been removed from the specification.
Although it does classify it as "Operation illegal in current state",
so maybe that is enough.
Because this is my first review of a patch: I hacked glx and tested
the tests under:
NULL returned to glxGetProcAddress PASSED
A signal from a test function returned by glxGetProcAddress PASSED
0 returned from that test function FAILED (like it is supposed to)
Normal behavior of mesa PASSED
Whether or not any changes are made to the patch:
Tested-by: Albert Freeman <albertwdfreeman at gmail.com>
Reviewed-by: Albert Freeman <albertwdfreeman at gmail.com>
More information about the Piglit
mailing list