From: Nicolai Hähnle <[email protected]> When determining whether the array is implicitly sized, we must avoid accessing var->data.mode (around line 5270), because var may have been deleted.
Instead of adding yet another ternary condition based on earlier == NULL, refactor get_variable_being_redeclared so that we do not keep dangling pointers around. Found by ASAN in GL45-CTS.gtf21.GL.build.array10_frag. Cc: [email protected] --- src/compiler/glsl/ast_to_hir.cpp | 55 ++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index b90ad97..57a9db9 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -3942,44 +3942,50 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual, "the shared storage qualifiers can only be used with " "compute shaders"); } apply_image_qualifier_to_variable(qual, var, state, loc); } /** * Get the variable that is being redeclared by this declaration * + * \p pvar is changed to point to an existing variable in the current scope if + * the declaration that it initially points to is a redeclaration. + * + * The declaration originally pointed to by \p pvar may be deleted. + * * Semantic checks to verify the validity of the redeclaration are also * performed. If semantic checks fail, compilation error will be emitted via - * \c _mesa_glsl_error, but a non-\c NULL pointer will still be returned. + * \c _mesa_glsl_error, but a non-\c NULL pointer will still be provided. * * \returns - * A pointer to an existing variable in the current scope if the declaration - * is a redeclaration, \c NULL otherwise. + * Whether the declaration is a redeclaration. */ -static ir_variable * -get_variable_being_redeclared(ir_variable *var, YYLTYPE loc, +static bool +get_variable_being_redeclared(ir_variable **pvar, YYLTYPE loc, struct _mesa_glsl_parse_state *state, bool allow_all_redeclarations) { + ir_variable *var = *pvar; + /* Check if this declaration is actually a re-declaration, either to * resize an array or add qualifiers to an existing variable. * * This is allowed for variables in the current scope, or when at * global scope (for built-ins in the implicit outer scope). */ ir_variable *earlier = state->symbols->get_variable(var->name); if (earlier == NULL || (state->current_function != NULL && !state->symbols->name_declared_this_scope(var->name))) { - return NULL; + return false; } /* From page 24 (page 30 of the PDF) of the GLSL 1.50 spec, * * "It is legal to declare an array without a size and then * later re-declare the same name as an array of the same * type and specify a size." */ if (earlier->type->is_unsized_array() && var->type->is_array() @@ -4082,21 +4088,22 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc, var->name); } else if (earlier->type != var->type) { _mesa_glsl_error(&loc, state, "redeclaration of `%s' has incorrect type", var->name); } } else { _mesa_glsl_error(&loc, state, "`%s' redeclared", var->name); } - return earlier; + *pvar = earlier; + return true; } /** * Generate the IR for an initializer in a variable declaration */ ir_rvalue * process_initializer(ir_variable *var, ast_declaration *decl, ast_fully_specified_type *type, exec_list *initializer_instructions, struct _mesa_glsl_parse_state *state) @@ -5196,56 +5203,54 @@ ast_declarator_list::hir(exec_list *instructions, * list. This list will be added to the instruction stream (below) after * the declaration is added. This is done because in some cases (such as * redeclarations) the declaration may not actually be added to the * instruction stream. */ exec_list initializer_instructions; /* Examine var name here since var may get deleted in the next call */ bool var_is_gl_id = is_gl_identifier(var->name); - ir_variable *earlier = - get_variable_being_redeclared(var, decl->get_location(), state, + bool is_redeclaration = + get_variable_being_redeclared(&var, decl->get_location(), state, false /* allow_all_redeclarations */); - if (earlier != NULL) { + if (is_redeclaration) { if (var_is_gl_id && - earlier->data.how_declared == ir_var_declared_in_block) { + var->data.how_declared == ir_var_declared_in_block) { _mesa_glsl_error(&loc, state, "`%s' has already been redeclared using " - "gl_PerVertex", earlier->name); + "gl_PerVertex", var->name); } - earlier->data.how_declared = ir_var_declared_normally; + var->data.how_declared = ir_var_declared_normally; } if (decl->initializer != NULL) { - result = process_initializer((earlier == NULL) ? var : earlier, - decl, this->type, + result = process_initializer(var, decl, this->type, &initializer_instructions, state); } else { validate_array_dimensions(var_type, state, &loc); } /* From page 23 (page 29 of the PDF) of the GLSL 1.10 spec: * * "It is an error to write to a const variable outside of * its declaration, so they must be initialized when * declared." */ if (this->type->qualifier.flags.q.constant && decl->initializer == NULL) { _mesa_glsl_error(& loc, state, "const declaration of `%s' must be initialized", decl->identifier); } if (state->es_shader) { - const glsl_type *const t = (earlier == NULL) - ? var->type : earlier->type; + const glsl_type *const t = var->type; /* Skip the unsized array check for TCS/TES/GS inputs & TCS outputs. * * The GL_OES_tessellation_shader spec says about inputs: * * "Declaring an array size is optional. If no size is specified, * it will be taken from the implementation-dependent maximum * patch size (gl_MaxPatchVertices)." * * and about TCS outputs: @@ -5286,21 +5291,21 @@ ast_declarator_list::hir(exec_list *instructions, */ _mesa_glsl_error(& loc, state, "unsized array declarations are not allowed in " "GLSL ES"); } /* If the declaration is not a redeclaration, there are a few additional * semantic checks that must be applied. In addition, variable that was * created for the declaration should be added to the IR stream. */ - if (earlier == NULL) { + if (!is_redeclaration) { validate_identifier(decl->identifier, loc, state); /* Add the variable to the symbol table. Note that the initializer's * IR was already processed earlier (though it hasn't been emitted * yet), without the variable in scope. * * This differs from most C-like languages, but it follows the GLSL * specification. From page 28 (page 34 of the PDF) of the GLSL 1.50 * spec: * @@ -7862,34 +7867,34 @@ ast_interface_block::hir(exec_list *instructions, var->data.matrix_layout = fields[i].matrix_layout; } if (var->data.mode == ir_var_shader_storage) apply_memory_qualifiers(var, fields[i]); /* Examine var name here since var may get deleted in the next call */ bool var_is_gl_id = is_gl_identifier(var->name); if (redeclaring_per_vertex) { - ir_variable *earlier = - get_variable_being_redeclared(var, loc, state, + bool is_redeclaration = + get_variable_being_redeclared(&var, loc, state, true /* allow_all_redeclarations */); - if (!var_is_gl_id || earlier == NULL) { + if (!var_is_gl_id || !is_redeclaration) { _mesa_glsl_error(&loc, state, "redeclaration of gl_PerVertex can only " "include built-in variables"); - } else if (earlier->data.how_declared == ir_var_declared_normally) { + } else if (var->data.how_declared == ir_var_declared_normally) { _mesa_glsl_error(&loc, state, "`%s' has already been redeclared", - earlier->name); + var->name); } else { - earlier->data.how_declared = ir_var_declared_in_block; - earlier->reinit_interface_type(block_type); + var->data.how_declared = ir_var_declared_in_block; + var->reinit_interface_type(block_type); } continue; } if (state->symbols->get_variable(var->name) != NULL) _mesa_glsl_error(&loc, state, "`%s' redeclared", var->name); /* Propagate the "binding" keyword into this UBO/SSBO's fields. * The UBO declaration itself doesn't get an ir_variable unless it * has an instance name. This is ugly. -- 2.9.3 _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
