[PATCH v2 0/2] MR308: vkd3d-shader/hlsl: Initialize all constant values in hlsl_fold_constant_swizzles().

[Francisco Casas (＠fcasas)]

Even though the uninitialized values shouldn't be used in the output
binary program, they still show up in it, and affect the checksum, so
we better make them zeroes.

This error was reported by valgrind:

```
libtool --mode=execute valgrind --track-origins=yes vkd3d-build/tests/shader_runner vkd3d/tests/hlsl/static-initializer.shader_test
```

```
==46180== Conditional jump or move depends on uninitialised value(s)
==46180==    at 0x48D98C7: parse_dxbc.isra.0 (dxbc.c:182)
==46180==    by 0x48DA044: vkd3d_shader_parse_dxbc (dxbc.c:308)
==46180==    by 0x488D1B6: vkd3d_shader_parse_dxbc_source_type (vkd3d_shader_utils.h:37)
==46180==    by 0x488D1B6: create_shader_stage.isra.0 (state.c:1988)
==46180==    by 0x48926B6: d3d12_pipeline_state_init_graphics (state.c:3084)
==46180==    by 0x4893A96: d3d12_pipeline_state_create_graphics (state.c:3280)
==46180==    by 0x4878498: d3d12_device_CreateGraphicsPipelineState (device.c:2619)
==46180==    by 0x1FFEFFECC7: ???
==46180==    by 0xE8: ???
==46180==    by 0x47: ???
==46180==    by 0x61: ???
==46180==    by 0x660066000000023: ???
==46180==    by 0x661066100000044: ???
==46180==  Uninitialised value was created by a stack allocation
==46180==    at 0x48F3FF0: hlsl_fold_constant_swizzles (hlsl_constant_ops.c:1010)
```

Thank you valgrind! :smile:

--
  v2: vkd3d-shader/hlsl: Initialize unused constant components to zero.
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308

[PATCH v2 1/2] vkd3d-shader/hlsl: Initialize all constant values in hlsl_fold_constant_swizzles().

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

Even though the uninitialized values shouldn't be used in the output
binary program, they still show up in it, and affect the checksum, so
we better make them zeroes.

This error was reported by valgrind

    libtool --mode=execute valgrind --track-origins=yes vkd3d-build/tests/shader_runner vkd3d/tests/hlsl/static-initializer.shader_test

    ==46180== Conditional jump or move depends on uninitialised value(s)
    ==46180==    at 0x48D98C7: parse_dxbc.isra.0 (dxbc.c:182)
    ==46180==    by 0x48DA044: vkd3d_shader_parse_dxbc (dxbc.c:308)
    ==46180==    by 0x488D1B6: vkd3d_shader_parse_dxbc_source_type (vkd3d_shader_utils.h:37)
    ==46180==    by 0x488D1B6: create_shader_stage.isra.0 (state.c:1988)
    ==46180==    by 0x48926B6: d3d12_pipeline_state_init_graphics (state.c:3084)
    ==46180==    by 0x4893A96: d3d12_pipeline_state_create_graphics (state.c:3280)
    ==46180==    by 0x4878498: d3d12_device_CreateGraphicsPipelineState (device.c:2619)
    ==46180==    by 0x1FFEFFECC7: ???
    ==46180==    by 0xE8: ???
    ==46180==    by 0x47: ???
    ==46180==    by 0x61: ???
    ==46180==    by 0x660066000000023: ???
    ==46180==    by 0x661066100000044: ???
    ==46180==  Uninitialised value was created by a stack allocation
    ==46180==    at 0x48F3FF0: hlsl_fold_constant_swizzles (hlsl_constant_ops.c:1010)
---
 libs/vkd3d-shader/hlsl_constant_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
index 41a72ab6..28df074d 100644
--- a/libs/vkd3d-shader/hlsl_constant_ops.c
+++ b/libs/vkd3d-shader/hlsl_constant_ops.c
@@ -1008,7 +1008,7 @@ bool hlsl_fold_constant_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
 
 bool hlsl_fold_constant_swizzles(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
 {
-    struct hlsl_constant_value value;
+    struct hlsl_constant_value value = {};
     struct hlsl_ir_swizzle *swizzle;
     struct hlsl_ir_constant *src;
     struct hlsl_ir_node *dst;
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308

[PATCH v2 2/2] vkd3d-shader/hlsl: Initialize unused constant components to zero.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

While some of the components of vec4 immconsts may not be used, they are
still written in the immconst declaration, so it is better if they are
initialized as zero.
---
 libs/vkd3d-shader/hlsl_codegen.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index bfa605f4..d3676c09 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -2355,7 +2355,7 @@ static bool lower_round(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *
 {
     struct hlsl_ir_node *arg, *neg, *sum, *frc, *half, *replacement;
     struct hlsl_type *type = instr->data_type;
-    struct hlsl_constant_value half_value;
+    struct hlsl_constant_value half_value = {};
     unsigned int i, component_count;
     struct hlsl_ir_expr *expr;
 
@@ -2466,7 +2466,7 @@ static bool lower_int_division(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
 {
     struct hlsl_ir_node *arg1, *arg2, *xor, *and, *abs1, *abs2, *div, *neg, *cast1, *cast2, *cast3, *high_bit;
     struct hlsl_type *type = instr->data_type, *utype;
-    struct hlsl_constant_value high_bit_value;
+    struct hlsl_constant_value high_bit_value = {};
     struct hlsl_ir_expr *expr;
     unsigned int i;
 
@@ -2532,7 +2532,7 @@ static bool lower_int_modulus(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
 {
     struct hlsl_ir_node *arg1, *arg2, *and, *abs1, *abs2, *div, *neg, *cast1, *cast2, *cast3, *high_bit;
     struct hlsl_type *type = instr->data_type, *utype;
-    struct hlsl_constant_value high_bit_value;
+    struct hlsl_constant_value high_bit_value = {};
     struct hlsl_ir_expr *expr;
     unsigned int i;
 
@@ -2675,7 +2675,7 @@ static bool lower_float_modulus(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr
 {
     struct hlsl_ir_node *arg1, *arg2, *mul1, *neg1, *ge, *neg2, *div, *mul2, *frc, *cond, *one, *mul3;
     struct hlsl_type *type = instr->data_type, *btype;
-    struct hlsl_constant_value one_value;
+    struct hlsl_constant_value one_value = {};
     struct hlsl_ir_expr *expr;
     unsigned int i;
 
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308

[Francisco Casas (＠fcasas)]

:arrow_up: As an afterthought, I added 2/2 to also initialize the `struct hlsl_constant_value` components to zero in other passes where this issue may also happen.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308#note_42645

[PATCH v2 0/2] MR308: vkd3d-shader/hlsl: Initialize unused constant values in compilation passes. - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.
-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308

[PATCH v2 0/2] MR308: vkd3d-shader/hlsl: Initialize unused constant values in compilation passes. - approved

[Henri Verbeet (＠hverbeet)]

This merge request was approved by Henri Verbeet.
-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308

[PATCH v2 0/2] MR308: vkd3d-shader/hlsl: Initialize unused constant values in compilation passes.

[Zebediah Figura (＠zfigura)]

If the concern is unused components being written in immconst registers, this doesn't seem like the right place to fix it.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308#note_42887

[PATCH v2 0/2] MR308: vkd3d-shader/hlsl: Initialize unused constant values in compilation passes.

[Henri Verbeet (＠hverbeet)]

> If the concern is unused components being written in immconst registers, this doesn't seem like the right place to fix it.

You're probably right; I don't think we should be writing these in the backend. I was thinking it's not great to carry uninitialised data in the HLSL IR either, but we should actually simply never access values beyond what's defined by the associated types.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308#note_42996