From d6ec0aa7edfbe1c86861a4643b6b095a243d24ad Mon Sep 17 00:00:00 2001 From: =?utf8?q?Nicolai=20H=C3=A4hnle?= Date: Fri, 12 May 2017 12:37:00 +0200 Subject: [PATCH] glsl: fix a race condition when inserting new types 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. Reviewed-by: Timothy Arceri --- 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 0fd12305b7d..063d3f1edc2 100644 --- a/src/compiler/glsl_types.cpp +++ b/src/compiler/glsl_types.cpp @@ -28,7 +28,8 @@ #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; @@ -62,13 +63,13 @@ glsl_type::glsl_type(GLenum gl_type, 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. */ @@ -85,13 +86,13 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type base_type, 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)); @@ -109,7 +110,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, { unsigned int i; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(name != NULL); @@ -123,7 +124,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, 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, @@ -139,7 +140,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, { unsigned int i; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); assert(name != NULL); @@ -152,7 +153,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, fields[i].name); } - mtx_unlock(&glsl_type::mutex); + mtx_unlock(&glsl_type::mem_mutex); } glsl_type::glsl_type(const glsl_type *return_type, @@ -166,7 +167,7 @@ glsl_type::glsl_type(const glsl_type *return_type, { unsigned int i; - mtx_lock(&glsl_type::mutex); + mtx_lock(&glsl_type::mem_mutex); init_ralloc_type_ctx(); @@ -185,7 +186,7 @@ glsl_type::glsl_type(const glsl_type *return_type, 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) : @@ -196,12 +197,12 @@ glsl_type::glsl_type(const char *subroutine_name) : 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 @@ -460,9 +461,9 @@ glsl_type::glsl_type(const glsl_type *array, unsigned length) : */ 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); @@ -882,7 +883,7 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size) 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, @@ -891,9 +892,7 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size) 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), @@ -904,7 +903,7 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size) 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; } @@ -1040,7 +1039,7 @@ glsl_type::get_record_instance(const glsl_struct_field *fields, { 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, @@ -1050,9 +1049,7 @@ glsl_type::get_record_instance(const glsl_struct_field *fields, 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); } @@ -1061,7 +1058,7 @@ glsl_type::get_record_instance(const glsl_struct_field *fields, 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; } @@ -1076,7 +1073,7 @@ glsl_type::get_interface_instance(const glsl_struct_field *fields, { 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, @@ -1086,10 +1083,8 @@ glsl_type::get_interface_instance(const glsl_struct_field *fields, 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); } @@ -1098,7 +1093,7 @@ glsl_type::get_interface_instance(const glsl_struct_field *fields, 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; } @@ -1108,7 +1103,7 @@ 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, @@ -1118,9 +1113,7 @@ glsl_type::get_subroutine_instance(const char *subroutine_name) 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); } @@ -1128,7 +1121,7 @@ glsl_type::get_subroutine_instance(const char *subroutine_name) 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; } @@ -1163,7 +1156,7 @@ glsl_type::get_function_instance(const glsl_type *return_type, { 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, @@ -1172,9 +1165,7 @@ glsl_type::get_function_instance(const glsl_type *return_type, 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); } @@ -1184,7 +1175,7 @@ glsl_type::get_function_instance(const glsl_type *return_type, 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; } diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h index 9da0cbcc3b0..55faac25331 100644 --- a/src/compiler/glsl_types.h +++ b/src/compiler/glsl_types.h @@ -149,7 +149,7 @@ struct glsl_type { * 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); @@ -159,7 +159,7 @@ struct glsl_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; } @@ -168,9 +168,9 @@ struct glsl_type { * 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); } /** @@ -820,7 +820,8 @@ struct glsl_type { private: - static mtx_t mutex; + static mtx_t mem_mutex; + static mtx_t hash_mutex; /** * ralloc context for all glsl_type allocations -- 2.30.2