[Mesa-dev] [PATCH 1/9] glsl: fix a race condition when inserting new types
Nicolai Hähnle
nhaehnle at gmail.com
Mon May 15 09:27:25 UTC 2017
From: Nicolai Hähnle <nicolai.haehnle at 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
More information about the mesa-dev
mailing list