From: Nicolai Hähnle <nicolai.haeh...@amd.com> By splitting glsl_type::mutex into two, we can avoid dropping the hash mutex while creating the new type instance (e.g. struct/record, interface).
This fixes a time-of-check/time-of-use race where two threads would simultaneously attempt to create the same type but end up with different instances of glsl_type. --- src/compiler/glsl_types.cpp | 61 +++++++++++++++++++-------------------------- src/compiler/glsl_types.h | 11 ++++---- 2 files changed, 32 insertions(+), 40 deletions(-) diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp index 0fd1230..063d3f1 100644 --- a/src/compiler/glsl_types.cpp +++ b/src/compiler/glsl_types.cpp @@ -21,21 +21,22 @@ * DEALINGS IN THE SOFTWARE. */ #include <stdio.h> #include "main/macros.h" #include "compiler/glsl/glsl_parser_extras.h" #include "glsl_types.h" #include "util/hash_table.h" -mtx_t glsl_type::mutex = _MTX_INITIALIZER_NP; +mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP; +mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP; hash_table *glsl_type::array_types = NULL; hash_table *glsl_type::record_types = NULL; hash_table *glsl_type::interface_types = NULL; hash_table *glsl_type::function_types = NULL; hash_table *glsl_type::subroutine_types = NULL; void *glsl_type::mem_ctx = NULL; void glsl_type::init_ralloc_type_ctx(void) { @@ -55,160 +56,160 @@ glsl_type::glsl_type(GLenum gl_type, vector_elements(vector_elements), matrix_columns(matrix_columns), length(0) { /* Values of these types must fit in the two bits of * glsl_type::sampled_type. */ STATIC_ASSERT((unsigned(GLSL_TYPE_UINT) & 3) == unsigned(GLSL_TYPE_UINT)); STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) == unsigned(GLSL_TYPE_INT)); STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) == unsigned(GLSL_TYPE_FLOAT)); - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(name != NULL); this->name = ralloc_strdup(this->mem_ctx, name); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); /* Neither dimension is zero or both dimensions are zero. */ assert((vector_elements == 0) == (matrix_columns == 0)); memset(& fields, 0, sizeof(fields)); } glsl_type::glsl_type(GLenum gl_type, glsl_base_type base_type, enum glsl_sampler_dim dim, bool shadow, bool array, unsigned type, const char *name) : gl_type(gl_type), base_type(base_type), sampler_dimensionality(dim), sampler_shadow(shadow), sampler_array(array), sampled_type(type), interface_packing(0), interface_row_major(0), length(0) { - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(name != NULL); this->name = ralloc_strdup(this->mem_ctx, name); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); memset(& fields, 0, sizeof(fields)); matrix_columns = vector_elements = 1; } glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, const char *name) : gl_type(0), base_type(GLSL_TYPE_STRUCT), sampler_dimensionality(0), sampler_shadow(0), sampler_array(0), sampled_type(0), interface_packing(0), interface_row_major(0), vector_elements(0), matrix_columns(0), length(num_fields) { unsigned int i; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(name != NULL); this->name = ralloc_strdup(this->mem_ctx, name); this->fields.structure = ralloc_array(this->mem_ctx, glsl_struct_field, length); for (i = 0; i < length; i++) { this->fields.structure[i] = fields[i]; this->fields.structure[i].name = ralloc_strdup(this->fields.structure, fields[i].name); } - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); } glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, enum glsl_interface_packing packing, bool row_major, const char *name) : gl_type(0), base_type(GLSL_TYPE_INTERFACE), sampler_dimensionality(0), sampler_shadow(0), sampler_array(0), sampled_type(0), interface_packing((unsigned) packing), interface_row_major((unsigned) row_major), vector_elements(0), matrix_columns(0), length(num_fields) { unsigned int i; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(name != NULL); this->name = ralloc_strdup(this->mem_ctx, name); this->fields.structure = rzalloc_array(this->mem_ctx, glsl_struct_field, length); for (i = 0; i < length; i++) { this->fields.structure[i] = fields[i]; this->fields.structure[i].name = ralloc_strdup(this->fields.structure, fields[i].name); } - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); } glsl_type::glsl_type(const glsl_type *return_type, const glsl_function_param *params, unsigned num_params) : gl_type(0), base_type(GLSL_TYPE_FUNCTION), sampler_dimensionality(0), sampler_shadow(0), sampler_array(0), sampled_type(0), interface_packing(0), interface_row_major(0), vector_elements(0), matrix_columns(0), length(num_params) { unsigned int i; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); this->fields.parameters = rzalloc_array(this->mem_ctx, glsl_function_param, num_params + 1); /* We store the return type as the first parameter */ this->fields.parameters[0].type = return_type; this->fields.parameters[0].in = false; this->fields.parameters[0].out = true; /* We store the i'th parameter in slot i+1 */ for (i = 0; i < length; i++) { this->fields.parameters[i + 1].type = params[i].type; this->fields.parameters[i + 1].in = params[i].in; this->fields.parameters[i + 1].out = params[i].out; } - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); } glsl_type::glsl_type(const char *subroutine_name) : gl_type(0), base_type(GLSL_TYPE_SUBROUTINE), sampler_dimensionality(0), sampler_shadow(0), sampler_array(0), sampled_type(0), interface_packing(0), interface_row_major(0), vector_elements(1), matrix_columns(1), length(0) { - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(subroutine_name != NULL); this->name = ralloc_strdup(this->mem_ctx, subroutine_name); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); } bool glsl_type::contains_sampler() const { if (this->is_array()) { return this->fields.array->contains_sampler(); } else if (this->is_record() || this->is_interface()) { for (unsigned int i = 0; i < this->length; i++) { if (this->fields.structure[i].type->contains_sampler()) @@ -453,23 +454,23 @@ glsl_type::glsl_type(const glsl_type *array, unsigned length) : * is represented by the size rather than the type. */ this->gl_type = array->gl_type; /* Allow a maximum of 10 characters for the array size. This is enough * for 32-bits of ~0. The extra 3 are for the '[', ']', and terminating * NUL. */ const unsigned name_length = strlen(array->name) + 10 + 3; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); char *const n = (char *) ralloc_size(this->mem_ctx, name_length); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); if (length == 0) snprintf(n, name_length, "%s[]", array->name); else { /* insert outermost dimensions in the correct spot * otherwise the dimension order will be backwards */ const char *pos = strchr(array->name, '['); if (pos) { int idx = pos - array->name; @@ -875,43 +876,41 @@ const glsl_type * glsl_type::get_array_instance(const glsl_type *base, unsigned array_size) { /* Generate a name using the base type pointer in the key. This is * done because the name of the base type may not be unique across * shaders. For example, two shaders may have different record types * named 'foo'. */ char key[128]; snprintf(key, sizeof(key), "%p[%u]", (void *) base, array_size); - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::hash_mutex); if (array_types == NULL) { array_types = _mesa_hash_table_create(NULL, _mesa_key_hash_string, _mesa_key_string_equal); } const struct hash_entry *entry = _mesa_hash_table_search(array_types, key); if (entry == NULL) { - mtx_unlock(&glsl_type::mutex); const glsl_type *t = new glsl_type(base, array_size); - mtx_lock(&glsl_type::mutex); entry = _mesa_hash_table_insert(array_types, ralloc_strdup(mem_ctx, key), (void *) t); } assert(((glsl_type *) entry->data)->base_type == GLSL_TYPE_ARRAY); assert(((glsl_type *) entry->data)->length == array_size); assert(((glsl_type *) entry->data)->fields.array == base); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::hash_mutex); return (glsl_type *) entry->data; } bool glsl_type::record_compare(const glsl_type *b, bool match_locations) const { if (this->length != b->length) return false; @@ -1033,109 +1032,103 @@ glsl_type::record_key_hash(const void *a) } const glsl_type * glsl_type::get_record_instance(const glsl_struct_field *fields, unsigned num_fields, const char *name) { const glsl_type key(fields, num_fields, name); - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::hash_mutex); if (record_types == NULL) { record_types = _mesa_hash_table_create(NULL, record_key_hash, record_key_compare); } const struct hash_entry *entry = _mesa_hash_table_search(record_types, &key); if (entry == NULL) { - mtx_unlock(&glsl_type::mutex); const glsl_type *t = new glsl_type(fields, num_fields, name); - mtx_lock(&glsl_type::mutex); entry = _mesa_hash_table_insert(record_types, t, (void *) t); } assert(((glsl_type *) entry->data)->base_type == GLSL_TYPE_STRUCT); assert(((glsl_type *) entry->data)->length == num_fields); assert(strcmp(((glsl_type *) entry->data)->name, name) == 0); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::hash_mutex); return (glsl_type *) entry->data; } const glsl_type * glsl_type::get_interface_instance(const glsl_struct_field *fields, unsigned num_fields, enum glsl_interface_packing packing, bool row_major, const char *block_name) { const glsl_type key(fields, num_fields, packing, row_major, block_name); - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::hash_mutex); if (interface_types == NULL) { interface_types = _mesa_hash_table_create(NULL, record_key_hash, record_key_compare); } const struct hash_entry *entry = _mesa_hash_table_search(interface_types, &key); if (entry == NULL) { - mtx_unlock(&glsl_type::mutex); const glsl_type *t = new glsl_type(fields, num_fields, packing, row_major, block_name); - mtx_lock(&glsl_type::mutex); entry = _mesa_hash_table_insert(interface_types, t, (void *) t); } assert(((glsl_type *) entry->data)->base_type == GLSL_TYPE_INTERFACE); assert(((glsl_type *) entry->data)->length == num_fields); assert(strcmp(((glsl_type *) entry->data)->name, block_name) == 0); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::hash_mutex); return (glsl_type *) entry->data; } const glsl_type * glsl_type::get_subroutine_instance(const char *subroutine_name) { const glsl_type key(subroutine_name); - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::hash_mutex); if (subroutine_types == NULL) { subroutine_types = _mesa_hash_table_create(NULL, record_key_hash, record_key_compare); } const struct hash_entry *entry = _mesa_hash_table_search(subroutine_types, &key); if (entry == NULL) { - mtx_unlock(&glsl_type::mutex); const glsl_type *t = new glsl_type(subroutine_name); - mtx_lock(&glsl_type::mutex); entry = _mesa_hash_table_insert(subroutine_types, t, (void *) t); } assert(((glsl_type *) entry->data)->base_type == GLSL_TYPE_SUBROUTINE); assert(strcmp(((glsl_type *) entry->data)->name, subroutine_name) == 0); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::hash_mutex); return (glsl_type *) entry->data; } static bool function_key_compare(const void *a, const void *b) { const glsl_type *const key1 = (glsl_type *) a; const glsl_type *const key2 = (glsl_type *) b; @@ -1156,42 +1149,40 @@ function_key_hash(const void *a) (key->length + 1) * sizeof(*key->fields.parameters)); } const glsl_type * glsl_type::get_function_instance(const glsl_type *return_type, const glsl_function_param *params, unsigned num_params) { const glsl_type key(return_type, params, num_params); - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::hash_mutex); if (function_types == NULL) { function_types = _mesa_hash_table_create(NULL, function_key_hash, function_key_compare); } struct hash_entry *entry = _mesa_hash_table_search(function_types, &key); if (entry == NULL) { - mtx_unlock(&glsl_type::mutex); const glsl_type *t = new glsl_type(return_type, params, num_params); - mtx_lock(&glsl_type::mutex); entry = _mesa_hash_table_insert(function_types, t, (void *) t); } const glsl_type *t = (const glsl_type *)entry->data; assert(t->base_type == GLSL_TYPE_FUNCTION); assert(t->length == num_params); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::hash_mutex); return t; } const glsl_type * glsl_type::get_mul_type(const glsl_type *type_a, const glsl_type *type_b) { if (type_a == type_b) { return type_a; diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h index 9da0cbc..55faac2 100644 --- a/src/compiler/glsl_types.h +++ b/src/compiler/glsl_types.h @@ -142,42 +142,42 @@ struct glsl_type { * GLSL_TYPE_FLOAT, \c GLSL_TYPE_INT, * and \c GLSL_TYPE_UINT are valid. */ unsigned interface_packing:2; unsigned interface_row_major:1; /* Callers of this ralloc-based new need not call delete. It's * easier to just ralloc_free 'mem_ctx' (or any of its ancestors). */ static void* operator new(size_t size) { - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); /* mem_ctx should have been created by the static members */ assert(glsl_type::mem_ctx != NULL); void *type; type = ralloc_size(glsl_type::mem_ctx, size); assert(type != NULL); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); return type; } /* If the user *does* call delete, that's OK, we will just * ralloc_free in that case. */ static void operator delete(void *type) { - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); ralloc_free(type); - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); } /** * \name Vector and matrix element counts * * For scalars, each of these values will be 1. For non-numeric types * these will be 0. */ /*@{*/ uint8_t vector_elements; /**< 1, 2, 3, or 4 vector elements. */ @@ -813,21 +813,22 @@ struct glsl_type { /** * Check if the type interface is row major */ bool get_interface_row_major() const { return (bool) interface_row_major; } private: - static mtx_t mutex; + static mtx_t mem_mutex; + static mtx_t hash_mutex; /** * ralloc context for all glsl_type allocations * * Set on the first call to \c glsl_type::new. */ static void *mem_ctx; void init_ralloc_type_ctx(void); -- 2.9.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev