Hi Ian, On 17 December 2015 at 02:40, Ian Romanick <i...@freedesktop.org> wrote: > From: Ian Romanick <ian.d.roman...@intel.com> > > See the giant comment in the top of > tests/general/object-namespace-pollution.c for more details. Currently > only buffer objects and texture objects are supported with glClear and > glGenerateMipmap. Addtional objects and operations will be added. > > Both glClear and glGenerateMipmap fail with buffer objects in Mesa > 11.0.6 and earlier. These texture object tests pass. > > Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> > Cc: Emil Velikov <emil.veli...@collabora.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92363 > --- > > I have tried to send this out a couple times, but git-send-mail keeps > failing in a way that I don't know how to debug. > > There will be more subtests here. I want to send the framework out for > review sooner rather than later. > > I do not like the first_unused_texture hack. Comments in the code > explain why it is necessary. I believe this will need to be extended to > support framebuffer objects as well. An alternative approach is to > modify the piglit infrastructure to not use glGenTextures or > glGenFramebuffers with -fbo on non-core profile. I'm not convinced > that's better. > > tests/all.py | 8 + > tests/general/CMakeLists.gl.txt | 1 + > tests/general/object-namespace-pollution.c | 603 > +++++++++++++++++++++++++++++ > 3 files changed, 612 insertions(+) > create mode 100644 tests/general/object-namespace-pollution.c > > diff --git a/tests/all.py b/tests/all.py > index e3719ee..cfd298e 100644 > --- a/tests/all.py > +++ b/tests/all.py > @@ -4579,5 +4579,13 @@ with profile.group_manager( > for sample_count in (str(x) for x in MSAA_SAMPLE_COUNTS): > g(['ext_shader_samples_identical', sample_count]) > > +with profile.group_manager( > + PiglitGLTest, > + grouptools.join('object namespace pollution')) as g: > + for object_type in ("buffer", "texture"): > + for operation in ("glClear", "glGenerateMipmap"): > + g(['object-namespace-pollution', operation, object_type], > + '{} with {}'.format(object_type, operation)) > + > if platform.system() is 'Windows': > profile.filter_tests(lambda p, _: not p.startswith('glx')) > diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt > index 298f59c..4b59d1f 100644 > --- a/tests/general/CMakeLists.gl.txt > +++ b/tests/general/CMakeLists.gl.txt > @@ -76,6 +76,7 @@ piglit_add_executable (line-flat-clip-color > line-flat-clip-color.c) > piglit_add_executable (lineloop lineloop.c) > piglit_add_executable (longprim longprim.c) > piglit_add_executable (masked-clear masked-clear.c) > +piglit_add_executable (object-namespace-pollution > object-namespace-pollution.c) > piglit_add_executable (pos-array pos-array.c) > piglit_add_executable (pbo-drawpixels pbo-drawpixels.c) > piglit_add_executable (pbo-read-argb8888 pbo-read-argb8888.c) > diff --git a/tests/general/object-namespace-pollution.c > b/tests/general/object-namespace-pollution.c > new file mode 100644 > index 0000000..1d8859e > --- /dev/null > +++ b/tests/general/object-namespace-pollution.c > @@ -0,0 +1,603 @@ > +/* > + * 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 object-namespace-pollution.c > + * Verify that the GL implementation does not pollute the object namespace. > + * > + * At least through Mesa 11.1.0, Mesa drivers that use "meta" have some > + * problems with respect to the OpenGL object namespace. Many places inside > + * meta allocate objects using a mechanism similar to \c glGen*. This poses > + * serious problems for applications that create objects without using the > + * associated \c glGen* function (so called "user generated names"). > + * > + * Section 3.8.12 (Texture Objects) of the OpenGL 2.1 (May 16, 2008) spec > + * says: > + * > + * "The command > + * > + * void GenTextures( sizei n, uint *textures ); > + * > + * returns n previously unused texture object names in textures. These > + * names are marked as used, for the purposes of GenTextures only, but > + * they acquire texture state and a dimensionality only when they are > + * first bound, just as if they were unused." > + * > + * Calling glBindTexture on an unused name makes that name be used. An > + * application can mix user generated names and GL generated names only if it > + * is careful not to reuse names that were previously returned by > + * glGenTextures. In practice this means that all user generated names must > + * be used (i.e., bound) before calling glGenTextures. > + * > + * This effectively means that the GL implementation (or, realistically, GL > + * middleware) is \b never allowed to use glGenTextures because the > + * application cannot know what names were returned. > + * > + * This applies to most kinds of GL objects. > + * > + * - buffers > + * - textures > + * - samplers > + * - framebuffers > + * - renderbuffers > + * - queries > + * - vertex programs (from GL_ARB_vertex_program) > + * - fragment programs (from GL_ARB_fragment_program) > + * - vertex arrays (from GL_APPLE_vertex_array_object) > + * - fragment shaders (from GL_ATI_fragment_shader) > + * > + * Many object types (ARB vertex array objects, transform feedback objects, > + * program pipeline objects, GLSL shader / program objects, Intel performance > + * query objects, etc.) forbid user generated names. > + * > + * Some object types (NVIDIA or APPLE fences, EXT vertex shaders, NVIDIA > + * transform feedback objects) could probably also suffer from this problem. > + * However, Mesa does not support these objects, so we don't need to test > + * them. > + * > + * GL_AMD_performance_monitor does not specify whether or not user generated > + * names are allowed. > + * > + * This test attempts to observe this kind of invalid behavior. First a GL > + * operation is performed that may need to create an object. Then the test > + * creates several objects with user generated names. Finally, the GL > + * operations are performed again. If the GL implemented generated new names > + * for the purpose of the operations, those names will likely conflict with > + * one of the user generated names. This should be observable in one of > three > + * ways. > + * > + * - When the test tries to create the object, the object will already exist. > + * This is detected by the \c glIs* functions. > + * > + * - After the second call to the GL operation, the application's object will > + * have modified state. > + * > + * - The second call to the GL operation will fail to perform correctly > + * because the application modified its data. > + * > + * Only the first two methods are employed by this test. This should catch > + * the vast majority of possible failures. Verifying the correctness of the > + * GL operation would add a lot of complexity to the test. > + */ > + > +#include "piglit-util-gl.h" > + > +PIGLIT_GL_TEST_CONFIG_BEGIN > + > + config.supports_gl_compat_version = 12; > + Do we want to have these tests in GLES flavour ? Can send a trivial patch that does so.
> +PIGLIT_GL_TEST_CONFIG_END > + > +struct enum_value_pair { > + GLenum value; > + GLint expected; > +}; > + > +/** > + * Spare objects used by test cases. > + * > + * Some tests need to use objects for the GL operation begin tested. For > + * example, the \c glGenerateMipmap test needs a texture. These objects > + * cannot be created using \c glGen* because that would conflict with the > rest > + * of the test. Instead statically allocate object names starting with some > + * high number that we hope the GL won't use or generate during the test. > + */ > +#define FIRST_SPARE_OBJECT 600 > + > +/** > + * Linear feedback shift register random number generator > + * > + * Simple Galois LFSRs that is loosely based on > + * https://en.wikipedia.org/wiki/Linear_feedback_shift_register > + * > + * The value of \c state is updated to reflect the new state of the LFSR. > + * This new value should be passed in for the next iteration. > + * > + * \return > + * Either 0 or 1 based on the incoming value of \c state. > + */ > +static unsigned > +lfsr(uint16_t *state) > +{ > + unsigned output = *state & 1; > + > + *state = (*state >> 1) ^ (~output & 0x0000B400); > + > + return output; > +} > + > +/** > + * Fill some memory with pseudorandom values > + * > + * Using two seed values, a pair of LFSRs are used to generate pseudorandom > + * values to fill the specified memory buffer. Seprarate invocations with > + * identical \c bytes, \c seed1, and \c seed2 parameters will generate > + * identical data. This can be used generate data to initialize a buffer and > + * regenerate the same data to validate the buffer. > + */ > +static void > +generate_random_data(void *_output, size_t bytes, uint16_t seed1, > + uint16_t seed2) > +{ > + uint8_t *output = (uint8_t *) _output; > + > + for (unsigned i = 0; i < bytes; i++) { > + uint8_t byte = 0; > + > + for (unsigned bit = 0; bit < 8; bit++) { > + byte <<= 1; > + byte |= lfsr(&seed1) ^ lfsr(&seed2); > + } > + > + output[i] = byte; > + } > +} > + [side note] Can imagine that others might will use of these. We can get those in piglit util at a later point. > +/** \name Methods for operating on buffer objects */ > +/*@{*/ > +#define BUFFER_DATA_SIZE 1024 > + > +static bool > +create_buffer(unsigned name, bool silent_skip) > +{ > + uint8_t data[BUFFER_DATA_SIZE]; > + > + if (!piglit_is_extension_supported("GL_ARB_vertex_buffer_object") && > + piglit_get_gl_version() < 14) { > + if (silent_skip) > + return true; > + > + printf("%s requires vertex buffer objects.\n", __func__); > + piglit_report_result(PIGLIT_SKIP); > + } > + > + generate_random_data(data, sizeof(data), GL_ARRAY_BUFFER, name); > + > + if (glIsBuffer(name)) { > + printf("\t%s,%d: %u is already a buffer\n", > + __func__, __LINE__, name); > + return false; > + } > + > + glBindBuffer(GL_ARRAY_BUFFER, name); > + glBufferData(GL_ARRAY_BUFFER, sizeof(data), data, GL_STATIC_DRAW); > + glBindBuffer(GL_ARRAY_BUFFER, 0); > + > + return piglit_check_gl_error(GL_NO_ERROR); > +} > + > +static bool > +validate_buffer(unsigned name, bool silent_skip) > +{ > + static const struct enum_value_pair test_vectors[] = { > + { GL_BUFFER_SIZE, BUFFER_DATA_SIZE }, > + { GL_BUFFER_USAGE, GL_STATIC_DRAW }, > + { GL_BUFFER_ACCESS, GL_READ_WRITE }, > + { GL_BUFFER_MAPPED, GL_FALSE }, > + }; > + bool pass = true; > + uint8_t data[BUFFER_DATA_SIZE]; > + void *map; > + > + if (!piglit_is_extension_supported("GL_ARB_vertex_buffer_object") && > + piglit_get_gl_version() < 14) { > + if (silent_skip) > + return true; > + > + printf("%s requires vertex buffer objects.\n", __func__); > + piglit_report_result(PIGLIT_SKIP); > + } Unless we expect silent_skip=true in setup and false here we can drop this hunk. > + > + if (!glIsBuffer(name)) { > + printf("\t%s,%d: %u is not a buffer\n", > + __func__, __LINE__, name); > + return false; > + } > + > + glBindBuffer(GL_ARRAY_BUFFER, name); > + > + for (unsigned i = 0; i < ARRAY_SIZE(test_vectors); i++) { > + GLint got; > + > + glGetBufferParameteriv(GL_ARRAY_BUFFER, > + test_vectors[i].value, > + &got); > + > + if (got != test_vectors[i].expected) { > + printf("\t%s,%d: %s of %u: got 0x%x, expected 0x%x\n", > + __func__, __LINE__, > + piglit_get_gl_enum_name(test_vectors[i].value), > + name, got, test_vectors[i].expected); > + pass = false; > + } > + } > + > + generate_random_data(data, sizeof(data), GL_ARRAY_BUFFER, name); > + map = glMapBuffer(GL_ARRAY_BUFFER, GL_READ_ONLY); > + if (map != NULL) { > + if (memcmp(map, data, sizeof(data)) != 0) { > + printf("\t%s,%d: Data mismatch in %u\n", > + __func__, __LINE__, name); > + pass = false; > + } > + } else { > + printf("\t%s,%d: Unable to map %u\n", > + __func__, __LINE__, name); > + pass = false; > + } > + > + glUnmapBuffer(GL_ARRAY_BUFFER); > + > + pass = piglit_check_gl_error(GL_NO_ERROR) && pass; > + > + return pass; > +} > +/*@}*/ > + > +/** \name Methods for operating on texture objects */ > +/*@{*/ > +#define TEXTURE_DATA_SIZE (16 * 16 * sizeof(GLuint)) > + > +static bool > +create_texture(unsigned name, bool silent_skip) > +{ > + uint8_t data[TEXTURE_DATA_SIZE]; > + > + /* Texture objects are always supported, so there is no opportunity to > + * skip. > + */ > + (void) silent_skip; > + > + generate_random_data(data, sizeof(data), GL_TEXTURE_2D, name); > + > + if (glIsTexture(name)) { > + printf("\t%s,%d: %u is already a texture\n", > + __func__, __LINE__, name); > + return false; > + } > + > + glBindTexture(GL_TEXTURE_2D, name); > + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 16, 16, 0, GL_RGBA, > + GL_UNSIGNED_INT_8_8_8_8, data); > + glBindTexture(GL_TEXTURE_2D, 0); > + > + return piglit_check_gl_error(GL_NO_ERROR); > +} > + > +static bool > +validate_texture(unsigned name, bool silent_skip) > +{ > + static const struct enum_value_pair tex_test_vectors[] = { > + { GL_TEXTURE_WRAP_S, GL_REPEAT }, > + { GL_TEXTURE_WRAP_T, GL_REPEAT }, > + { GL_TEXTURE_WRAP_R, GL_REPEAT }, > + { GL_TEXTURE_MIN_FILTER, GL_NEAREST_MIPMAP_LINEAR }, > + { GL_TEXTURE_MAG_FILTER, GL_LINEAR }, > + { GL_TEXTURE_BASE_LEVEL, 0 }, > + { GL_TEXTURE_MAX_LEVEL, 1000 }, > + }; > + static const struct enum_value_pair tex_level_test_vectors[] = { > + { GL_TEXTURE_WIDTH, 16 }, > + { GL_TEXTURE_HEIGHT, 16 }, > + { GL_TEXTURE_INTERNAL_FORMAT, GL_RGBA8 }, > + }; > + bool pass = true; > + uint8_t data[TEXTURE_DATA_SIZE]; > + uint8_t texels[TEXTURE_DATA_SIZE]; > + > + /* Texture objects are always supported, so there is no opportunity to > + * skip. > + */ > + (void) silent_skip; > + > + if (!glIsTexture(name)) { > + printf("\t%s,%d: %u is not a texture\n", > + __func__, __LINE__, name); > + return false; > + } > + > + glBindTexture(GL_TEXTURE_2D, name); > + > + for (unsigned i = 0; i < ARRAY_SIZE(tex_test_vectors); i++) { > + GLint got; > + > + glGetTexParameteriv(GL_TEXTURE_2D, > + tex_test_vectors[i].value, > + &got); > + > + if (got != tex_test_vectors[i].expected) { > + printf("\t%s,%d: %s of %u: got 0x%x, expected 0x%x\n", > + __func__, __LINE__, > + > piglit_get_gl_enum_name(tex_test_vectors[i].value), > + name, got, tex_test_vectors[i].expected); > + pass = false; > + } > + } > + > + for (unsigned i = 0; i < ARRAY_SIZE(tex_level_test_vectors); i++) { > + GLint got; > + > + glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, > + tex_level_test_vectors[i].value, > + &got); > + > + if (got != tex_level_test_vectors[i].expected) { > + printf("\t%s,%d: %s of %u: got 0x%x, expected 0x%x\n", > + __func__, __LINE__, > + > piglit_get_gl_enum_name(tex_level_test_vectors[i].value), > + name, got, tex_level_test_vectors[i].expected); > + pass = false; > + } > + } > + > + /* Try to use glGetnTexImageARB. If the test's 16x16 texture was > + * replaced with something larger, the call to glGetTexImage will > + * probably segfault. This could be worked around, but it doesn't > + * seem worth it. > + * > + * If the texture size did change, the glGetTexLevelParameteriv loop > + * above will have already detected it. > + */ > + generate_random_data(data, sizeof(data), GL_TEXTURE_2D, name); > + if (piglit_is_extension_supported("GL_ARB_robustness")) { > + /* Note: if sizeof(texels) is less than the size of the image, > + * glGetnTexImageARB will generate GL_INVALID_OPERATION. > + */ > + glGetnTexImageARB(GL_TEXTURE_2D, 0, GL_RGBA, > + GL_UNSIGNED_INT_8_8_8_8, > + sizeof(texels), texels); > + } else { > + glGetTexImage(GL_TEXTURE_2D, 0, GL_RGBA, > + GL_UNSIGNED_INT_8_8_8_8, > + texels); > + } > + > + if (memcmp(texels, data, sizeof(data)) != 0) { > + printf("\t%s,%d: Data mismatch in %u\n", > + __func__, __LINE__, name); > + pass = false; > + } > + > + glBindTexture(GL_TEXTURE_2D, 0); > + > + return piglit_check_gl_error(GL_NO_ERROR) && pass; > +} > +/*@}*/ > + > +/** \name GL operation wrapper functions. */ > +/*@{*/ > +static bool > +do_Clear(bool silent_skip) > +{ > + /* glClear is always supported, so there is no opportunity to skip. */ > + (void) silent_skip; > + > + glClear(GL_COLOR_BUFFER_BIT); > + return piglit_check_gl_error(GL_NO_ERROR); > +} > + > +static bool > +do_GenerateMipmap(bool silent_skip) > +{ > + const GLuint tex = FIRST_SPARE_OBJECT; > + bool pass = true; > + > + if (!piglit_is_extension_supported("GL_EXT_framebuffer_object") && > + !piglit_is_extension_supported("GL_ARB_framebuffer_object") && > + piglit_get_gl_version() < 30) { > + if (silent_skip) > + return true; > + > + printf("%s requires framebuffer objects.\n", __func__); > + piglit_report_result(PIGLIT_SKIP); > + } > + > + if (glIsTexture(tex)) { > + printf("\t%s,%d: %u is already a texture\n", > + __func__, __LINE__, tex); > + pass = false; > + } > + > + glBindTexture(GL_TEXTURE_2D, tex); > + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 16, 16, 0, GL_RGBA, > + GL_UNSIGNED_INT_8_8_8_8, NULL); > + glGenerateMipmap(GL_TEXTURE_2D); > + > + glBindTexture(GL_TEXTURE_2D, 0); > + glDeleteTextures(1, &tex); > + > + return piglit_check_gl_error(GL_NO_ERROR) && pass; > +} > +/*@}*/ > + > +static const struct { > + const char *name; > + bool (*func)(bool silent_skip); > +} operations[] = { > + { "glClear", do_Clear }, > + { "glGenerateMipmap", do_GenerateMipmap } > +}; > + > +static const struct { > + const char *name; > + bool (*create)(unsigned name, bool silent_skip); > + bool (*validate)(unsigned name, bool silent_skip); > +} object_types[] = { > + { "buffer", create_buffer, validate_buffer }, > + { "texture", create_texture, validate_texture }, > +}; > + > +static void nit: annotate as NORETURN > +usage(const char *prog) > +{ > + unsigned i; > + > + printf("Usage is one of:\n"); > + printf("\t%s\n", prog); > + printf("\t%s operation object-type\n\n", prog); > + printf("Where operation is one of:\n"); > + > + for (i = 0; i < ARRAY_SIZE(operations); i++) > + printf("\t%s\n", operations[i].name); > + > + printf("\nAnd object-type is one of:\n"); > + > + for (i = 0; i < ARRAY_SIZE(object_types); i++) > + printf("\t%s\n", object_types[i].name); > + > + piglit_report_result(PIGLIT_FAIL); > +} > + > +void > +piglit_init(int argc, char **argv) > +{ > + bool pass = true; > + unsigned i; > + unsigned first_object_type = 0; > + unsigned last_object_type = ARRAY_SIZE(object_types) - 1; > + unsigned first_operation = 0; > + unsigned last_operation = ARRAY_SIZE(operations) - 1; > + bool silent = true; > + unsigned first_unused_texture; > + > + /* The test is either invoked with no command line parameters (in > + * which case all test combinations are run) or with an operation name > + * and an object type (in which case a single test combination is > + * run). > + */ > + if (argc != 1 && argc != 3) > + usage(argv[0]); > + > + if (argc == 3) { > + silent = false; > + > + for (i = 0; i < ARRAY_SIZE(operations); i++) { > + if (strcmp(argv[1], operations[i].name) == 0) { > + first_operation = i; > + last_operation = i; > + break; > + } > + } > + > + if (i == ARRAY_SIZE(operations)) > + usage(argv[0]); > + > + for (i = 0; i < ARRAY_SIZE(object_types); i++) { > + if (strcmp(argv[2], object_types[i].name) == 0) { > + first_object_type = i; > + last_object_type = i; > + break; > + } > + } > + Fwiw one could replace these first/last with a struct pointers for the operation and object. > + if (i == ARRAY_SIZE(object_types)) > + usage(argv[0]); > + > + printf("Single test case %s with %s\n", > + object_types[first_object_type].name, > + operations[first_operation].name); > + } > + > + /* This is a bit ugly, but it is necessary. When a test is run with > + * -fbo, the piglit framework will create some textures before calling > + * piglit_init. These textures will likely have names that conflict > + * with the names that are used by this test, so the test should avoid > + * them. > + * > + * HOWEVER, the test should only avoid lower numbered textures. If > + * the piglit framework created a texture named 1, the test should > + * still try to use a buffer object named 1. > + */ > + for (first_unused_texture = 1; > + first_unused_texture < 16; > + first_unused_texture++) { > + if (!glIsTexture(first_unused_texture)) > + break; > + } > + > + if (first_unused_texture >= 16) > + piglit_report_result(PIGLIT_FAIL); > + > + for (i = first_operation; i <= last_operation; i++) > + pass = operations[i].func(silent) && pass; > + > + pass = piglit_check_gl_error(GL_NO_ERROR) && pass; > + > + for (i = first_object_type; i <= last_object_type; i++) { > + for (unsigned name = 1; name < 16; name++) { IMHO here comes the tricky bit(s) - 16 does not always cover all the names for every time mentioned in the comment above. I'd imagine that we want to check them all ? To handle this one could just fold the loop within the create/validate functions. To avoid rechecking things in validate (and even pass the set values to validate) one can use void **state between create and validate. - some object types do not allow binding an object which name did not come from the respective glGen* function. Such as samplers, framebuffer... I'm a bit short on workarounds for this one, considering (if I understand correctly) the idea is to avoid glGen at setup/create stage. -Emil _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit