arb_vertex_program: Verify that unsupported functions generate errors

Submitted by Ian Romanick on Sept. 9, 2015, 11:48 p.m.

Details

Message ID 1441842504-3171-1-git-send-email-idr@freedesktop.org
State New
Headers show

Not browsing as part of any series.

Commit Message

Ian Romanick Sept. 9, 2015, 11:48 p.m.
From: Ian Romanick <ian.d.romanick@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@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

Patch hide | download patch | download mbox

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;
+}

Comments

On 9 September 2015 at 23:48, Ian Romanick <idr@freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick@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@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@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@gmail.com>
Reviewed-by: Albert Freeman <albertwdfreeman@gmail.com>
On 09/10/2015 08:22 AM, Albert Freeman wrote:
> On 9 September 2015 at 23:48, Ian Romanick <idr@freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick@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@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@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)?

That's also necessary because GL_ARB_fragment_program adds some of these
same functions.  If that did happen, the calls to glDeleteProgramsARB
and glIsProgramARB would not generate errors.

> 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'll fix those.  Thanks. :)

> 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.

Calling a function that is part of an extension that is not supported
falls in the "Operation illegal in current state" category.  I believe
it also falls in the undefined behavior category... "possibly including
program termination."  Mesa used to leave the dispatch entries for
unknown functions (leading to segfaults), but now we try to stub in a
dummy function that sets GL_INVALID_OPERATION.

> 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

Very thorough. :)  I verified the signal function by modifying the
ATTEMPT_FUNCTION macro to do

               type proc = (type) (intptr_t) 0xDEADBEEF;

> Whether or not any changes are made to the patch:
> Tested-by: Albert Freeman <albertwdfreeman@gmail.com>
> Reviewed-by: Albert Freeman <albertwdfreeman@gmail.com>
>