-- v2: vkd3d-shader/hlsl: Forbid recursive calls.
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-shader/hlsl.y | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 019c875c..b7fb52bb 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3878,11 +3878,8 @@ func_prototype_no_attrs: struct hlsl_type *type;
if (modifiers & ~HLSL_MODIFIERS_MAJORITY_MASK) - { hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, "Only majority modifiers are allowed on functions."); - YYABORT; - } if (!(type = apply_type_modifiers(ctx, $2, &modifiers, @1))) YYABORT; if ((var = hlsl_get_var(ctx->globals, $3))) @@ -3891,7 +3888,6 @@ func_prototype_no_attrs: ""%s" is already declared as a variable.", $3); hlsl_note(ctx, &var->loc, VKD3D_SHADER_LOG_ERROR, ""%s" was previously declared here.", $3); - YYABORT; } if (hlsl_types_are_equal(type, ctx->builtin_types.Void) && $7.semantic.name) {
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-shader/hlsl.c | 2 +- libs/vkd3d-shader/hlsl.h | 2 +- libs/vkd3d-shader/hlsl.y | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index a440aa39..8cca8d47 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2185,7 +2185,7 @@ static void free_function_rb(struct rb_entry *entry, void *context) free_function(RB_ENTRY_VALUE(entry, struct hlsl_ir_function, entry)); }
-void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function_decl *decl, bool intrinsic) +void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function_decl *decl) { struct hlsl_ir_function *func; struct rb_entry *func_entry, *old_entry; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 14070239..2a29b907 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -894,7 +894,7 @@ struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru struct vkd3d_string_buffer *hlsl_modifiers_to_string(struct hlsl_ctx *ctx, unsigned int modifiers); const char *hlsl_node_type_to_string(enum hlsl_ir_node_type type);
-void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function_decl *decl, bool intrinsic); +void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function_decl *decl); bool hlsl_add_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *decl, bool local_var);
void hlsl_dump_function(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl *func); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index b7fb52bb..a8942d61 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3612,7 +3612,7 @@ hlsl_prog: } }
- hlsl_add_function(ctx, $2.name, $2.decl, false); + hlsl_add_function(ctx, $2.name, $2.decl); } | hlsl_prog buffer_declaration buffer_body | hlsl_prog declaration_statement
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-shader/hlsl.c | 2 +- libs/vkd3d-shader/hlsl.h | 4 ++-- libs/vkd3d-shader/hlsl.y | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 8cca8d47..13aac3d4 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -931,7 +931,7 @@ struct hlsl_ir_store *hlsl_new_store_component(struct hlsl_ctx *ctx, struct hlsl return store; }
-struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl *decl, +struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *decl, const struct vkd3d_shader_location *loc) { struct hlsl_ir_call *call; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 2a29b907..3d8a8b4b 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -393,7 +393,7 @@ struct hlsl_ir_function_decl struct hlsl_ir_call { struct hlsl_ir_node node; - const struct hlsl_ir_function_decl *decl; + struct hlsl_ir_function_decl *decl; };
struct hlsl_ir_if @@ -927,7 +927,7 @@ struct hlsl_ir_node *hlsl_new_binary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_exp struct hlsl_ir_constant *hlsl_new_bool_constant(struct hlsl_ctx *ctx, bool b, const struct vkd3d_shader_location *loc); struct hlsl_buffer *hlsl_new_buffer(struct hlsl_ctx *ctx, enum hlsl_buffer_type type, const char *name, const struct hlsl_reg_reservation *reservation, struct vkd3d_shader_location loc); -struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl *decl, +struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *decl, const struct vkd3d_shader_location *loc); struct hlsl_ir_expr *hlsl_new_cast(struct hlsl_ctx *ctx, struct hlsl_ir_node *node, struct hlsl_type *type, const struct vkd3d_shader_location *loc); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index a8942d61..511a7b90 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2183,12 +2183,12 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t struct find_function_call_args { const struct parse_initializer *params; - const struct hlsl_ir_function_decl *decl; + struct hlsl_ir_function_decl *decl; };
static void find_function_call_exact(struct rb_entry *entry, void *context) { - const struct hlsl_ir_function_decl *decl = RB_ENTRY_VALUE(entry, const struct hlsl_ir_function_decl, entry); + struct hlsl_ir_function_decl *decl = RB_ENTRY_VALUE(entry, struct hlsl_ir_function_decl, entry); struct find_function_call_args *args = context; const struct hlsl_ir_var *param; unsigned int i = 0; @@ -2203,7 +2203,7 @@ static void find_function_call_exact(struct rb_entry *entry, void *context) args->decl = decl; }
-static const struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *ctx, +static struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *ctx, const char *name, const struct parse_initializer *params) { struct find_function_call_args args = {.params = params}; @@ -2860,8 +2860,8 @@ static int intrinsic_function_name_compare(const void *a, const void *b) static struct list *add_call(struct hlsl_ctx *ctx, const char *name, struct parse_initializer *args, const struct vkd3d_shader_location *loc) { - const struct hlsl_ir_function_decl *decl; struct intrinsic_function *intrinsic; + struct hlsl_ir_function_decl *decl;
if ((decl = find_function_call(ctx, name, args))) {
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-shader/hlsl_codegen.c | 47 ++++++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 1 + tests/hlsl-function.shader_test | 38 +++++++++++++++++++ tests/shader_runner.c | 10 +++++ 4 files changed, 96 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 71e515b8..1f3d5fef 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -457,6 +457,48 @@ static bool transform_ir(struct hlsl_ctx *ctx, bool (*func)(struct hlsl_ctx *ctx return progress; }
+struct recursive_call_ctx +{ + const struct hlsl_ir_function_decl **backtrace; + size_t count, capacity; +}; + +static bool find_recursive_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct recursive_call_ctx *call_ctx = context; + struct hlsl_ir_function_decl *decl; + const struct hlsl_ir_call *call; + size_t i; + + if (instr->type != HLSL_IR_CALL) + return false; + call = hlsl_ir_call(instr); + decl = call->decl; + + for (i = 0; i < call_ctx->count; ++i) + { + if (call_ctx->backtrace[i] == decl) + { + hlsl_error(ctx, &call->node.loc, VKD3D_SHADER_ERROR_HLSL_RECURSIVE_CALL, + "Recursive call to "%s".", decl->func->name); + /* Native returns E_NOTIMPL instead of E_FAIL here. */ + ctx->result = VKD3D_ERROR_NOT_IMPLEMENTED; + return false; + } + } + + if (!hlsl_array_reserve(ctx, (void **)&call_ctx->backtrace, &call_ctx->capacity, + call_ctx->count + 1, sizeof(*call_ctx->backtrace))) + return false; + call_ctx->backtrace[call_ctx->count++] = decl; + + transform_ir(ctx, find_recursive_calls, &decl->body, call_ctx); + + --call_ctx->count; + + return false; +} + /* Lower casts from vec1 to vecN to swizzles. */ static bool lower_broadcasts(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { @@ -2609,12 +2651,17 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry { const struct hlsl_profile_info *profile = ctx->profile; struct hlsl_block *const body = &entry_func->body; + struct recursive_call_ctx recursive_call_ctx; struct hlsl_ir_var *var; unsigned int i; bool progress;
list_move_head(&body->instrs, &ctx->static_initializers);
+ memset(&recursive_call_ctx, 0, sizeof(recursive_call_ctx)); + transform_ir(ctx, find_recursive_calls, body, &recursive_call_ctx); + vkd3d_free(recursive_call_ctx.backtrace); + LIST_FOR_EACH_ENTRY(var, &ctx->globals->vars, struct hlsl_ir_var, scope_entry) { if (var->storage_modifiers & HLSL_STORAGE_UNIFORM) diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 74edf049..5002a13c 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -121,6 +121,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF = 5022, VKD3D_SHADER_ERROR_HLSL_INVALID_THREAD_COUNT = 5023, VKD3D_SHADER_ERROR_HLSL_MISSING_ATTRIBUTE = 5024, + VKD3D_SHADER_ERROR_HLSL_RECURSIVE_CALL = 5025,
VKD3D_SHADER_WARNING_HLSL_IMPLICIT_TRUNCATION = 5300, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO = 5301, diff --git a/tests/hlsl-function.shader_test b/tests/hlsl-function.shader_test index e8bb80c4..eee6f2d1 100644 --- a/tests/hlsl-function.shader_test +++ b/tests/hlsl-function.shader_test @@ -162,3 +162,41 @@ float4 main() : sv_target [test] draw quad todo probe all rgba (0.6, 0.1, 0.5, 0) + +% Recursion is forbidden. + +[pixel shader notimpl] + +void bar(); + +void foo() +{ + bar(); +} + +void bar() +{ + foo(); +} + +float4 main() : sv_target +{ + foo(); + return 0; +} + +[pixel shader notimpl todo] + +% Even trivially finite recursion is forbidden. + +void func(bool x) +{ + if (x) + func(false); +} + +float4 main() : sv_target +{ + func(true); + return 0; +} diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 0ed070a1..0f205bfe 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -893,6 +893,16 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const state = STATE_SHADER_PIXEL_TODO; expect_hr = E_FAIL; } + else if (!strcmp(line, "[pixel shader notimpl]\n")) + { + state = STATE_SHADER_PIXEL; + expect_hr = E_NOTIMPL; + } + else if (!strcmp(line, "[pixel shader notimpl todo]\n")) + { + state = STATE_SHADER_PIXEL_TODO; + expect_hr = E_NOTIMPL; + } else if (sscanf(line, "[sampler %u]\n", &index)) { state = STATE_SAMPLER;
From: Zebediah Figura zfigura@codeweavers.com
--- tests/shader_runner.c | 52 +++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 34 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 4750b946..0ed070a1 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -82,10 +82,6 @@ enum parse_state STATE_SAMPLER, STATE_SHADER_COMPUTE, STATE_SHADER_COMPUTE_TODO, - STATE_SHADER_INVALID_COMPUTE, - STATE_SHADER_INVALID_COMPUTE_TODO, - STATE_SHADER_INVALID_PIXEL, - STATE_SHADER_INVALID_PIXEL_TODO, STATE_SHADER_PIXEL, STATE_SHADER_PIXEL_TODO, STATE_SHADER_VERTEX, @@ -661,7 +657,7 @@ unsigned int get_vb_stride(const struct shader_runner *runner, unsigned int slot return stride; }
-static void compile_shader(struct shader_runner *runner, const char *source, size_t len, const char *type, bool invalid) +static void compile_shader(struct shader_runner *runner, const char *source, size_t len, const char *type, HRESULT expect) { ID3D10Blob *blob = NULL, *errors = NULL; char profile[7]; @@ -678,10 +674,7 @@ static void compile_shader(struct shader_runner *runner, const char *source, siz
sprintf(profile, "%s_%s", type, shader_models[runner->minimum_shader_model]); hr = D3DCompile(source, len, NULL, NULL, NULL, "main", profile, 0, 0, &blob, &errors); - if (invalid) - ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); - else - ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == expect, "Got unexpected hr %#x.\n", hr); if (hr == S_OK) { ID3D10Blob_Release(blob); @@ -708,6 +701,7 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const unsigned int i, line_number = 0; const char *filename = NULL; char *shader_source = NULL; + HRESULT expect_hr = S_OK; char line[256]; FILE *f;
@@ -766,7 +760,7 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const case STATE_SHADER_COMPUTE: case STATE_SHADER_COMPUTE_TODO: todo_if (state == STATE_SHADER_COMPUTE_TODO) - compile_shader(runner, shader_source, shader_source_len, "cs", false); + compile_shader(runner, shader_source, shader_source_len, "cs", expect_hr); free(runner->cs_source); runner->cs_source = shader_source; shader_source = NULL; @@ -777,7 +771,7 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const case STATE_SHADER_PIXEL: case STATE_SHADER_PIXEL_TODO: todo_if (state == STATE_SHADER_PIXEL_TODO) - compile_shader(runner, shader_source, shader_source_len, "ps", false); + compile_shader(runner, shader_source, shader_source_len, "ps", expect_hr); free(runner->ps_source); runner->ps_source = shader_source; shader_source = NULL; @@ -786,7 +780,7 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const break;
case STATE_SHADER_VERTEX: - compile_shader(runner, shader_source, shader_source_len, "vs", false); + compile_shader(runner, shader_source, shader_source_len, "vs", expect_hr); free(runner->vs_source); runner->vs_source = shader_source; shader_source = NULL; @@ -794,20 +788,6 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const shader_source_size = 0; break;
- case STATE_SHADER_INVALID_COMPUTE: - case STATE_SHADER_INVALID_COMPUTE_TODO: - todo_if (state == STATE_SHADER_INVALID_COMPUTE_TODO) - compile_shader(runner, shader_source, shader_source_len, "cs", true); - shader_source_len = 0; - break; - - case STATE_SHADER_INVALID_PIXEL: - case STATE_SHADER_INVALID_PIXEL_TODO: - todo_if (state == STATE_SHADER_INVALID_PIXEL_TODO) - compile_shader(runner, shader_source, shader_source_len, "ps", true); - shader_source_len = 0; - break; - case STATE_PREPROC_INVALID: { ID3D10Blob *blob = NULL, *errors = NULL; @@ -872,18 +852,22 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const if (!strcmp(line, "[compute shader]\n")) { state = STATE_SHADER_COMPUTE; + expect_hr = S_OK; } else if (!strcmp(line, "[compute shader todo]\n")) { state = STATE_SHADER_COMPUTE_TODO; + expect_hr = S_OK; } else if (!strcmp(line, "[compute shader fail]\n")) { - state = STATE_SHADER_INVALID_COMPUTE; + state = STATE_SHADER_COMPUTE; + expect_hr = E_FAIL; } else if (!strcmp(line, "[compute shader fail todo]\n")) { - state = STATE_SHADER_INVALID_COMPUTE_TODO; + state = STATE_SHADER_COMPUTE_TODO; + expect_hr = E_FAIL; } else if (!strcmp(line, "[require]\n")) { @@ -892,18 +876,22 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const else if (!strcmp(line, "[pixel shader]\n")) { state = STATE_SHADER_PIXEL; + expect_hr = S_OK; } else if (!strcmp(line, "[pixel shader todo]\n")) { state = STATE_SHADER_PIXEL_TODO; + expect_hr = S_OK; } else if (!strcmp(line, "[pixel shader fail]\n")) { - state = STATE_SHADER_INVALID_PIXEL; + state = STATE_SHADER_PIXEL; + expect_hr = E_FAIL; } else if (!strcmp(line, "[pixel shader fail todo]\n")) { - state = STATE_SHADER_INVALID_PIXEL_TODO; + state = STATE_SHADER_PIXEL_TODO; + expect_hr = E_FAIL; } else if (sscanf(line, "[sampler %u]\n", &index)) { @@ -1012,10 +1000,6 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const case STATE_PREPROC_INVALID: case STATE_SHADER_COMPUTE: case STATE_SHADER_COMPUTE_TODO: - case STATE_SHADER_INVALID_COMPUTE: - case STATE_SHADER_INVALID_COMPUTE_TODO: - case STATE_SHADER_INVALID_PIXEL: - case STATE_SHADER_INVALID_PIXEL_TODO: case STATE_SHADER_PIXEL: case STATE_SHADER_PIXEL_TODO: case STATE_SHADER_VERTEX:
Annoyingly I missed Francisco's comment due to the bridge malfunctioning. Pushed a v2 now with that fixed, thanks.
This merge request was approved by Francisco Casas.
This merge request was approved by Giovanni Mascellani.
This merge request was approved by Henri Verbeet.
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)